Help - Inline::C arrayref deref?

[Lee]

Please help: I'm trying to do the my first real thing in Inline::C and
have got stuck trying to get expected values from an array reference.

I pass in an anonymous array of one value, the number 1, and I get back
135435488.

From the code you can see that I realise it is the way I getting at the

values in the input array, but I can't figure out where to go from
here...

Any help would be appreciated.

Thanks in anticipation
lee

Output from the code below:

$VAR1 = [ 99 ];
$VAR1 = [ 135435488 ];

Code:

#! perl
package Testing;
use strict;
use warnings;
use Data:umper;
use Inline "C";
Inline->init;
my @amps = test( 1, [1] );
warn Dumper @amps;
my @amps = test( 2, [2] );
warn Dumper @amps;
exit;
1;

__DATA__

__C__

AV* test (
int mode,
SV* rpcm
){
AV* av_ret = newAV(); // Return values
AV* av_pcm = newAV(); // Dereferenced PCM input array
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

if (mode==1){
int x = 99;
av_push( av_ret, newSViv(x) );
} else {
int x = av_fetch(av_pcm, 0, 0);
av_push( av_ret, newSViv(x) );
}

return av_ret;
}

 

[xhoster]

__C__

AV* test (
int mode,
SV* rpcm
){
AV* av_ret = newAV(); // Return values
AV* av_pcm = newAV(); // Dereferenced PCM input array
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

if (mode==1){
int x = 99;
av_push( av_ret, newSViv(x) );
} else {
int x = av_fetch(av_pcm, 0, 0);

av_fetch returns a SV**. You need to dereference it twice, once
in the normal C way and once with a special macro to get the integer
out of it.

int x = SvIV(*(av_fetch(av_pcm, 0, 0)));

av_push( av_ret, newSViv(x) );
}

return av_ret;
}

However, your code leaks like a sieve and I'm not sure how to fix that.

Xho

 

[Lee]

av_fetch returns a SV**. You need to dereference it twice, once
in the normal C way and once with a special macro to get the integer
out of it.

int x = SvIV(*(av_fetch(av_pcm, 0, 0)));


However, your code leaks like a sieve and I'm not sure how to fix that.

Me neither - I'm having problems enough with perlguts, but thank you
for your help. I totally overlooked the double star in the definition.

Lee

 

[xhoster]

Me neither - I'm having problems enough with perlguts, but thank you
for your help. I totally overlooked the double star in the definition.

I stopped the leaks by adding sv_2moral in two places:

