[perl #67838] lvalue substr leaks memory? (was: lvalue substr keeping lexical alive)
Bram via RT
> The program foo.pl below prints
>
> SCALAR(0x874b2c0)
>
> where I hoped it would print undef, ie. the lexical scalar $str would
> be
> garbage collected on going out of scope. undef is what I get without
> the lvalue substr() assignment, or with a 4-arg substr call.
>
> Some digging around suggests the scratchpad array in foo() holds a
> reference to the $str scalar if an lvalue substr is used this way. I
> don't know if that's a bug, a feature, or an unavoidable side-effect
> of
> the implementation.
>
> If a feature or unavoidable then take this report as a wish for
> something in the docs on the subject, as even perlguts seems very
thin
> on anything about lvalue scalars.
>
>
lvalue substr seems to leak...
Test case:
#!/usr/bin/perl -l
use strict;
use warnings;
my $str = 'Hello World';
print "before: " . Internals::SvREFCNT($str);
substr($str,0,1) = 'x';
print "after (1): " . Internals::SvREFCNT($str);
substr($str,0,1) = 'x';
print "after (2): " . Internals::SvREFCNT($str);
for (3, 4) {
print "before ($_) (loop): " . Internals::SvREFCNT($str);
substr($str,0,1) = 'x';
print "after ($_) (loop): " . Internals::SvREFCNT($str);
}
__END__
Output (with blead):
before: 1
after (1): 2
after (2): 3
before (3) (loop): 3
after (3) (loop): 4
before (4) (loop): 4
after (4) (loop): 4
(perl-5.6.0 (tested with Devel::Peek), perl-5.8.0 and everything in
between behaves the same as blead)
I'm guessing this is due to:
LvTYPE(TARG) = 'x';
if (LvTARG(TARG) != sv) {
if (LvTARG(TARG))
SvREFCNT_dec(LvTARG(TARG));
LvTARG(TARG) = SvREFCNT_inc_simple(sv);
}
in pp_substr.
Looking at the blame log this seems to be added in:
http://perl5.git.perl.org/perl.git/blobdiff/
15e73149a8419f18d739227762eab108524cec56..ae389c8a29b487f4434c465442dfb611507a4a38:/
pp.c
[core language changes]
Title: "5.004_04m5t1: Fix dangling references in LVs", "Fix dangling
references in LVs"
Msg-ID: <1998040105...@Orb.Nashua.NH.US>,
<1998042216...@perl.org>
Files: embed.h keywords.h opcode.h perl.h proto.h doop.c global.sym
mg.c
pp.c sv.c
Title: "Fix SvGMAGIC typo in change 904"
Files: doop.c
p4raw-id: //depot/maint-5.004/perl@906
Unfortunally no tests are added in that change :(
This change also indicates that the same happens for vec() and pos():
#!/usr/bin/perl -l
use strict;
use warnings;
my $str = 'Hello World';
print "before: " . Internals::SvREFCNT($str);
vec($str,0,1) = 0;
print "after (1): " . Internals::SvREFCNT($str);
vec($str,0,1) = 0;
print "after (2): " . Internals::SvREFCNT($str);
for (3, 4) {
print "before ($_) (loop): " . Internals::SvREFCNT($str);
vec($str,0,1) = 0;
print "after ($_) (loop): " . Internals::SvREFCNT($str);
}
__END__
before: 1
after (1): 2
after (2): 3
before (3) (loop): 3
after (3) (loop): 4
before (4) (loop): 4
after (4) (loop): 4
#!/usr/bin/perl -l
use strict;
use warnings;
my $str = 'Hello World';
print "before: " . Internals::SvREFCNT($str);
pos($str) = 0;
print "after (1): " . Internals::SvREFCNT($str);
pos($str) = 0;
print "after (2): " . Internals::SvREFCNT($str);
for (3, 4) {
print "before ($_) (loop): " . Internals::SvREFCNT($str);
pos($str) = 0;
print "after ($_) (loop): " . Internals::SvREFCNT($str);
}
__END__
before: 1
after (1): 2
after (2): 3
before (3) (loop): 3
after (3) (loop): 4
before (4) (loop): 4
after (4) (loop): 4
Anyone remembers the reason why this is/was nessesary?
(I haven't tested yet what happens when the refcount isn't increased)
Best regards,
Bram
Kevin Ryde
# Please include the string: [perl #67838]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=67838 >
The program foo.pl below prints
SCALAR(0x874b2c0)
where I hoped it would print undef, ie. the lexical scalar $str would be
garbage collected on going out of scope. undef is what I get without
the lvalue substr() assignment, or with a 4-arg substr call.
Some digging around suggests the scratchpad array in foo() holds a
reference to the $str scalar if an lvalue substr is used this way. I
don't know if that's a bug, a feature, or an unavoidable side-effect of
the implementation.
If a feature or unavoidable then take this report as a wish for
something in the docs on the subject, as even perlguts seems very thin
on anything about lvalue scalars.
For what it's worth I struck this in DBI.pm where it does a substr
modify like this and the resulting scalar looks like a memory leak to
Test::Weaken. I think it really is a leak, but only a temporary one
since the next call to foo() or whatever function seems to clear it out.
Of course if a string is very big it'd be bad to have it hanging around
in core beyond what you normally expect to be its scope.
-----------------------------------------------------------------
---
Flags:
category=core
severity=medium
---
Site configuration information for perl 5.10.0:
Configured by Debian Project at Thu Jul 9 09:30:18 UTC 2009.
Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
Platform:
osname=linux, osvers=2.6.30.1-dsa-ia32, archname=i486-linux-gnu-thread-multi
uname='linux murphy 2.6.30.1-dsa-ia32 #1 smp fri jul 3 12:55:10 cest 2009 i686 gnulinux '
config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.0 -Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2 -g',
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
ccversion='', gccversion='4.3.3', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='cc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib /usr/lib64
libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
perllibs=-ldl -lm -lpthread -lc -lcrypt
libc=/lib/libc-2.9.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
gnulibc_version='2.9'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'
Eric Brine
> # New Ticket Created by Kevin Ryde
> # Please include the string: [perl #67838]
> # in the subject line of all future correspondence about this issue.
> # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=67838 >
>
What's the impact of the bug?
substr, pos and vec operate on strings. Delaying the freeing of strings has
next to no impact. Problems can occur if the scalar containing the string is
then repurposed (e.g. to hold an object with a destructor), but the odds of
this occurring is probably next to nil.
keys operate on hashes. Delaying the freeing of a hash could have a
significant impact. On the other hand, lvalue keys is probably almost never
used.
What's the impact of the fix?
Small slowdown due to the creation of a new SV for every lvalue call to
these ops
Our options at this time:
- Apply the provided patch, even though it will cause returning
substr/pos/vec/keys from an lvalue sub croaks.
- Apply an adjusted patch that doesn't fix the leak when substr/pos/vec/keys
are returned from an lvalue sub.
- Don't fix until a better solution is found.
- WONTFIX
Kevin Ryde
Is that a question for me? As I said I didn't know if it was a bug, a
feature, or a side-effect. It was just it looked a bit leak-like.
> Delaying the freeing of strings has next to no impact.
If it's a big string it would use up memory for a lot longer than you'd
expect. Ie. you thought you were careful to chuck that big string, but
it gets held onto.
If the scalar is tied or has other magic it could be bad to delay its
destructor, eg. a write-back of held data or something which otherwise
end-of-scope normally handled. Sample programs below with tie and a
File::Map mmap() magic. (The mmap only holds up address space and
system resources of course, writes go through immediately.)
> - Don't fix until a better solution is found.
I wouldn't mind knowing a way to identify scalars held alive like this,
so as to excuse them from Test::Weaken or similar leak checking.
Eric Brine
> "Eric Brine via RT" <perlbug-...@perl.org> writes:
> >
> > What's the impact of the bug?
>
> Is that a question for me?
It was rhetorical. The answer followed.
> > Delaying the freeing of strings has next to no impact.
>
> If it's a big string it would use up memory for a lot longer than you'd
> expect.
Yes, but Perl already does that all over the place intentionally. For
example, lexicals aren't freed when they go out of scope. They stay
allocated (along with their string buffer) for reuse the next time that
scope is entered. If that's the extent of the problem, it's not a bug.
If the scalar is tied or has other magic it could be bad to delay its
> destructor,
I must have been tired, but I forgot magic had destructors. I may have
underestimated the impact. I definitely understated it.
> > - Don't fix until a better solution is found.
>
> I wouldn't mind knowing a way to identify scalars held alive like this,
> so as to excuse them from Test::Weaken or similar leak checking.
>
Since TARG variables are stored in the pad, you could go through the pad
looking for PVLVs that have associated variables. It may not be the perfect
answer (any maybe you can refine it by looking at the flags), but it should
be a very good heuristic.
David Nicol
> If the scalar is tied or has other magic it could be bad to delay its
>> destructor,
>
>
> I must have been tired, but I forgot magic had destructors. I may have
> underestimated the impact. I definitely understated it.
>> > - Don't fix until a better solution is found.
>>
>> I wouldn't mind knowing a way to identify scalars held alive like this,
>> so as to excuse them from Test::Weaken or similar leak checking.
>>
>
> Since TARG variables are stored in the pad, you could go through the pad
> looking for PVLVs that have associated variables. It may not be the
perfect
> answer (any maybe you can refine it by looking at the flags), but it
should
> be a very good heuristic.
1: are there situations where a RAIL object will be the subject of one of
these functions?
(resource acquisition is locking is the big design pattern that relies on
timely destruction)
2: can TARG be a weak reference using current weak reference technology?
That was mentioned earlier in this thread, and seems from a high and distant
level to be the way to go. What's wrong with that suggestion? When does TARG
hold the last reference to something, and if never, can TARG manipulation
stuff simply leave reference counts alone?
--
"In the case of an infinite collection, the question of the existence of a
choice function is problematic"
Eric Brine
> 1: are there situations where a RAIL object will be the subject of one of
> these functions?
> (resource acquisition is locking is the big design pattern that relies on
> timely destruction)
>
Yes.
I previously gave the following example which demonstrates resources being
help until global destruction (marked by "G") rather than being released
timely:
# Timely release
>perl -le"{ my $x=''; $x = bless {}; } print 'G'; DESTROY { print 'D' }"
D
G
# Resource held until global destruction
>perl -le"{ my $x=''; vec($x,0,1)=0; $x = bless {}; } print 'G'; DESTROY {
print 'D' }"
G
D
Here's an example that uses lvalue keys(%h) in the most straightforward
manner:
perl -le'
sub init {
my %h;
keys(%h) = @_;
%h = map { $_ => bless {} } @_;
return \%h;
}
DESTROY { print "D" }
{ my $h = init(qw(a b c)); }
print "G";
'
GDDD
2: can TARG be a weak reference using current weak reference technology?
>
That was mentioned earlier in this thread, and seems from a high and distant
> level to be the way to go.
>
Yes, I believe so.
What's wrong with that suggestion?
The only downside is overhead. It makes yet another variable magical (the
var passed as an arg). I can write up a patch tonight if you wish.
Should I only use weaken when necessary (lvalue subs)?
When does TARG hold the last reference to something,
>
See the reply to your first question.
can TARG manipulation stuff simply leave reference counts alone?
>
If these ops couldn't be used as the return value for lvalue subs, I believe
we could do forgo ref counting. I don't think that's a condition we can
meet.
Eric
Eric Brine
> On Tue, Nov 17, 2009 at 2:27 PM, David Nicol <david...@gmail.com> wrote:
>
>> 2: can TARG be a weak reference using current weak reference technology?
>>
> That was mentioned earlier in this thread, and seems from a high and
>> distant
>> level to be the way to go.
>>
>
> Yes, I believe so.
>
No, I was wrong. Conditions that must be met:
- A magical var (e.g. PVLV) must be returned.
- The magical var cannot be a TEMP
- The magical var must reference the arg var
- The magical var must have a counted reference to the arg var.
If the TARG is a PLVL that targets an RV that weekly references the arg var,
it violates the fourth point causing the following to fail:
my $r; { my $s = ""; $r = \substr($s, 0, 1); } $$r = 'a'; print $$r;
If the TARG is an RV that weekly references a PVLV that references the arg
var, the PVLV would be a TEMP. That violates the second causing the
following to fail:
sub :lvalue { my $s = ""; substr($s, 0, 1) }->();
Kevin Ryde
>
> lexicals aren't freed when they go out of scope. They stay
> allocated (along with their string buffer) for reuse the next time that
> scope is entered.
Ah, I didn't know that. Makes it hard to work carefully with big
strings. You'd be tempted to free big things, above some threshold, on
the relative badness of time taken to malloc a new block.
H.Merijn Brand
wrote:
$s = undef;
especially safe when $s is an object (e.g. a DBI statement handle) that
may contain big structures.
--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, OpenSuSE 10.3, 11.0, and 11.1, AIX 5.2 and 5.3.
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Eirik Berg Hanssen
> On Fri, 27 Nov 2009 11:41:19 +1100, Kevin Ryde <use...@zip.com.au>
> wrote:
>
>> "Eric Brine via RT" <perlbug-...@perl.org> writes:
>> >
>> > lexicals aren't freed when they go out of scope. They stay
>> > allocated (along with their string buffer) for reuse the next time that
>> > scope is entered.
>>
>> Ah, I didn't know that. Makes it hard to work carefully with big
>> strings. You'd be tempted to free big things, above some threshold, on
>> the relative badness of time taken to malloc a new block.
>
> $s = undef;
Did you mean undef($s), or did something change while I was not
looking? ;-)
Eirik, who doesn't use that feature often either
--
The price of success in philosophy is triviality.
-- C. Glymour.
H.Merijn Brand
<Eirik-Ber...@allverden.no> wrote:
> "H.Merijn Brand" <h.m....@xs4all.nl> writes:
>
> > On Fri, 27 Nov 2009 11:41:19 +1100, Kevin Ryde <use...@zip.com.au>
> > wrote:
> >
> >> "Eric Brine via RT" <perlbug-...@perl.org> writes:
> >> >
> >> > lexicals aren't freed when they go out of scope. They stay
> >> > allocated (along with their string buffer) for reuse the next time that
> >> > scope is entered.
> >>
> >> Ah, I didn't know that. Makes it hard to work carefully with big
> >> strings. You'd be tempted to free big things, above some threshold, on
> >> the relative badness of time taken to malloc a new block.
> >
> > $s = undef;
>
> Did you mean undef($s), or did something change while I was not
> looking? ;-)
Both is allowed, but indeed only 'undef ($x)' frees the variable. I was
not aware of the difference until I just checked.
Look at the flags:
$ perl -MDP -wle'$_="x"x10;DDump$_;$_=undef;DDump$_'
SV = PV(0x743158) at 0x782198
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x753660 "xxxxxxxxxx"\0
CUR = 10
LEN = 16
SV = PV(0x743158) at 0x782198
REFCNT = 1
FLAGS = ()
PV = 0x753660 "xxxxxxxxxx"\0
CUR = 10
LEN = 16
$ perl -MDP -wle'$_="x"x10;DDump$_;undef$_;DDump$_'
SV = PV(0x743158) at 0x782198
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x753660 "xxxxxxxxxx"\0
CUR = 10
LEN = 16
SV = PV(0x743158) at 0x782198
REFCNT = 1
FLAGS = ()
PV = 0
$
> Eirik, who doesn't use that feature often either
--
H.Merijn Brand
<Eirik-Ber...@allverden.no> wrote:
> > On Fri, Nov 27, 2009 at 8:52 AM, H.Merijn Brand <h.m....@xs4all.nl> wrote:
> >> On Fri, 27 Nov 2009 15:38:03 +0100, Eirik Berg Hanssen
> >
> >>> Did you mean undef($s), or did something change while I was not
> >>> looking? ;-)
> >>
> >> Both is allowed, but indeed only 'undef ($x)' frees the variable.
If '$s = undef' is clearly not doing what might be expected, is there
any chance in breaking code when making '$s = undef' to do the same as
'undef $s' ?
How easy would it be to `optimize' that in perl itself?
> Aside: If anything were to change, my suggestion would be the
> addition of a warning for an assignment, the right hand side of which
> is a simple literal undef (without arguments, parens or such). It is
> often a mistake and never the clearest way to write something:
>
> $x = undef; # unclear – did you really mean this?
> $x = (); # same thing, clearer – yes, I mean it
> undef $x; # not the same thing: this frees the memory
>
> @x = undef; # unclear – did you really mean this?
> @x = (undef); # same thing, clearer – yes, I mean it
> @x = (); # not the same thing: empty array
> undef @x; # not the same thing: this frees the memory
>
> %x = undef; # unclear – did you really mean this?
> %x = ('' => undef); # same thing, clearer – and no warning
> %x = (); # not the same thing: empty hash
> undef %x; # not the same thing: this frees the memory
>
> ($x, @y) = undef; # unclear – did you really mean this?
> ($x, @y) = (undef); # same thing, clearer
> ($x, @y) = (); # also the same thing, perhaps even clearer
> undef $x; undef @y; # not the same thing: this frees the memory
>
> lsub($x) = undef; # ... okay, I suppose that one is reasonably clear
> lsub($x) = (undef); # same thing, even clearer ;-)
> undef lsub($x); # not the same thing ... but yes, it works ;-)
>
>
> ... and at least the scalar case is a mistake that, apparently, even
> an experienced Perl hacker could make. ;-)
>
>
> Eirik
Kevin Ryde
>
> $s = undef;
Umm, sounds a bit like hard work if you have to catch all variables that
hold or might hold big things ... :-)