Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 555 forks source link

Freeing variables referred to by coderefs #14783

Closed p5pRT closed 9 years ago

p5pRT commented 9 years ago

Migrated from rt.perl.org#125530 (status was 'rejected')

Searchable as RT125530$

p5pRT commented 9 years ago

From dp13@sanger.ac.uk

This is a bug report for perl from dp13@​sanger.ac.uk\, generated with the help of perlbug 1.39 running under perl 5.14.4.

The attached script contains a variable "$leak". If it has a false value\, an operation is performed in such a way that memory remains constant (amount varies per system​: below 1MB on an Ubuntu perl); with a true value\, it behaves as if there is a memory leak (exceeds 10MB on the same perl).

I have reproduced this with v5.14.4 built for cygwin-thread-multi; perl 5.14 on Ubuntu; 5.22.0 on OSX.

There are no cyclical references.

My guess is that the behaviour described by perlsub​:"Persistent variables with closures"\, is more pervasive than the preservation of variables referenced inside named functions (which remain accessible even when the variable referenced is not)\, which is how it is documented and demonstrated\, and that in fact this behaviour applies to all coderefs even after they fall out of scope - which to me seems a substantially different case where I would expect the behaviour to be different.

In the example given in the documentation the variable referenced in the function clearly should not be garbage collected because a named function is never destroyed and always remains usable therefore indirect access to that variable (through the coderef) is always required.

However\, in the code attached (and the following hypothetical example)\, I would expect values used by a coderef to be garbage collected when the coderef falls out of scope (because they are no longer accessible directly or indirectly). I had assumed that part of the benefit of creating lexically scoped coderefs as opposed to named functions was the ability for them to refer to lexically scoped variables without needing to be passed them as arguments.

  # Hypothetical Example​:   {   my $secret_val = 0;   my $gimme_another = sub {   return ++$secret_val;   };   }

  # At this point $gimme_another is no longer referenced by anything in scope; it should be garbage collected and therefore so too should $secret_val.   # I do not know how to confirm in this smallest case that $secret_val is indeed freed; I suspect that it is not.

Is it currently intended/accepted behaviour of current perls that in the above case\, "$secret_val" is not freed?

- If so\, I would like to register a feature request that in the above case "$secret_val" is freed once "$gimme_another" is garbage collected. If that's not possible (or until it is)\, could perlsub be amended to make the accepted behaviour clear (happy to write a patch if that's useful)?

- Otherwise\, I have a script that somehow breaks expectations and I would like to report this as a bug.

Thanks\,

Daniel


Flags​:   category=core   severity=medium


Site configuration information for perl 5.14.4​:

Configured by Yaakov at Mon Mar 11 18​:16​:36 CDT 2013.

Summary of my perl5 (revision 5 version 14 subversion 4) configuration​:  
  Platform​:   osname=cygwin\, osvers=1.7.18(0.26353)\, archname=cygwin-thread-multi   uname='cygwin_nt-6.1 yaakov04 1.7.18(0.26353) 2013-03-07 19​:25 x86_64 cygwin '   config_args='-d -e -Dprefix=/usr -Dmksymlinks -Dusethreads -Darchname=x86_64-cygwin-threads -Dlibperl=cygperl5_14.dll -Dcc=gcc -Dld=g++'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=define\, usemultiplicity=define   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=define\, use64bitall=define\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='gcc'\, ccflags ='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -fno-strict-aliasing -pipe -fstack-protector'\,   optimize='-O3'\,   cppflags='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -fno-strict-aliasing -pipe -fstack-protector'   ccversion=''\, gccversion='4.8.0 20130307 (experimental)'\, gccosandvers=''   intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='g++'\, ldflags =' -Wl\,--enable-auto-import -Wl\,--export-all-symbols -Wl\,--enable-auto-image-base -fstack-protector'   libpth=/usr/lib /lib   libs=-lgdbm -ldb -ldl -lcrypt -lgdbm_compat   perllibs=-ldl -lcrypt   libc=/usr/lib/libc.a\, so=dll\, useshrplib=true\, libperl=cygperl5_14.dll   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=dll\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags=' '\, lddlflags=' --shared -Wl\,--enable-auto-import -Wl\,--export-all-symbols -Wl\,--enable-auto-image-base -fstack-protector'

Locally applied patches​:   Bug#55162 File​::Spec​::case_tolerant performance   CYG07 $vendorarch/auto/.rebase   CYG15 static Win32CORE   CYG17 cyg-1.7 paths-utf8   0c612ce82 Fix building static extensions on cygwin\, -UUSEIMPORTLIB   1bac5ecc1 Fix 64-bit threading sv.c​: S_anonymise_cv_maybe   Cygwin​::sync_winenv added