AV* av_pcm=newAV() ; // Dereferenced PCM input array
sv_2mortal(av_pcm);
(Althought I don't particularly like this. The only reason the newAV()
seems to here is to prevent errors in the case that the thing passed in is
not an array reference, which should probably be handled down in the "if",
not up here.

And:

sv_2mortal(av_ret);
return av_ret;

Xho

 

[Sisyphus]

..
..

I stopped the leaks by adding sv_2moral in two places:

AV* av_pcm=newAV() ; // Dereferenced PCM input array
sv_2mortal(av_pcm);
(Althought I don't particularly like this. The only reason the newAV()
seems to here is to prevent errors in the case that the thing passed in is
not an array reference, which should probably be handled down in the "if",
not up here.

How does one conclusively detect the leakage ? I didn't really expect there
to be any leaks (and couldn't detect any) - but on second thoughts perhaps
there *is* a need to clean up av_pcm.

And:

sv_2mortal(av_ret);
return av_ret;

Looks to me that the C code (generated by xsubpp from the XS code)
mortalises av_ret - so this should not achieve anything. Here's the relevant
code from that C file:

XS(XS_main_test); /* prototype to pass -Wmissing-prototypes */
XS(XS_main_test)
{
dXSARGS;
if (items != 2)
Perl_croak(aTHX_ "Usage: main::test(mode, rpcm)");
{
int mode = (int)SvIV(ST(0));
SV * rpcm = ST(1);
AV * RETVAL;

RETVAL = test(mode, rpcm);
ST(0) = newRV((SV*)RETVAL);
sv_2mortal(ST(0));
}
XSRETURN(1);
}

Also, I was puzzled by the SvREFCNT_inc() call in the original code. I can't
see that it is either needed or desirable ..... still, one thing I've learnt
about XS issues is
to have little confidence in my expectations

Cheers,
Rob

 

[Sisyphus]

..
..

How does one conclusively detect the leakage ?

Sorry - dumb question. The answer is system dependent - and if I had been
looking in the *appropriate* place in Win32's Task manager I wouldn't have
had to ask.

Looks to me that the C code (generated by xsubpp from the XS code)
mortalises av_ret - so this should not achieve anything.

Nope - wrong again (surprise, surprise). Memory usage *does* increase
without the " sv_2mortal(av_ret);" .

Now .... off to make some senseless noise elsewhere

Cheers,
Rob

 

[Lee]

I stopped the leaks by adding sv_2moral in two places:

AV* av_pcm=newAV() ; // Dereferenced PCM input array
sv_2mortal(av_pcm);
(Althought I don't particularly like this. The only reason the newAV()
seems to here is to prevent errors in the case that the thing passed in is
not an array reference, which should probably be handled down in the "if",
not up here.

And:

sv_2mortal(av_ret);
return av_ret;

I've a feeling that would have taken me a few hours to find in the
manuals - thank you for taking the time.

Lee

 

[xhoster]

.
.

How does one conclusively detect the leakage ? I didn't really expect
there to be any leaks (and couldn't detect any) - but on second thoughts
perhaps there *is* a need to clean up av_pcm.

For the record, on Linux I just added this in the C code to detect
the leaks:

foreach (1..1_000) {
foreach (1..10_000) {
@amps=test(2,$x);
}
print +(`ps -p $$ -o rss `)[1];
};

Looks to me that the C code (generated by xsubpp from the XS code)
mortalises av_ret - so this should not achieve anything. Here's the
relevant code from that C file:

XS(XS_main_test); /* prototype to pass -Wmissing-prototypes */
XS(XS_main_test)
{
dXSARGS;
if (items != 2)
Perl_croak(aTHX_ "Usage: main::test(mode, rpcm)");
{
int mode = (int)SvIV(ST(0));
SV * rpcm = ST(1);
AV * RETVAL;

RETVAL = test(mode, rpcm);
ST(0) = newRV((SV*)RETVAL);
sv_2mortal(ST(0));

newRV increments the ref count, so this mortalization only accounts
for the reference held in ST(0), not the one held in av_ret,
which needs to be mortalized separately.

}
XSRETURN(1);
}

Also, I was puzzled by the SvREFCNT_inc() call in the original code.

You are right, that also shouldn't be there. I had cleaned it up but
forgot to mention doing so. That one is particularly pernicious, because
it causes the leak to occur in the regular Perl code.

This stub won't leak (much) because althougth the refcount on @$x increases
there is only ever one @$x to get leaked.

my $x=[2]
foreach (1..1_000) {
foreach (1..10_000) {
@amps=test2(2,$x);
}
print +(`ps -p $$ -o rss `)[1];
};

However, if you move the my $x=[2] to the inner most loop, then it will
leak unless you remove the SvREFCNT_inc()

Xho

 

[xhoster]

For the record, on Linux I just added this in the C code to detect
the leaks:

Argh. I added that in the Perl code, of course, not the C code.

foreach (1..1_000) {
foreach (1..10_000) {
@amps=test(2,$x);
}
print +(`ps -p $$ -o rss `)[1];
};

Xho

 

[Lee]

You are right, that also shouldn't be there. I had cleaned it up but
forgot to mention doing so. That one is particularly pernicious, because
it causes the leak to occur in the regular Perl code.

Could you please explain? I'm confused because my XS and perlguts is
minimal. If I replace this:

AV* av_pcm = newAV();
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

with this:

AV* av_pcm = newAV();
av_pcm = (AV*)(SvRV(rpcm));

I get told, "Attempt to free unreferenced scalar: SV 0xb79d7540"

Lee

 

[Sisyphus]

Could you please explain? I'm confused because my XS and perlguts is
minimal. If I replace this:

AV* av_pcm = newAV();
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

with this:

AV* av_pcm = newAV();
av_pcm = (AV*)(SvRV(rpcm));

I get told, "Attempt to free unreferenced scalar: SV 0xb79d7540"

Lee

It seems to depend upon the placement of the sv_2mortal call. In the below
script, the foo2 function will try to "free unreferenced scalar", but with
foo1 there's no such problem (and neither foo1 nor foo2 leak memory afaict):

-- av.pl --
use warnings;
use Inline C => Config => BUILD_NOISY => 1;

use Inline C => <<'EOC';

AV * foo1(SV * x) {
AV * av_ret = newAV();
sv_2mortal(av_ret);
av_ret = (AV*)SvRV(x);
return av_ret;
}


AV * foo2(SV * x) {
AV * av_ret = newAV();
av_ret = (AV*)SvRV(x);
sv_2mortal(av_ret);
return av_ret;
}

AV * foo3(SV * x) {
AV * av_ret = newAV();
av_ret = (AV*)SvRV(x);
return av_ret;
}

EOC

@z = (1 .. 10);

print "Running 10 iterations of foo1\n";
for(1..10) {$x1 = foo1(\@z)}

print "Running 10 iterations of foo2\n";
for(1..10) {$x2 = foo2(\@z)}

print "Running 10 iterations of foo3\n";
for(1..10) {$x3 = foo3(\@z)}
__END__

I really don't know whether it's correct to be providing an AV* as the
argument to sv_2mortal(). There's no denying that it fixes the memory leak
problem, but it also produces the following warnings during the compilation
phase:

av_pl_4246.xs: In function `foo1':
av_pl_4246.xs:8: warning: passing arg 2 of `Perl_sv_2mortal' from
incompatible pointer type
av_pl_4246.xs: In function `foo2':
av_pl_4246.xs:17: warning: passing arg 2 of `Perl_sv_2mortal' from
incompatiblepointer type

The foo3 function leaks memory, but at least demonstrates (I hope) that the
refcount increase is not necessary.

Someone else may be able to provide more definitive advice - but, hopefully,
there's something here that helps.

Btw, the output of the above script (for me) is:

Name "main::x1" used only once: possible typo at av.pl line 32.
Name "main::x3" used only once: possible typo at av.pl line 38.
Name "main::x2" used only once: possible typo at av.pl line 35.
Running 10 iterations of foo1
Running 10 iterations of foo2
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Attempt to free unreferenced scalar: SV 0x3f5d78, Perl interpreter: 0x3f416c
at av.pl line 35.
Running 10 iterations of foo3

Cheers,
Rob

 

[xhoster]

Could you please explain? I'm confused because my XS and perlguts is
minimal. If I replace this:

AV* av_pcm = newAV();
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

with this:

AV* av_pcm = newAV();
av_pcm = (AV*)(SvRV(rpcm));

I get told, "Attempt to free unreferenced scalar: SV 0xb79d7540"

I don't get that if I make that change on the code you originally posted in
the first post of this thread. Are you sure you don't make some other
change too? Can you show the complete code for the new C sub?


Xho

 

[Lee]

I don't get that if I make that change on the code you originally posted in
the first post of this thread. Are you sure you don't make some other
change too? Can you show the complete code for the new C sub?

Here it is - my first C in years, my first every C/Perl integration:

__C__

AV* get_y (
int samples_per_pixel,
int height,
float v_scale,
SV* rpcm
){
int i,j,fin;
AV* av_ret = newAV();
AV* av_pcm = newAV();
if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));
sv_2mortal(av_pcm); // Thx perl.misc xho
fin = av_len(av_pcm) - samples_per_pixel;

for (i=0; i <= av_len(av_pcm); i += samples_per_pixel ){
int v = 0;
if (i < fin){
for (j=i; j <= i + samples_per_pixel; j++){
v += SvIV(*(av_fetch(av_pcm, j, 0))); // Get the velocity element
}
v /= samples_per_pixel ;
} else {
for (j=i; j <= av_len(av_pcm); j++){
v += SvIV(*(av_fetch(av_pcm, j, 0))); // Get the velocity element
}
v /= (av_len(av_pcm) - i);
}
v = height - (v * v_scale); // Scale the velocity for the graph
SV* sv_subtot = newSViv( v );
av_push( av_ret, sv_subtot );
}

sv_2mortal(av_ret);
return av_ret;
}

Lee

 

[xhoster]

It seems to depend upon the placement of the sv_2mortal call. In the
below script, the foo2 function will try to "free unreferenced scalar",
but with foo1 there's no such problem (and neither foo1 nor foo2 leak
memory afaict):

In foo2, the initial value of av_ret (the one from newAV()) never gets
freed at all, well the 2nd value of av_ret gets freed twice. If that
doesn't leak (and it appears not to) then apparently sometimes two wrongs
can indeed make a right!

AV * foo2(SV * x) {
AV * av_ret = newAV();
av_ret = (AV*)SvRV(x);
sv_2mortal(av_ret);
return av_ret;
}
....


I really don't know whether it's correct to be providing an AV* as the
argument to sv_2mortal(). There's no denying that it fixes the memory
leak problem, but it also produces the following warnings during the
compilation phase:

If you cast the AV* to an SV* first, the warning goes away.

sv_2mortal((SV*) av_ret);

Of course, the lack of warning still doesn't prove that it is proper. I
haven't found any examples of this in the docs, but there are plenty of
analogs to it:
SV* newRV_inc((SV*) thing);
SV* newRV_noinc((SV*) thing);


Xho

 

[xhoster]

Here it is - my first C in years, my first every C/Perl integration:

__C__

AV* get_y (
int samples_per_pixel,
int height,
float v_scale,
SV* rpcm
){
int i,j,fin;
AV* av_ret = newAV();
AV* av_pcm = newAV();

The new thingy that av_pcm points to needs to be cleaned up eventually.
The best way to make sure that happens is to call sv_2mortal now.
(Although I still question the need for the newAV at all, since in
non-error conditions it will immediately get thrown away.

if (SvROK(rpcm) && (SvTYPE(SvRV(rpcm)) == SVt_PVAV))
av_pcm = (AV*)SvREFCNT_inc(SvRV(rpcm));

Here (assuming the if statement works out, which it will if you actually
passed an arrayref), you overwrite the value of av_pcm, permanently loosing
it's old value. Now it is impossible for it to get cleaned up, since
no one any longer has track of it.

sv_2mortal(av_pcm); // Thx perl.misc xho

Here you tell it to mortalize the new value of av_pcm (again, assuming the
if statement was satisfied). Because of the SvREFCNT_inc, the new value
actually does need to be mortalized. If you don't do the SvREFCNT_inc,
then this doesn't need to be (and in fact needs not to be) mortalized.

So get rid of the SvREFCNT_inc, then move the sv_2mortal to before rather
than after the if statement.

Xho

 

[Anno Siegel]

If you cast the AV* to an SV* first, the warning goes away.

sv_2mortal((SV*) av_ret);

Of course, the lack of warning still doesn't prove that it is proper. I
haven't found any examples of this in the docs, but there are plenty of
analogs to it:
SV* newRV_inc((SV*) thing);
SV* newRV_noinc((SV*) thing);

It's okay. I don't know about documentation, but the perl source has
examples,

pad.c: *out_capture = sv_2mortal((SV*)newAV());
pad.c: *out_capture = sv_2mortal((SV*)newHV());

for two.

However, the code (now snipped) didn't need the newAV() at all, as far
as I can see.

Anno

Anno