Mailing List Archive

memory leak
I have a memory leak in an xsub, and I have a small example that
illustrates my problem. I just can't figure out what I'm doing wrong.
I would have thought that the following two functions (one in C, the
other in Perl) would be exactly equivalent. But if I run the C
version in a big loop it uses increasingly more memory, whereas the
Perl version stays constant. Can someone explain my error? Thanks.

- Rob

sub LEAK::new
{
my($pkg, $s) = @_;
my($x) = 'x' x $s;
return bless( \$x, $pkg );
}


XS(XS_LEAK_new)
{
dXSARGS;
char * pkg = SvPV(ST(0),na);
int n = SvIV(ST(1));
char * buf;

Newz( 2001, buf, n+1, char ); /* all set to '\0' */
memset( buf, 'x', n);
ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );

sv_bless( (SV*)ST(0), gv_stashpv(pkg, 1) );

sv_2mortal( ST(0) );
XSRETURN(1);
}
Re: memory leak [ In reply to ]
> From: Nick.Ing-Simmons@tiuk.ti.com
>
> In <199509042159.AA05351@sait407.morgan.com>
> On Mon, 4 Sep 1995 17:59:05 -0400
> Rob Torop <robt@morgan.com> writes:
> >I have a memory leak in an xsub, and I have a small example that
> >illustrates my problem. I just can't figure out what I'm doing wrong.
> >I would have thought that the following two functions (one in C, the
> >other in Perl) would be exactly equivalent. But if I run the C
> >version in a big loop it uses increasingly more memory, whereas the
> >Perl version stays constant. Can someone explain my error? Thanks.
> >
> >- Rob
> >
> >XS(XS_LEAK_new)
> >{
> > dXSARGS;
> > char * pkg = SvPV(ST(0),na);
> > int n = SvIV(ST(1));
> > char * buf;
> >
> > Newz( 2001, buf, n+1, char ); /* all set to '\0' */
> > memset( buf, 'x', n);
> > ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );
> >
> > sv_bless( (SV*)ST(0), gv_stashpv(pkg, 1) );
> >
> > sv_2mortal( ST(0) );
> > XSRETURN(1);
> >}
>
> Newz malloc's a buffer but it is never free'd.
> newSVpv takes a copy.
>
Yeap, try something like this ...
change:

Newz( 2001, buf, n+1, char ); /* all set to '\0' */
memset( buf, 'x', n);
ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );

to:
sv = sv_2mortal(newSV(n+1)); /* +1 also ensures buffer is created */
buf = SvPVX(sv); /* or: foo = (struct foo_t*)SvPVX(sv) */
memset(buf, 'x', n);
buf[n] = '\0';
ST(0) = newRV(sv);

Tim.

p.s. I think the sv_2mortal on the sv prior to the newRV(sv) is misleading
and inefficient. I'd like to see a newRV_noinc(sv) macro for this kind of
situation.
Re: memory leak [ In reply to ]
In <199509042159.AA05351@sait407.morgan.com>
On Mon, 4 Sep 1995 17:59:05 -0400
Rob Torop <robt@morgan.com> writes:
>I have a memory leak in an xsub, and I have a small example that
>illustrates my problem. I just can't figure out what I'm doing wrong.
>I would have thought that the following two functions (one in C, the
>other in Perl) would be exactly equivalent. But if I run the C
>version in a big loop it uses increasingly more memory, whereas the
>Perl version stays constant. Can someone explain my error? Thanks.
>
>- Rob
>
>sub LEAK::new
>{
> my($pkg, $s) = @_;
> my($x) = 'x' x $s;
> return bless( \$x, $pkg );
>}
>
>
>XS(XS_LEAK_new)
>{
> dXSARGS;
> char * pkg = SvPV(ST(0),na);
> int n = SvIV(ST(1));
> char * buf;
>
> Newz( 2001, buf, n+1, char ); /* all set to '\0' */
> memset( buf, 'x', n);
> ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );
>
> sv_bless( (SV*)ST(0), gv_stashpv(pkg, 1) );
>
> sv_2mortal( ST(0) );
> XSRETURN(1);
>}

Newz malloc's a buffer but it is never free'd.
newSVpv takes a copy.
Re: memory leak [ In reply to ]
: newSVpv takes a copy.

The sv_usepvn() function will grab the malloced string for its own use.

Larry
Re: memory leak [ In reply to ]
Tim Bunce writes:
> change:
>
> Newz( 2001, buf, n+1, char ); /* all set to '\0' */
> memset( buf, 'x', n);
> ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );
>
> to:
> sv = sv_2mortal(newSV(n+1)); /* +1 also ensures buffer is created */
> buf = SvPVX(sv); /* or: foo = (struct foo_t*)SvPVX(sv) */
> memset(buf, 'x', n);
> buf[n] = '\0';
> ST(0) = newRV(sv);
>
> Tim.
>
> p.s. I think the sv_2mortal on the sv prior to the newRV(sv) is misleading
> and inefficient. I'd like to see a newRV_noinc(sv) macro for this kind of
> situation.
>

I do not see why one should postpone decrementing refcount on sv. On
the other hand, one need to do sv_2mortal on ST(0).

sv = sv_newSV(n+1); /* +1 also ensures buffer is created */
buf = SvPVX(sv); /* or: foo = (struct foo_t*)SvPVX(sv) */
memset(buf, 'x', n);
buf[n] = '\0';
ST(0) = sv_2mortal(newRV(sv));
Sv_REFCNT_dec(sv);

I did not try it too ;-).

Ilya
Re: memory leak [ In reply to ]
> From: Ilya Zakharevich <ilya@math.ohio-state.edu>
>
> Tim Bunce writes:
> >
> > ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );
> >
> > p.s. I think the sv_2mortal on the sv prior to the newRV(sv) is misleading
> > and inefficient. I'd like to see a newRV_noinc(sv) macro for this kind of
> > situation.
>
> I do not see why one should postpone decrementing refcount on sv.

There isn't one. I guess I'm just fond of 'functional' style code
(foo(bar(baz()))) where possible. A newRV_noinc(sv) seems natural to me.

The shortest alternative to:

ST(0) = newRV( sv_2mortal(newSVpv( buf, n )) );
is
ST(0) = newRV( newSVpv( buf, n ) );
Sv_REFCNT_dec(SvRV(ST(0)));

which seems less than ideal. Using newRV_noinc() would give us just:

ST(0) = newRV_noinc( newSVpv( buf, n ) );

> On the other hand, one need to do sv_2mortal on ST(0).
>
True.

> Ilya
>
Tim.