DESTROY handler not firing until after function return. #14032

p5pRT · 2014-08-18T18:58:59Z

Migrated from rt.perl.org#122556 (status was 'resolved')

Searchable as RT122556$

p5pRT · 2014-08-18T18:58:59Z

From @FGasper

Created by @FGasper

Consider the following:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;

#When the below is commented out, $t isn't garbage-collected until
#body()'s return. When it's uncommented, $t is garbage-collected
#right here.
#
#return; #<== UNCOMMENT, AND COMPARE Carp::longmess OUTPUT.
}->();

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub, Perl is waiting until after that scope is finished to do the GC. This runs counter to expectation since the last reference to the Thingie is removed by setting $t to undef.

Moreover, this can produce errors if Thingie references a global in its DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The fact that Perl doesn't behave this way currently looks to me like a bug.

Perl Info


Flags:
    category=core
    severity=medium

Site configuration information for perl 5.14.4:

Configured by cPanel at Thu Jul 10 15:51:11 CDT 2014.

Summary of my perl5 (revision 5 version 14 subversion 4) configuration:
   
  Platform:
    osname=linux, osvers=2.6.32-358.23.2.el6.i686, archname=i386-linux-64int
    uname='linux rpmb-32-centos-6 2.6.32-358.23.2.el6.i686 #1 smp wed oct 16 17:21:31 utc 2013 i686 i686 i386 gnulinux '
    config_args='-des -Dusedevel -Darchname=i386-linux-64int -Dcc=/usr/bin/gcc -Dcpp=/usr/bin/cpp -DDEBUGGING=none -Doptimize=-Os -Dusemymalloc=n -Duseshrplib -Duselargefiles=yes -Duseposix=true -Dhint=recommended -Duseperlio=yes -Dccflags=-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -Dcppflags=-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -Dldflags=-Wl,-rpath -Wl,/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -Dprefix=/usr/local/cpanel/3rdparty/perl/514 -Dsiteprefix=/opt/cpanel/perl5/514 -Dsitebin=/opt/cpanel/perl5/514/bin -Dsitelib=/opt/cpanel/perl5/514/site_lib -Dusevendorprefix=true -Dvendorbin=/usr/local/cpanel/3rdparty/perl/514/bin -Dvendorprefix=/usr/local/cpanel/3rdparty/perl/514/lib/perl5 -Dvendorlib=/usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib -Dprivlib=/usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4 -Dman1dir=none -Dman3dir=none -Dscriptdir=/usr/local/cpanel/3rdparty/perl/514/bin -Dscriptdirexp=/usr/local/cpanel/3rdparty/perl/514/bin -Dsiteman1dir=none -Dsiteman3dir=none -Dinstallman1dir=none -Dversiononly=no -Dinstallusrbinperl=no -Dcf_by=cPanel -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=support@cpanel.net -Di_dbm=/usr/local/cpanel/3rdparty/include -Di_gdbm=/usr/local/cpanel/3rdparty/include -Di_ndbm=/usr/local/cpanel/3rdparty/include -Ud_dosuid -Uuserelocatableinc -Umad -Uusethreads -Uusemultiplicity -Uusesocks -Uuselongdouble -Ui_db -Aldflags=-L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm -Dlocincpth=/usr/local/cpanel/3rdparty/perl/514/include /usr/local/cpanel/3rdparty/include /usr/local/include  -Duse64bitint -Uuse64bitall -Acflags=-fPIC -DPIC -m32 -I/usr/local/cpanel/3rdparty/perl/514/include -I/usr/local/cpanel/3rdparty/include -Dlibpth=/usr/local/cpanel/3rdparty/perl/514/lib /usr/local/cpanel/3rdparty/lib /usr/local/lib /lib /usr/lib '
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='/usr/bin/gcc', ccflags ='-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-Os',
    cppflags='-I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -I/usr/local/cpanel/3rdparty/perl/514/include -L/usr/local/cpanel/3rdparty/perl/514/lib -I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib -DAPPLLIB_EXP="/usr/local/cpanel" -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.7 20120313 (Red Hat 4.4.7-3)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='/usr/bin/gcc', ldflags ='-Wl,-rpath -Wl,/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.12.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.12'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4/i386-linux-64int/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -Os -L/usr/local/cpanel/3rdparty/perl/514/lib -L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -L/usr/local/lib -fstack-protector'

Locally applied patches:
    cPanel patches
    cPanel INC path changes
    Disabled some unstable tests on a kvm build server
    Cherry pick of 49498ca from p5p (RT 113514)
    Remove improper use of each() in B::walksymtable (5cc8528c90)


