search

Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

@ISA self-loop #11868

Open p5pRT opened 12 years ago

p5pRT commented 12 years ago

Migrated from rt.perl.org#108280 (status was 'open')

Searchable as RT108280$

p5pRT commented 12 years ago

From @cpansprout

$ ./perl -Ilib -le '@​ISA = "foo"; use Scalar​::Util weaken; weaken($a=\@​ISA); undef *ISA; print $a' ARRAY(0x826ba0)

@​ISA has its own loop that prevents it from being freed.

It seems we need a way for the backreferences array to point to magic\, not just SVs.

Flags​:   category=core   severity=low

Site configuration information for perl 5.15.6​:

Configured by sprout at Sat Dec 31 10​:12​:16 PST 2011.

Summary of my perl5 (revision 5 version 15 subversion 6) configuration​:   Local Commit​: b2635083831c8935c437465bbeb03aec8b599c01   Ancestor​: 407287f90891c4292ac8268e6566164f3992e28e   Platform​:   osname=darwin\, osvers=10.5.0\, archname=darwin-thread-multi-2level   uname='darwin pint.local 10.5.0 darwin kernel version 10.5.0​: fri nov 5 23​:20​:39 pdt 2010; root​:xnu-1504.9.17~1release_i386 i386 '   config_args='-de -Dusedevel -Duseithreads -DDEBUGGING -Dmad'   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 ='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'\,   optimize='-O3 -g'\,   cppflags='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.2.1 (Apple Inc. build 5664)'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc'\, ldflags =' -fstack-protector -L/usr/local/lib'   libpth=/usr/local/lib /usr/lib   libs=-ldbm -ldl -lm -lutil -lc   perllibs=-ldl -lm -lutil -lc   libc=\, so=dylib\, useshrplib=false\, libperl=libperl.a   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=bundle\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags=' '\, lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:  

@​INC for perl 5.15.6​:   /usr/local/lib/perl5/site_perl/5.15.6/darwin-thread-multi-2level   /usr/local/lib/perl5/site_perl/5.15.6   /usr/local/lib/perl5/5.15.6/darwin-thread-multi-2level   /usr/local/lib/perl5/5.15.6   /usr/local/lib/perl5/site_perl   .

Environment for perl 5.15.6​:   DYLD_LIBRARY_PATH (unset)   HOME=/Users/sprout   LANG=en_US.UTF-8   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 11 years ago

From @cpansprout

On Sun Jan 15 12​:43​:57 2012\, sprout wrote​:

$ ./perl -Ilib -le '@​ISA = "foo"; use Scalar​::Util weaken; weaken($a=\@​ISA); undef *ISA; print $a' ARRAY(0x826ba0)

@​ISA has its own loop that prevents it from being freed.

It seems we need a way for the backreferences array to point to magic\, not just SVs.

No\, what we need is for the elementsā€™Ā magic to have no reference count on the array and for the array to have free magic that clears the magic of its elements (for efficiency only if their refcount > 1).

--

Father Chrysostomos

p5pRT commented 11 years ago

From [Unknown Contact. See original ticket]

On Sun Jan 15 12​:43​:57 2012\, sprout wrote​:

$ ./perl -Ilib -le '@​ISA = "foo"; use Scalar​::Util weaken; weaken($a=\@​ISA); undef *ISA; print $a' ARRAY(0x826ba0)

@​ISA has its own loop that prevents it from being freed.

It seems we need a way for the backreferences array to point to magic\, not just SVs.

No\, what we need is for the elementsā€™Ā magic to have no reference count on the array and for the array to have free magic that clears the magic of its elements (for efficiency only if their refcount > 1).

--

Father Chrysostomos

p5pRT commented 11 years ago

@cpansprout - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From @dur-randir

Created by @dur-randir

The following code leaks on bleadperl​:

@​G​::ISA = qw/F/; while (1) {   undef *G​::ISA;   @​G​::ISA = qw/F/; }

This is a regression - leak appeared somewhere between 5.16.3 and 5.18.0. Versions 5.10.1 - 5.16.3 are unaffected. As a side effect\, 'free' magic doesn't get called on @​ISA arrays that had at least one entry in them.

