av_push, as documented, takes ownership of one reference count. If you mortalise the scalars, their refcnt will be reduced to zero even though the array still hold a reference to them. Bug. Don't mortalise them.

AV* return_an_array() 
{
    AV* av = newAV();
    av_push( av, newSViv( 7  ) );
    av_push( av, newSviv( 42 ) );
    return av;
}
[download]

The thing is, Perl has a bug of its own. The output typemap for AV*

1. Creates a reference to the AV.
2. Increases the refcount of the AV (REFCNT=2: BUG!!!)
3. Mortalises the reference.

So the AV and thus its content leak. Avoid it by using the following:

SV* return_an_array() 
{
    AV* av = newAV();
    av_push( av, newSViv( 7  ) );
    av_push( av, newSviv( 42 ) );
    return newRV_noinc(av);
}
[download]

The output typemap for SV* will mortialise the reference.

av_push, as documented, takes ownership of one reference count.

Ok, yes, I saw that documentation, and was unclear whether "takes ownership of one reference count" meant what it seems to mean (receives the existing reference count, up to a max of one), or whether it owns the responsibility of managing the ref-count by adding one to it. I knew I was trying to read too much into it but still managed to get it wrong. ;)

Thanks for clarification, and thanks for pointing out the AV* bug! That one was the one I wasn't even considering to be an issue. :) Strange though; I had been passing back 150000 element arrays in a loop that had thousands of iterations (just doing some testing) and hadn't seen the memory leak. Given the quantity of data, in retrospect it should have been enough of a leak to cause some swapping to take place, which I never saw.




Dave

Ignoring the typo (newSvif), the sub as posted doesn't work -- the reference returned to Perl refers to an empty array -- and leaks memory -- the reference returned is never garbage collected.

You should (only) use sv_2mortal() on scalars that are placed on the stack. The reason is that the way the perl code gets access to these scalars is when they are copied (by assignment: my $var = someCfunc();) when the sub is called. As soon as that assignment is done, the SV placed on the stack is no longer needed or accessible, so unless you drop its refcount ("make it mortal") it will persist until the program ends. This is where your posted code is leaking, because you are not making the AV* you push onto the stack mortal. Call the function in a loop and your memory will grow and grow.

Conversely, you are making the SVs you are pushing into the (anonymous) AV mortal. As the are never copied by the call/return process -- only the ref is copied, not the referent -- by the time you try to dereference the copy of the AV* returned in order to access its contents, they have been garbage collected and the anonymous array is empty:

#! perl -slw
use strict;
use Inline C => Config => BUILD_NOISY => 1;
use Inline C => <<'END_C',  NAME => 'junk', CLEAN_AFTER_BUILD => 0;

AV* return_an_array() {
    AV* av;
    av = newAV();
    av_push( av, sv_2mortal( newSViv( 7  ) ) );
    av_push( av, sv_2mortal( newSViv( 42 ) ) );
    return av;
}

END_C

my $ref = return_an_array();
print "@{ $ref }";

__END__
C:\test>junk
Use of uninitialized value in join or string at C:\test\junk.pl line 1
+7.
Use of uninitialized value in join or string at C:\test\junk.pl line 1
+7.
[download]

Run the above code in a large loop to see the memory leak.

Fixing those two errors and the program now returns the anon array of values and stops leaking:

#! perl -slw
use strict;
use Inline C => Config => BUILD_NOISY => 1;
use Inline C => <<'END_C',  NAME => 'junk', CLEAN_AFTER_BUILD => 0;

AV* return_an_array() {
    AV* av;
    av = newAV();
    av_push( av, newSViv( 7  ) );
    av_push( av, newSViv( 42 ) );
    return sv_2mortal( av );
}

END_C

for ( 1 .. 1e8 ) {
    my $ref = return_an_array();
    print "@{ $ref }";
}

__END__
C:\test>junk
7 42
7 42
7 42
7 42
7 42
...
[download]

I did look at perlapi, and perlxstut,

For question 2, the documentation you need, and examples are in perlcall.

Ah, that really makes a lot of sense. Especially seeing your explanation about the second bug... the light bulb flicked to on for me.

