Returning AV*/HV* from XSUBs
Marcus Holland-Moritz
xsubpp is correct. (It's not really wrong, but it's potentially evoking
memory leaks.)
lib/ExtUtils/typemap has the following lines:
OUTPUT
[...]
T_AVREF
$arg = newRV((SV*)$var);
T_HVREF
$arg = newRV((SV*)$var);
The problem with this code is that newRV() is a synonym for newRV_inc()
and thus it increments the reference count of $var. This means that the
following intuitively correct XSUB code will leak memory:
AV *
foo()
CODE:
RETVAL = newAV(); /* refcount is 1 */
/* ... */
OUTPUT:
RETVAL /* will silently increment refcount */
This sort of code can be found in lots of XS modules on CPAN, e.g. in:
ApacheBench Audio::CD Audio::RaveMP BSD::Resource BerkeleyDB
CORBA::MICO Data::Structure::Util Env::C GTop Locale::Hebrew::Calendar
MIME::Fast MP3::ID3Lib Net::LDAPapi PerlQt SearchSDK Tie::CArray
Unicode::Map Xmms::Perl
A couple of other modules have more or less elegant workarounds, like:
CLEANUP:
SvREFCNT_dec(RETVAL);
or:
sv_2mortal((SV *)RETVAL);
or:
RETVAL = (AV*)sv_2mortal((SV*)newAV());
IMHO, newRV() should have been newRV_noinc(). Unfortunately, I think
this cannot be fixed, as it would break all XS modules that have been
using any of the above workarounds.
I guess the best way to fix the problem is by updating the docs. I'll
have a look...
--
It seems that more and more mathematicians are using a new, high level
language named "research student".
Marcus Holland-Moritz
> I guess the best way to fix the problem is by updating the docs. I'll
> have a look...
...but not very soon, unfortunately. The harddisk of my notebook that I'm
using for just about everything crashed only a few hours after that mail.
New disk is installed, I just have to get everything to work again.
--
Malek's Law:
Any simple idea will be worded in the most complicated way.
Marcus Holland-Moritz
> I'm wondering if the code for returning AV*'s and HV*'s generated by
> xsubpp is correct. (It's not really wrong, but it's potentially evoking
> memory leaks.)
>
> [...]
>
> This sort of code can be found in lots of XS modules on CPAN, e.g. in:
>
> ApacheBench Audio::CD Audio::RaveMP BSD::Resource BerkeleyDB
> CORBA::MICO Data::Structure::Util Env::C GTop Locale::Hebrew::Calendar
> MIME::Fast MP3::ID3Lib Net::LDAPapi PerlQt SearchSDK Tie::CArray
> Unicode::Map Xmms::Perl
>
> IMHO, newRV() should have been newRV_noinc(). Unfortunately, I think
> this cannot be fixed, as it would break all XS modules that have been
> using any of the above workarounds.
>
> I guess the best way to fix the problem is by updating the docs. I'll
> have a look...
Done as change #22330, which also fixes a leak in POSIX::localconv
caused by this behaviour.
Marcus
--
BOFH Excuse #266:
All of the packets are empty.