Perl Info ``` Flags: category=core severity=high Site configuration information for perl 5.21.8: Configured by dur-randir at Sun Dec 28 01:02:28 MSK 2014. Summary of my perl5 (revision 5 version 21 subversion 8) configuration: Snapshot of: f0e5c859d36afe5a36325793f8c14f71229c5ba4 Platform: osname=darwin, osvers=13.4.0, archname=darwin-thread-multi-2level uname='darwin isengard.local 13.4.0 darwin kernel version 13.4.0: sun aug 17 19:50:11 pdt 2014; root:xnu-2422.115.4~1release_x86_64 x86_64 ' config_args='-de -Dprefix=/Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg -Doptimize=-O2 -g -ggdb3 -Dusethreads -DDEBUGGING -Dusedevel -Aeval:scriptdir=/Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/bin' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include', optimize='-O2 -g -ggdb3', cppflags='-fno-common -DPERL_DARWIN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/6.0/lib /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/lib /usr/local/lib /usr/lib libs=-lgdbm -ldbm -ldl -lm -lutil -lc -lpthread perllibs=-ldl -lm -lutil -lc -lpthread libc=, so=dylib, useshrplib=false, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector' @INC for perl 5.21.8: /Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/lib/site_perl/5.21.8/darwin-thread-multi-2level /Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/lib/site_perl/5.21.8 /Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/lib/5.21.8/darwin-thread-multi-2level /Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/lib/5.21.8 . Environment for perl 5.21.8: DYLD_LIBRARY_PATH (unset) HOME=/Users/dur-randir LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/texbin PERLBREW_BASHRC_VERSION=0.69 PERLBREW_HOME=/Users/dur-randir/.perlbrew PERLBREW_MANPATH=/Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/man PERLBREW_PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-blead-thr-dbg/bin PERLBREW_PERL=perl-blead-thr-dbg PERLBREW_ROOT=/Users/dur-randir/perlbrew PERLBREW_VERSION=0.69 PERL_BADLANG (unset) SHELL=/usr/local/bin/zsh ```
p5pRT commented 9 years ago

From @jkeenan

On Wed Jan 07 15​:41​:34 2015\, randir wrote​:

This is a bug report for perl from sergey.aleynikov@​gmail.com\, generated with the help of perlbug 1.40 running under perl 5.21.8.

----------------------------------------------------------------- [Please describe your issue here]

The following code leaks on bleadperl​:

@​G​::ISA = qw/F/; while (1) { undef *G​::ISA; @​G​::ISA = qw/F/; }

This is a regression - leak appeared somewhere between 5.16.3 and 5.18.0. Versions 5.10.1 - 5.16.3 are unaffected. As a side effect\, 'free' magic doesn't get called on @​ISA arrays that had at least one entry in them.

I tried to run this program and it completely froze my system (Ubuntu Linux 14.04 LTS). I also tried to run this program through Porting/bisect.pl and got the same result.

This is a killer!

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From @ilmari

"James E Keenan via RT" \perlbug\-followup@​perl\.org writes​:

On Wed Jan 07 15​:41​:34 2015\, randir wrote​:

This is a bug report for perl from sergey.aleynikov@​gmail.com\, generated with the help of perlbug 1.40 running under perl 5.21.8.

----------------------------------------------------------------- [Please describe your issue here]

The following code leaks on bleadperl​:

@​G​::ISA = qw/F/; while (1) { undef *G​::ISA; @​G​::ISA = qw/F/; }

This is a regression - leak appeared somewhere between 5.16.3 and 5.18.0. Versions 5.10.1 - 5.16.3 are unaffected. As a side effect\, 'free' magic doesn't get called on @​ISA arrays that had at least one entry in them.

I tried to run this program and it completely froze my system (Ubuntu Linux 14.04 LTS). I also tried to run this program through Porting/bisect.pl and got the same result.

This is a killer!

Well\, it's a memory leak\, so after a while your machine will swap itself to death if you don't set a limit on the amount of memory it can consume. On my machine\, with a virtual memory ulimit of 1GB it takes about 13s before dying with "Out of memory!".

-- "A disappointingly low fraction of the human race is\, at any given time\, on fire." - Stig Sandbeck Mathisen

p5pRT commented 9 years ago

From @dur-randir

Bisect leads to

commit 986d39eeb4080ac83a841368252abaf063cc1486 Author​: Father Chrysostomos \sprout@​cpan\.org Date​: Thu Jul 12 17​:50​:51 2012 -0700

  Fix @​{*ISA} autovivification

  It was not attaching magic to the array\, preventing subsequent changes   to the array from updating isa caches.

But it only exposes problem that had already been there.

p5pRT commented 9 years ago

From @dur-randir

*G​::ISA = [] also leaks\, and that originates from

commit d851b1227a19f97a94b5fca3f346e709d37be33b Author​: Tony Cook \tony@​develop\-help\.com Date​: Thu Feb 18 20​:59​:33 2010 +1100

  rt #72866 - add magic to arrayrefs assigned to *Foo​::ISA

  The fix for rt #60220 (26d68d86) updated the isa cache when an   arrayref was assigned to some *ISA\, but didn't add the magic to the   new @​ISA to catch any further updates to it. Add the magic\, and   tests.

But the problem is not in the PERL_MAGIC_isa being cast on @​ISA - it get it's refcount correctly not incremented by sv_magicext. Above mentioned commits just fix future per-element @​ISA behaviour.

Actual problem is in the per-element PERL_MAGIC_isaelem magic. It's being cast on each element separately with mg_obj = @​ISA. So the more elements you get\, the more refcount for @​ISA grows\, creating efficient circular loop.

p5pRT commented 9 years ago

From @cpansprout

On Thu Jan 08 17​:18​:27 2015\, randir wrote​:

*G​::ISA = [] also leaks\, and that originates from

commit d851b1227a19f97a94b5fca3f346e709d37be33b Author​: Tony Cook \tony@​develop\-help\.com Date​: Thu Feb 18 20​:59​:33 2010 +1100

rt #72866 - add magic to arrayrefs assigned to *Foo​::ISA