@INC for perl 5.14.4:
    /usr/local/cpanel
    /usr/local/cpanel
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/cpanel_lib
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/514/lib/perl5/5.14.4
    /opt/cpanel/perl5/514/site_lib/i386-linux-64int
    /opt/cpanel/perl5/514/site_lib
    .


Environment for perl 5.14.4:
    HOME=/root
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/root/.opt/bin:/usr/local/cpanel/3rdparty/perl/514/bin:/usr/local/cpanel/3rdparty/bin:/usr/local/cpanel/3rdparty/node/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
    PERL5LIB=/usr/local/cpanel
    PERL_BADLANG (unset)
    SHELL=/bin/bash

p5pRT · 2014-08-21T13:32:47Z

From @iabyn

On Mon, Aug 18, 2014 at 11:59:00AM -0700, felipe@felipegasper.com wrote:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;
    \#When the below is commented out\, $t isn't garbage\-collected until
    \#body's return\. When it's uncommented\, $t is garbage\-collected
    \#right here\.
    \#
    \#return;    \#\<== UNCOMMENT\, AND COMPARE Carp&#8203;::longmess OUTPUT\.
\}\->;

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub, Perl is
waiting until after that scope is finished to do the GC. This runs
counter to expectation since the last reference to the Thingie is
removed by setting $t to undef.

Moreover, this can produce errors if Thingie references a global in its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The fact
that Perl doesn't behave this way currently looks to me like a bug.

Well, the actual behaviour is that 'undef $ref' defers decrementing the
reference on on the referent until the boundary between the end of the
current statement and the start of the next statement. This deferring is
part of a general mechanism when converting a scalar ref into something
else, so that things like '$a = $a->[0]' work.

When there is a 'return' statement (or any other statement following the
undef), the destructor is called just before executing the next statement
(e.g. the return).

When the undef is the last statement, the destructor is called
after exit from the anon sub, but *before* any further statements within
the 'body' sub are executed.

From a technical viewpoint, the exit from a sub (pp_leavesub) can't free
the current frame's worth of temporaries (of which $$t is one) because
that frame may contain one or more of the return values; e.g. if the
value of the last statement is a temporary value, then freeing temporaries
during return would free the return value. So temporaries can't be freed
until the start of the next statement in the caller.

--
No matter how many dust sheets you use, you will get paint on the carpet.

p5pRT · 2014-08-21T13:32:47Z

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

p5pRT · 2014-08-21T14:41:08Z

From @FGasper

Hm.

It sounds like this should be documented, then? It seems a pretty conspicuous exception to how one would normally think that this works.

-FG

Sent from my android device.

-----Original Message-----
From: Dave Mitchell via RT <perlbug-followup@perl.org>
To: felipe@felipegasper.com
Sent: Thu, 21 Aug 2014 8:32 AM
Subject: Re: [perl #122556] DESTROY handler not firing until after function return.

On Mon, Aug 18, 2014 at 11:59:00AM -0700, felipe@felipegasper.com wrote:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;
    \#When the below is commented out\, $t isn't garbage\-collected until
    \#body's return\. When it's uncommented\, $t is garbage\-collected
    \#right here\.
    \#
    \#return;    \#\<== UNCOMMENT\, AND COMPARE Carp&#8203;::longmess OUTPUT\.
\}\->;

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub, Perl is
waiting until after that scope is finished to do the GC. This runs
counter to expectation since the last reference to the Thingie is
removed by setting $t to undef.

Moreover, this can produce errors if Thingie references a global in its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The fact
that Perl doesn't behave this way currently looks to me like a bug.

Well, the actual behaviour is that 'undef $ref' defers decrementing the
reference on on the referent until the boundary between the end of the
current statement and the start of the next statement. This deferring is
part of a general mechanism when converting a scalar ref into something
else, so that things like '$a = $a->[0]' work.

When there is a 'return' statement (or any other statement following the
undef), the destructor is called just before executing the next statement
(e.g. the return).

When the undef is the last statement, the destructor is called
after exit from the anon sub, but *before* any further statements within
the 'body' sub are executed.

From a technical viewpoint, the exit from a sub (pp_leavesub) can't free
the current frame's worth of temporaries (of which $$t is one) because
that frame may contain one or more of the return values; e.g. if the
value of the last statement is a temporary value, then freeing temporaries
during return would free the return value. So temporaries can't be freed
until the start of the next statement in the caller.

--
No matter how many dust sheets you use, you will get paint on the carpet.

p5pRT · 2014-08-21T14:59:10Z

From @Leont

On Mon, Aug 18, 2014 at 8:59 PM, felipe@felipegasper.com <
perlbug-followup@perl.org> wrote:

