Re: When to (and not to) use sv_2mortal()

[BrowserUk]

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.

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
...

I did look at perlapi, and perlxstut,

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

[davido]

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. */
}

The output:

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

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.

[BrowserUk]

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.

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

which is because I'm using 64-bit.

It runs clean showing no signs of memory leaks.

[davido]

Yes, it compiles for me, but to your point I'll clean up the declarations within the post. I've been using C++ a lot more than C lately, and given the similarities in syntax between the two it starts to get easy to let one leak into the other. I learned Portuguese as a second language, and then picked up Spanish along the way a few years later. Both being romance languages, there are enough similarities that I found it necessary to concentrate on not slipping a Portuguese-specific word into my Spanish.

Thanks for taking a look at it. This is the sort of thing that if I'm going to invest the time in learning it, I want to learn it right.

I considered picking up the Extending and Embedding Perl book, but given that it came along before 5.8.8 (I think), there have been changes to XS since then that might make the book more trouble than it's worth. If anyone reading this knows otherwise let me know and I'll probably go pick up a copy somewhere. Meanwhile, POD, tinker, and more POD.

[patcat88]

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 )

[BrowserUk]

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?

[patcat88]

cpan/Win32API-File/buffers.h#l244 in perl.git

I'm talking about the above. Original author even says its intended to be a replacement for alloca. Also, someone correct me if I'm wrong, but I've read rumors in Perl's source that SVs can "steal" the strings/PVs of other SVs leaving the source SV undefined. I've seen it on defelem 'y' magic SVs that come in on the Perl Stack. The mess in sv_setsv sv.c#l4227 in perl.git does, sometimes, maybe, steal the PV from source to destination SV. I guess some programmers might be intentionally taking advantage of the fact that mortalized SVs can give up their PVs to a destination SV. For example, in an XSUB with C style data return in parameter scalars, if the C func fails, dont touch the value of the parameter Perl Stack SV, let the mortal SV disappear in the return, or in the croak. The author of Win32API::File obviously isn't doing that since the SV * is tossed leaving only the char */PV around. Remember (not to you BrowserUk), croaks in XS and dies in Perl are implemented using a Perl Stack unwinder (will call DESTROY if objs), and a C Stack unwinder (setjmp/longjmp). If you malloc a block (or new anything in C++), callback into Perl without eval protection and the CV dies, you just leaked memory. Using the sv_grow/mortal macro above would give you a croak safe C++ smart pointer in XS, using alloca would too. The magic system can also cause you to run Perl unintentionally in XS. Not sure if the magic system sets up an eval catch or not if the C func or Perl sub croaks/dies in the magic setter/getter. Not sure what else in the XS/C perl api can croak other than magic SVs and callbacks into Perl. The definition of mortal should be "the caller shall delete the SV if the caller shall not need it when the callee returns to the caller". The warnings about needing to return to Perl or ENTER/LEAVE blocks in perlguts or whereever it is still stands, AKA, never make mortal SVs in a non hard coded iteration loop (while(i<10) is good, while(i<runtimes), while(!exit), while(job = oldjob->Next) is very bad). You'll run out of memory/ "C stack overflow" (but its not a C stack overflow, its heap memory you ran out of). Can you think of any other uses of mortal system? :-)