The fix for rt #60220 (26d68d86) updated the isa cache when an arrayref was assigned to some *ISA\, but didn't add the magic to the new @​ISA to catch any further updates to it. Add the magic\, and tests.

But the problem is not in the PERL_MAGIC_isa being cast on @​ISA - it get it's refcount correctly not incremented by sv_magicext. Above mentioned commits just fix future per-element @​ISA behaviour.

Actual problem is in the per-element PERL_MAGIC_isaelem magic. It's being cast on each element separately with mg_obj = @​ISA. So the more elements you get\, the more refcount for @​ISA grows\, creating efficient circular loop.

This is a duplicate of #108280. If I knew an easy way to fix it\, I probably would have done so by now. Iā€™m merging the two tickets.

--

Father Chrysostomos

p5pRT commented 9 years ago

From @dur-randir

This is a duplicate of #108280. If I knew an easy way to fix it\, I probably would have done so by now. Iā€™m merging the two tickets.

Is the following method to fool inheritance cache known? If not\, I'll file a separate bug report​:

*F​::foo = sub {}; @​G​::ISA='F'; *G​::ISA = ['F']; G->foo; $G​::ISA[0] = 'H'; G->foo;

This code runs OK\, but the 2nd '->foo' call is expected to fail.

Besides that\, currently there's no way for an XS programmer to get note of a change in a hierarchy chain for a particular package really fast. Assume we have A->B->C\, and then B gets D as a parent\, resulting in A->B->D hierarchy. Starting from A\, the only way to note this change is to traverse the whole chain to its root\, as 'mro_isa_changed_in' and likes will change meta->pkg_gen only for B and leave its children's pkg_gen intact\, only recalculating caches.

I tried to achieve that goal with magic on @​ISA\, but PL_magic_vtables is not part of the API\, so I can't change it to set a global hook\, and setting magic individually is never fully relied upon even in core itself (all globe *ISA assignments trigger no magic\, but directly call mro_* instead)\, so it'd be too much work for a fickle effect.

Are there any concerns for not having a way to check inheritance changes in O(1) and not O(length-of-our-linearized-isa)\, or am I missing some other obvious option?

p5pRT commented 9 years ago

From @cpansprout

On Fri Jan 09 14​:19​:09 2015\, randir wrote​:

This is a duplicate of #108280. If I knew an easy way to fix it\, I probably would have done so by now. Iā€™m merging the two tickets.

Is the following method to fool inheritance cache known? If not\, I'll file a separate bug report​:

*F​::foo = sub {}; @​G​::ISA='F'; *G​::ISA = ['F']; G->foo; $G​::ISA[0] = 'H'; G->foo;

This code runs OK\, but the 2nd '->foo' call is expected to fail.

That I didnā€™t know about\, so please do file a separate report. I also find this surprising\, since I thought $G​::ISA[0] would attach magic to the element when it is fetched.

Besides that\, currently there's no way for an XS programmer to get note of a change in a hierarchy chain for a particular package really fast. Assume we have A->B->C\, and then B gets D as a parent\, resulting in A->B->D hierarchy. Starting from A\, the only way to note this change is to traverse the whole chain to its root\, as 'mro_isa_changed_in' and likes will change meta->pkg_gen only for B and leave its children's pkg_gen intact\, only recalculating caches.

I donā€™t quite understand this. Why would you start from A? If the hierarchy changes\, that would happen because @​B​::ISA changed\, and that should trigger mro_isa_changed_in on all its subclasses\, too\, including A.

--

Father Chrysostomos

p5pRT commented 9 years ago

From @dur-randir

That I didnā€™t know about\, so please do file a separate report. I also find this surprising\, since I thought $G​::ISA[0] would attach magic to the element when it is fetched.

Done as https://rt.perl.org/Public/Bug/Display.html?id=123585

Besides that\, currently there's no way for an XS programmer to get note of a change in a hierarchy chain for a particular package really fast. Assume we have A->B->C\, and then B gets D as a parent\, resulting in A->B->D hierarchy. Starting from A\, the only way to note this change is to traverse the whole chain to its root\, as 'mro_isa_changed_in' and likes will change meta->pkg_gen only for B and leave its children's pkg_gen intact\, only recalculating caches.

I donā€™t quite understand this. Why would you start from A? If the hierarchy changes\, that would happen because @​B​::ISA changed\, and that should trigger mro_isa_changed_in on all its subclasses\, too\, including A.

I'd expect the following code to to output two different pkg_gen's\, since G's grand-parent has changed\, but the output is 'G->pkg_gen = 2' for both calls​:

use Inline C => \<\<'END_C'; void dump_pkggen(SV* stashref) {   HV* stash = SvRV(stashref);   warn("%s->pkg_gen = %u\n"\, HvNAME(stash)\, HvMROMETA(stash)->pkg_gen); } END_C

@​G​::ISA=qw/H/; @​H​::ISA=qw/J/; dump_pkggen(\%G​::); @​H​::ISA=qw/K/; dump_pkggen(\%G​::);

Are my expectations wrong? The only counter that changes is mro->cache_gen\, but it changes not only for @​ISA\, but also for method addition/etc.