Consider the following:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;
    \#When the below is commented out\, $t isn't garbage\-collected until
    \#body's return\. When it's uncommented\, $t is garbage\-collected
    \#right here\.
    \#
    \#return;    \#\<== UNCOMMENT\, AND COMPARE Carp&#8203;::longmess OUTPUT\.
\}\->;

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub, Perl is
waiting until after that scope is finished to do the GC. This runs counter
to expectation since the last reference to the Thingie is removed by
setting $t to undef.

Moreover, this can produce errors if Thingie references a global in its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The fact
that Perl doesn't behave this way currently looks to me like a bug.

I can see why this happens (it mortalized the referenced SV, instead of
destroying it immediately). The obvious fix breaks assignment under some
circumstances. I think this can be fixed, but it needs to be thought out
well.

Leon

p5pRT · 2014-08-21T15:11:38Z

From @iabyn

On Thu, Aug 21, 2014 at 04:58:17PM +0200, Leon Timmermans wrote:

I can see why this happens (it mortalized the referenced SV, instead of
destroying it immediately). The obvious fix breaks assignment under some
circumstances. I think this can be fixed, but it needs to be thought out
well.

The real fix of course is to make the stack refcounted. The sv_2mortal()
hack in sv_unref_flags() is just one of many of workarounds for this
design flaw. (A "grep -c mortal *.c" shows almost 500 lines; I wonder how
mnay of those are just compensating for unrefcounted stack.)

--
In England there is a special word which means the last sunshine
of the summer. That word is "spring".

p5pRT · 2014-08-28T05:24:22Z

From @cpansprout

On Thu Aug 21 07:59:10 2014, LeonT wrote:

On Mon, Aug 18, 2014 at 8:59 PM, felipe@felipegasper.com <
perlbug-followup@perl.org> wrote:
Consider the following:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;
    \#When the below is commented out\, $t isn't garbage\-collected until
    \#body's return\. When it's uncommented\, $t is garbage\-collected
    \#right here\.
    \#
    \#return;    \#\<== UNCOMMENT\, AND COMPARE Carp&#8203;::longmess OUTPUT\.
\}\->;

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub, Perl is
waiting until after that scope is finished to do the GC. This runs counter
to expectation since the last reference to the Thingie is removed by
setting $t to undef.

Moreover, this can produce errors if Thingie references a global in its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The fact
that Perl doesn't behave this way currently looks to me like a bug.
I can see why this happens (it mortalized the referenced SV, instead of
destroying it immediately). The obvious fix breaks assignment under some
circumstances. I think this can be fixed, but it needs to be thought out
well.

undef is distinct from assignment. We can fix this case by having pp_undef pass the SV_IMMEDIATE_UNREF flag, no? (I am testing a patch right now.) Would that cause any unforeseen problems?

--

Father Chrysostomos

p5pRT · 2014-08-28T13:23:47Z

From @cpansprout

On Wed Aug 27 22:24:22 2014, sprout wrote:

On Thu Aug 21 07:59:10 2014, LeonT wrote:

On Mon, Aug 18, 2014 at 8:59 PM, felipe@felipegasper.com <
perlbug-followup@perl.org> wrote:

Consider the following:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;

#When the below is commented out, $t isn't garbage-collected until
#body()'s return. When it's uncommented, $t is garbage-collected
#right here.
#
#return; #<== UNCOMMENT, AND COMPARE Carp::longmess OUTPUT.
}->();

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub,
Perl is
waiting until after that scope is finished to do the GC. This runs
counter
to expectation since the last reference to the Thingie is removed
by
setting $t to undef.

Moreover, this can produce errors if Thingie references a global in
its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The
fact
that Perl doesn't behave this way currently looks to me like a bug.

I can see why this happens (it mortalized the referenced SV, instead
of
destroying it immediately). The obvious fix breaks assignment under
some
circumstances. I think this can be fixed, but it needs to be thought
out
well.

undef is distinct from assignment. We can fix this case by having
pp_undef pass the SV_IMMEDIATE_UNREF flag, no? (I am testing a patch
right now.) Would that cause any unforeseen problems?

I have gone ahead and applied my patch as 4dda930, fixing ‘undef $scalar’.

--

Father Chrysostomos

p5pRT · 2014-08-28T13:23:49Z

@cpansprout - Status changed from 'open' to 'pending release'

p5pRT · 2014-09-01T09:13:57Z

From @bulk88

On Wed Aug 27 22:24:22 2014, sprout wrote:

undef is distinct from assignment. We can fix this case by having
pp_undef pass the SV_IMMEDIATE_UNREF flag, no? (I am testing a patch
right now.) Would that cause any unforeseen problems?