My sincere thanks to both you and ikegami, for treading previously where I'm wandering now, and especially for taking the time to put me back on track.

Regarding the second question: I did read through perlcall, but a second run-through it seemed to work out better (there's a lot to remember when diving into all this at first). Here's a working (correctly, I think) example of the (resolved) question. It's a basic example within an advanced topic, I suppose. All we're doing is passing an integer from Perl into a C function, which then calls a Perl function, passes the integer to it, accepts the integer back from the Perl function, and then the C function returns it back to Perl. A round-trip:

use strict;
use warnings;

use Inline C => 'DATA';

my $pass_param = 42;

my $ret = test_perlcall( $pass_param );
print "And test_perlcall( $pass_param ) returned $ret\n";

sub return_input {
    shift;
}

__DATA__

__C__


int test_perlcall( int input ) {

    int rv;
    int count;

    dSP;  /* Declare and initialize local copy of Perl stack ptr */

    printf( "Called test_perlcall( %i )\n", input );
    
    /* Create a boundary for temps we create. */
    ENTER;
    SAVETMPS;
    
    /* Make mental note of current stack ptr (even if no params) */
    PUSHMARK(SP);
    
    /* Push a mortal param onto the stack. */
    XPUSHs(sv_2mortal(newSViv( input )));
    
    /* Sync the global stack pointer up to our local */
    PUTBACK;
    
    /* Call the Perl function; keep note of 'count' to test it. */
    /* G_SCALAR flag meaning we want a scalar returned. */
    count = call_pv( "return_input", G_SCALAR );
    
    /* Refresh local copy of stack pointer because */
    /* it may have been invalidated by call_pv()   */
    SPAGAIN;
    
    /* We never want the Perl stack to fall into an      */
    /* inconsistent state, so we must verify that we     */
    /* got exactly one RV where we expected exactly one. */
    if( count != 1 ) croak( "Well, THAT failed to work.\n" );
    
    /* Now that we know we got a RV, we can pop it off the stack. */
    rv = POPi;
    
    printf( "C called Perl function, return_input( %i )\n", rv );
    
    PUTBACK;
    
    /* Free our temps, leave the boundary we created earleir */
    FREETMPS;
    LEAVE;
    
    return rv; /* Inline::C handles this typemap automatically. */
}
[download]

The output:

Called test_perlcall( 42 )
C called Perl function, return_input( 42 )
And test_perlcall( 42 ) returned 42
[download]

I post it here just because nobody likes to hear "I got it working.", without seeing what works. I peppered the C code with notes from my read of perlcall. Maybe it will be helpful to someone else stumbling into this thread in the future.

Updated... Moved declarations to top of the C sub as suggested by BrowserUk to maintain compatibility with strict C compilers.




Dave

Did your posted code compile for you? Cos I get this lot:

junk.c
junk.xs(21) : error C2275: 'SV' : illegal use of this type as an expre
+ssion
        C:\Perl64\lib\CORE\perl.h(2400) : see declaration of 'SV'
junk.xs(21) : error C2065: 'sp' : undeclared identifier
junk.xs(21) : error C2100: illegal indirection
junk.xs(23) : error C2143: syntax error : missing ';' before 'type'
junk.xs(30) : error C2065: 'sp' : undeclared identifier
junk.xs(30) : error C2113: '-' : pointer can only be subtracted from a
+nother pointer
junk.xs(33) : error C2065: 'sp' : undeclared identifier
junk.xs(33) : warning C4047: '<' : 'SV **' differs in levels of indire
+ction from 'int'
junk.xs(33) : error C2065: 'sp' : undeclared identifier
junk.xs(33) : error C2065: 'sp' : undeclared identifier
junk.xs(33) : warning C4047: 'function' : 'SV **' differs in levels of
+ indirection from 'int'
junk.xs(33) : warning C4024: 'Perl_stack_grow' : different types for f
+ormal and actual parameter 2
junk.xs(33) : error C2065: 'sp' : undeclared identifier
junk.xs(33) : warning C4047: 'function' : 'SV **' differs in levels of
+ indirection from 'int'
junk.xs(33) : warning C4024: 'Perl_stack_grow' : different types for f
+ormal and actual parameter 3
junk.xs(33) : warning C4047: '=' : 'int' differs in levels of indirect
+ion from 'SV **'
junk.xs(33) : error C2065: 'sp' : undeclared identifier
junk.xs(33) : error C2100: illegal indirection
junk.xs(33) : warning C4047: '=' : 'int' differs in levels of indirect
+ion from 'SV *'
junk.xs(33) : error C2106: '=' : left operand must be l-value
junk.xs(36) : error C2065: 'sp' : undeclared identifier
junk.xs(36) : warning C4047: '=' : 'SV **' differs in levels of indire
+ction from 'int'
junk.xs(40) : error C2065: 'count' : undeclared identifier
junk.xs(44) : error C2065: 'sp' : undeclared identifier
junk.xs(44) : warning C4047: '=' : 'int' differs in levels of indirect
+ion from 'SV **'
junk.xs(49) : error C2065: 'count' : undeclared identifier
junk.xs(52) : error C2065: 'sp' : undeclared identifier
junk.xs(52) : error C2100: illegal indirection
junk.xs(52) : warning C4047: '=' : 'SV *' differs in levels of indirec
+tion from 'int'
junk.xs(52) : warning C4244: '=' : conversion from 'IV' to 'int', poss
+ible loss of data
junk.xs(56) : error C2065: 'sp' : undeclared identifier
junk.xs(56) : warning C4047: '=' : 'SV **' differs in levels of indire
+ction from 'int'
NMAKE : fatal error U1077: '"c:\Program Files (x86)\Microsoft Visual S
+tudio 9.0\VC\Bin\amd64\cl.EXE"' : return code '0x2'
Stop.
[download]

Maybe your compiler allows you to take liberties with the C rule that declarations must come before executable statements?

Anyway, I've always felt that placing dSP; and similar XS declarations at the very top of the function is probably best. I also had to move int rv; above the printf. That fixed up all the errors and warnings except for:

    rv = POPi;
junk.xs(51) : warning C4244: '=' : conversion from 'IV' to 'int', poss
+ible loss of data
[download]

which is because I'm using 64-bit.

It runs clean showing no signs of memory leaks.

---

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

You should (only) use sv_2mortal() on scalars that are placed on the stack.

I disagree. Some people, including CPAN authors, use sv_2mortal() and sv_grow() as a direct replacement for alloca in XS. I prefer the real alloca.

# define alloca( size )    sv_grow( sv_newmortal(), size )

My assertion is meant to signify that you should only use sv_2mortal() on SVs returned to the user via the stack, and not on SVs returned to the user embedded within other data structures (hashes & arrays), references to which are returned to the user via the stack.

In that context, I believe the advice is correct.

I never heard or read of this 'substitute for alloca' use to which you allude, but I would be very interested if you could expand a little or point to some documentation regarding that?

---

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

use warnings;
use strict;

use Inline C => Config =>
    BUILD_NOISY => 1;

use Inline C =><<'EOC';

void call_Adder(int a, int b) {
     dSP;
     int count;

     ENTER;
     SAVETMPS;

     PUSHMARK(SP);
     XPUSHs(sv_2mortal(newSViv(a)));
     XPUSHs(sv_2mortal(newSViv(b)));
     PUTBACK;

     count = call_pv("Adder", G_SCALAR);

     SPAGAIN;

     if (count != 1)
        croak("Big trouble\n");

     printf ("The sum of %d and %d is %d\n", a, b, POPi);

     PUTBACK;
     FREETMPS;
     LEAVE;
}

EOC

call_Adder(17, 16);

sub Adder {
    my($a, $b) = @_;
    $a + $b;
}
[download]

I gather you've already discovered that the Inline::C stack macros are, by themselves, usually incapable of dealing with callbacks.

Yes... that's sort of how I got here. I was imagining a situation where I would bury a Perl callback into a C function, either as an iterator or as an actor. Of course it sort of defeats the purpose of inlining C if you're suddenly burying a Perl sub-based iterator into it. But it seemed like higher-order-kinky fun. However a callback as an action could be useful.




Dave