@​INC for perl 5.14.4​:   /home/dp13/perl5/lib/perl5/cygwin-thread-multi   /home/dp13/perl5/lib/perl5   /usr/lib/perl5/site_perl/5.14/x86_64-cygwin-threads   /usr/lib/perl5/site_perl/5.14   /usr/lib/perl5/vendor_perl/5.14/x86_64-cygwin-threads   /usr/lib/perl5/vendor_perl/5.14   /usr/lib/perl5/5.14/x86_64-cygwin-threads   /usr/lib/perl5/5.14   .


Environment for perl 5.14.4​:   HOME=/home/dp13   LANG=en_US.UTF-8   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/home/dp13/perl5/bin​:/usr/local/bin​:/usr/bin​:/cygdrive/c/WINDOWS/system32​:/cygdrive/c/WINDOWS​:/cygdrive/c/WINDOWS/System32/Wbem​:/cygdrive/c/WINDOWS/System32/WindowsPowerShell/v1.0​:/cygdrive/c/WINDOWS/CCM​:/cygdrive/c/WINDOWS/CCM​:/cygdrive/c/Program Files (x86)/Toshiba/Bluetooth Toshiba Stack/sys​:/cygdrive/c/Program Files (x86)/Toshiba/Bluetooth Toshiba Stack/sys/x64​:/cygdrive/c/Strawberry/c/bin​:/cygdrive/c/Strawberry/perl/site/bin​:/cygdrive/c/Strawberry/perl/bin​:/cygdrive/c/HashiCorp/Vagrant/bin​:/cygdrive/c/Users/dp13/AppData/Local/atom/bin​:/usr/lib/lapack   PERL5LIB=/home/dp13/perl5/lib/perl5   PERL_BADLANG (unset)   PERL_LOCAL_LIB_ROOT=/home/dp13/perl5   PERL_MB_OPT=--install_base "/home/dp13/perl5"   PERL_MM_OPT=INSTALL_BASE=/home/dp13/perl5   SHELL=/bin/bash

-- The Wellcome Trust Sanger Institute is operated by Genome Research Limited\, a charity registered in England with number 1021457 and a company registered in England with number 2742969\, whose registered office is 215 Euston Road\, London\, NW1 2BE.

p5pRT commented 9 years ago

From dp13@sanger.ac.uk

leak-final.pl

p5pRT commented 9 years ago

From zefram@fysh.org

Daniel Perrett wrote​:

The attached script contains a variable "$leak". If it has a false value\, an operation is performed in such a way that memory remains constant (amount varies per system​: below 1MB on an Ubuntu perl); with a true value\, it behaves as if there is a memory leak (exceeds 10MB on the same perl).

Actually both versions leak. With $leak false it just leaks *less*. Add system("ps u $$") inside the loop and watch VSZ gradually increasing.

There are no cyclical references.

There is a cyclical reference​: The $pathfinder sub is closed over the $pathfinder variable which contains a reference to the sub.

-zefram

p5pRT commented 9 years ago

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

p5pRT commented 9 years ago

From dp13@sanger.ac.uk

Ah\, I'm a fool!

Presumably\, creating a weak reference to do the recursion should work ... and indeed it does appear to do the trick.

Thankyou very much\, and apologies for the bug report.

Daniel

-----Original Message----- From​: Zefram via RT [mailto​:perlbug-followup@​perl.org] Sent​: 02 July 2015 14​:13 To​: Daniel Perrett Subject​: Re​: [perl #125530] Freeing variables referred to by coderefs

Daniel Perrett wrote​:

The attached script contains a variable "$leak". If it has a false value\, an operation is performed in such a way that memory remains constant (amount varies per system​: below 1MB on an Ubuntu perl); with a true value\, it behaves as if there is a memory leak (exceeds 10MB on the same perl).

Actually both versions leak. With $leak false it just leaks *less*. Add system("ps u $$") inside the loop and watch VSZ gradually increasing.

There are no cyclical references.

There is a cyclical reference​: The $pathfinder sub is closed over the $pathfinder variable which contains a reference to the sub.

-zefram

p5pRT commented 9 years ago

From @jkeenan

What was at first thought to be a bug has been explained\, apparently to the satisfaction of the OP. Hence\, nothing more to be done; closing ticket.

Thank you very much. -- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 9 years ago

@jkeenan - Status changed from 'open' to 'rejected'

p5pRT commented 9 years ago

From @rjbs

* Daniel Perrett \dp13@​sanger\.ac\.uk [2015-07-02T09​:45​:08]

Presumably\, creating a weak reference to do the recursion should work ... and indeed it does appear to do the trick.

Bonus advice​: as of perl 5.16.0\, you can use __SUB__ to refer to the current subroutine without needing a weak reference to avoid a memory cycle.

  my $sub = sub {   ...   __SUB__->();   };

You will need to "use v5.16.0" or "use feature 'current_sub'"

-- rjbs