a perl object constructor (new()), that uses a global error var like errno or package var/C library equivelent, if the new call fails, and the undef is assigned to an existing object ref of the same class, triggers a DESTROY on the existing object, and wiping the global error var. So you dont know why, or its a totally different uninitialized (since the DESTROY was successful) error code, the new() failed. Changing when DESTROY is called can cause subtle bugs like that. The solution is DESTROY has to save and restore global state, but in a complex app its very difficult to know when DESTROYs will run and what classes they are from, and the code behind those classes, etc.

--
bulk88 ~ bulk88 at hotmail.com

p5pRT · 2014-09-09T19:37:32Z

From @FGasper

Thank you to everyone who has looked at this! :)

On 8/28/14 8:23 AM, Father Chrysostomos via RT wrote:

On Wed Aug 27 22:24:22 2014, sprout wrote:

On Thu Aug 21 07:59:10 2014, LeonT wrote:

On Mon, Aug 18, 2014 at 8:59 PM, felipe@felipegasper.com <
perlbug-followup@perl.org> wrote:

Consider the following:

==============================
#!/usr/bin/perl

package Thingie;

use Carp;

sub DESTROY { print Carp::longmess() }

package main;

use strict;
use warnings;

sub body {
sub {
my $t = bless [], 'Thingie';
undef $t;

#When the below is commented out, $t isn't garbage-collected until
#body()'s return. When it's uncommented, $t is garbage-collected
#right here.
#
#return; #<== UNCOMMENT, AND COMPARE Carp::longmess OUTPUT.
}->();

return;
}

body();

When the "undef $t" is the last statement in the anonymous sub,
Perl is
waiting until after that scope is finished to do the GC. This runs
counter
to expectation since the last reference to the Thingie is removed
by
setting $t to undef.

Moreover, this can produce errors if Thingie references a global in
its
DESTROY and the anonsub did a "local" on that global.

The proper course would seem to be to do GC within the scope. The
fact
that Perl doesn't behave this way currently looks to me like a bug.

I can see why this happens (it mortalized the referenced SV, instead
of
destroying it immediately). The obvious fix breaks assignment under
some
circumstances. I think this can be fixed, but it needs to be thought
out
well.

undef is distinct from assignment. We can fix this case by having
pp_undef pass the SV_IMMEDIATE_UNREF flag, no? (I am testing a patch
right now.) Would that cause any unforeseen problems?

I have gone ahead and applied my patch as 4dda930, fixing ‘undef $scalar’.

p5pRT · 2015-06-02T03:41:18Z

From @khwilliamson

Thanks for submitting this ticket

The issue should be resolved with the release today of Perl v5.22. If you find that the problem persists, feel free to reopen this ticket

--
Karl Williamson for the Perl 5 porters team

p5pRT · 2015-06-02T03:41:19Z

@khwilliamson - Status changed from 'pending release' to 'resolved'

p5pRT closed this Jun 2, 2015

p5pRT added Severity Medium distro-Linux type-core labels Oct 19, 2019

Perl / perl5 Public

DESTROY handler not firing until after function return. #14032

DESTROY handler not firing until after function return. #14032

p5pRT commented Aug 18, 2014

p5pRT commented Aug 18, 2014

p5pRT commented Aug 21, 2014

body();

p5pRT commented Aug 21, 2014

p5pRT commented Aug 21, 2014

body();

p5pRT commented Aug 21, 2014

body();

p5pRT commented Aug 21, 2014

p5pRT commented Aug 28, 2014

body();

p5pRT commented Aug 28, 2014

body();

p5pRT commented Aug 28, 2014

p5pRT commented Sep 1, 2014

p5pRT commented Sep 9, 2014

body();

p5pRT commented Jun 2, 2015

p5pRT commented Jun 2, 2015

Perl / perl5 Public

DESTROY handler not firing until after function return. #14032

DESTROY handler not firing until after function return. #14032

Comments

p5pRT commented Aug 18, 2014

p5pRT commented Aug 18, 2014

From @FGasper

Created by @FGasper

body();

p5pRT commented Aug 21, 2014

From @iabyn

body();

p5pRT commented Aug 21, 2014

p5pRT commented Aug 21, 2014

From @FGasper

body();

p5pRT commented Aug 21, 2014

From @Leont

body();

p5pRT commented Aug 21, 2014

From @iabyn

p5pRT commented Aug 28, 2014

From @cpansprout

body();

p5pRT commented Aug 28, 2014

From @cpansprout

body();

p5pRT commented Aug 28, 2014

p5pRT commented Sep 1, 2014

From @bulk88

p5pRT commented Sep 9, 2014

From @FGasper

body();

p5pRT commented Jun 2, 2015

From @khwilliamson

p5pRT commented Jun 2, 2015