Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Objects destroyed in the wrong order during global destruction #7628

Closed
p5pRT opened this issue Nov 30, 2004 · 44 comments
Closed

Objects destroyed in the wrong order during global destruction #7628

p5pRT opened this issue Nov 30, 2004 · 44 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 30, 2004

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

Searchable as RT32714$

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2001

From francois@fdesar.net

Hello, all of you.

It's been a long time since I last posted, but
I came across a Perl weirdness while trying to
build a new XS based module.

As it shows both on 5.6.0 and 5.6.1, I prefer to
follow it to perlbug.

I finally got a minimalistic demonstartion of the
problem, which is closely related to final global
destructions.

First of all, the stubs have been generated using

h2xs -cfn Foo

Then you can use this file as a patch file against
the 'Foo' directory.

After the usual 'perl Makefile.PL' and 'make', here
is the output (including stderr) of "make test" :

--------------------------------------------
[ blah blah ]
Foo​::DESTROY >>BARPtr=SCALAR(0x80f5a30)<<
Use of uninitialized value in print at blib/lib/Foo.pm line 51 during
global destruction.
Foo​::DESTROY >><<
--------------------------------------------

The first 'Foo​::DESTROY' is ok. It destroys the $y variable,
and the second one, which should be passed $x is wrong.

- Using 'my' variables doesn't show the problem.
- Explicitly 'undefing' the variables is ok.
- Adding '1;' after each variable creation hides the problem.
- Adding a third variable generate one more error
- ...

Cheers,

François

Start of patch -----------------------------

Inline Patch
--- Foo.xs.orig	Wed Apr 11 19:13:58 2001
+++ Foo.xs	Wed Apr 11 19:00:34 2001
@@ -2,6 +2,16 @@
 #include "perl.h"
 #include "XSUB.h"
 
+typedef int BAR;
+
+static BAR bar;
 
 MODULE = Foo		PACKAGE = Foo		
 
+BAR *
+_create(init)
+    char * init
+    CODE:
+        RETVAL=&bar;
+    OUTPUT:
+        RETVAL
End of patch -------------------------------

Start of patch -----------------------------

Inline Patch
--- Foo.pm.orig	Wed Apr 11 19:13:39 2001
+++ Foo.pm	Wed Apr 11 20:56:16 2001
@@ -34,6 +34,23 @@
 
 # Autoload methods go after =cut, and are processed by the autosplit
program.

+sub new {
+ my($class) = shift;
+ my($self) = { val => undef };
+
+ $self->{val} = _create('a');
+
+ $class = ref($class) || $class;
+
+ bless($self, $class);
+}
+
+sub DESTROY {
+ my($self) = shift;
+
+ print "Foo​::DESTROY >>", $self->{val}, "<<\n";
+}
+
1;
__END__
# Below is stub documentation for your module. You better edit it!
End of patch -------------------------------

Start of patch -----------------------------

Inline Patch
--- test.pl.orig	Wed Apr 11 19:19:18 2001
+++ test.pl	Wed Apr 11 18:18:46 2001
@@ -15,3 +15,5 @@
 # Insert your test code below, the Test module is use()ed here so read
 # its man page ( perldoc Test ) for help writing this test script.
 
+$x = new Foo();
+$y = new Foo();
End of patch -------------------------------
Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl v5.6.1:

Configured by fdesar at Wed Apr 11 13:52:45 CEST 2001.

Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration:
  Platform:
    osname=linux, osvers=2.2.17, archname=i686-linux
    uname='linux fdesar 2.2.17 #1 sun jun 25 09:24:41 est 2000 i686
unknown '
    config_args='-d'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef useithreads=undef
usemultiplicity=undef
    useperlio=undef d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -I/usr/local/include'
    ccversion='', gccversion='2.95.2 20000220 (Debian GNU/Linux)',
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, usemymalloc=n, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lndbm -ldb -ldl -lm -lc -lposix -lcrypt -lutil
    perllibs=-lnsl -ldl -lm -lc -lposix -lcrypt -lutil
    libc=/lib/libc-2.1.3.so, so=so, useshrplib=false, libperl=libperl.a
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.6.1:
    /usr/local/lib/perl5/5.6.1/i686-linux
    /usr/local/lib/perl5/5.6.1
    /usr/local/lib/perl5/site_perl/5.6.1/i686-linux
    /usr/local/lib/perl5/site_perl/5.6.1
    /usr/local/lib/perl5/site_perl
    .


Environment for perl v5.6.1:
    HOME=/home/fdesar
    LANG=C
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2001

From @timj

This is a bug report for perl from francois@​fdesar.net,
generated with the help of perlbug 1.33 running under perl v5.6.1.

-----------------------------------------------------------------
[Please enter your report here]

Hello, all of you.

It's been a long time since I last posted, but
I came across a Perl weirdness while trying to
build a new XS based module.

As it shows both on 5.6.0 and 5.6.1, I prefer to
follow it to perlbug.

I finally got a minimalistic demonstartion of the
problem, which is closely related to final global
destructions.

First of all, the stubs have been generated using

h2xs -cfn Foo

Then you can use this file as a patch file against
the 'Foo' directory.

After the usual 'perl Makefile.PL' and 'make', here
is the output (including stderr) of "make test" :

--------------------------------------------
[ blah blah ]
Foo​::DESTROY >>BARPtr=SCALAR(0x80f5a30)<<
Use of uninitialized value in print at blib/lib/Foo.pm line 51 during
global destruction.
Foo​::DESTROY >><<
--------------------------------------------

I assume you are using T_PTROBJ in your local typemap?

Can you check that you really are getting a different object each time.
You're blessing a static pointer into a perl object so each object
is attached to the same pointer. I imagine the first destroy is killing
the pointer object and the second destroy is simply killing your wrapper
object (the input arg to _create is never used). Print $self->{val}
after each object is created to see whether the objects are different.

It seems that the destruction order is​:

  Object $x
  Object $x->{val}
  Object $y

but you need to check this (I haven't got time to actual build this
module to confirm).

Tim

The first 'Foo​::DESTROY' is ok. It destroys the $y variable,
and the second one, which should be passed $x is wrong.

- Using 'my' variables doesn't show the problem.
- Explicitly 'undefing' the variables is ok.
- Adding '1;' after each variable creation hides the problem.
- Adding a third variable generate one more error
- ...

Cheers,

François

Start of patch -----------------------------
--- Foo.xs.orig Wed Apr 11 19​:13​:58 2001
+++ Foo.xs Wed Apr 11 19​:00​:34 2001
@​@​ -2,6 +2,16 @​@​
#include "perl.h"
#include "XSUB.h"

+typedef int BAR;
+
+static BAR bar;

MODULE = Foo PACKAGE = Foo

+BAR *
+_create(init)
+ char * init
+ CODE​:
+ RETVAL=&bar;
+ OUTPUT​:
+ RETVAL
End of patch -------------------------------

Start of patch -----------------------------
--- Foo.pm.orig Wed Apr 11 19​:13​:39 2001
+++ Foo.pm Wed Apr 11 20​:56​:16 2001
@​@​ -34,6 +34,23 @​@​

# Autoload methods go after =cut, and are processed by the autosplit
program.

+sub new {
+ my($class) = shift;
+ my($self) = { val => undef };
+
+ $self->{val} = _create('a');
+
+ $class = ref($class) || $class;
+
+ bless($self, $class);
+}
+
+sub DESTROY {
+ my($self) = shift;
+
+ print "Foo​::DESTROY >>", $self->{val}, "<<\n";
+}
+
1;
__END__
# Below is stub documentation for your module. You better edit it!
End of patch -------------------------------

Start of patch -----------------------------
--- test.pl.orig Wed Apr 11 19​:19​:18 2001
+++ test.pl Wed Apr 11 18​:18​:46 2001
@​@​ -15,3 +15,5 @​@​
# Insert your test code below, the Test module is use()ed here so read
# its man page ( perldoc Test ) for help writing this test script.

+$x = new Foo();
+$y = new Foo();
End of patch -------------------------------

[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags​:
category=core
severity=low
---
Site configuration information for perl v5.6.1​:

Configured by fdesar at Wed Apr 11 13​:52​:45 CEST 2001.

Summary of my perl5 (revision 5.0 version 6 subversion 1) configuration​:
Platform​:
osname=linux, osvers=2.2.17, archname=i686-linux
uname='linux fdesar 2.2.17 #1 sun jun 25 09​:24​:41 est 2000 i686
unknown '
config_args='-d'
hint=recommended, useposix=true, d_sigaction=define
usethreads=undef use5005threads=undef useithreads=undef
usemultiplicity=undef
useperlio=undef d_sfio=undef uselargefiles=define usesocks=undef
use64bitint=undef use64bitall=undef uselongdouble=undef
Compiler​:
cc='cc', ccflags ='-fno-strict-aliasing -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2',
cppflags='-fno-strict-aliasing -I/usr/local/include'
ccversion='', gccversion='2.95.2 20000220 (Debian GNU/Linux)',
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, usemymalloc=n, prototype=define
Linker and Libraries​:
ld='cc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib
libs=-lnsl -lndbm -ldb -ldl -lm -lc -lposix -lcrypt -lutil
perllibs=-lnsl -ldl -lm -lc -lposix -lcrypt -lutil
libc=/lib/libc-2.1.3.so, so=so, useshrplib=false, libperl=libperl.a
Dynamic Linking​:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:

---
@​INC for perl v5.6.1​:
/usr/local/lib/perl5/5.6.1/i686-linux
/usr/local/lib/perl5/5.6.1
/usr/local/lib/perl5/site_perl/5.6.1/i686-linux
/usr/local/lib/perl5/site_perl/5.6.1
/usr/local/lib/perl5/site_perl
.

---
Environment for perl v5.6.1​:
HOME=/home/fdesar
LANG=C
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/bin/X11​:/usr/games
PERL_BADLANG (unset)
SHELL=/bin/bash

--
Tim Jenness
JAC software
http​://www.jach.hawaii.edu/~timj

@p5pRT
Copy link
Author

p5pRT commented Apr 12, 2001

From [Unknown Contact. See original ticket]

Oooops yes. The typemap is

BAR * T_PTROBJ

Can you check that you really are getting a different object each time.
You're blessing a static pointer into a perl object so each object
is attached to the same pointer. I imagine the first destroy is killing
the pointer object and the second destroy is simply killing your wrapper
object (the input arg to _create is never used). Print $self->{val}
after each object is created to see whether the objects are different.

It seems that the destruction order is​:

Object $x
Object $x->{val}
Object $y

but you need to check this (I haven't got time to actual build this
module to confirm).

Yes, I get a different object each time and a diffierent scalar for
each 'val' hash entry :


Foo​::new >>Foo=HASH(0x80f59e8)<<>>BARPtr=SCALAR(0x80f5a30)<<
Foo​::new >>Foo=HASH(0x80f5a18)<<>>BARPtr=SCALAR(0x8221338)<<
Foo​::DESTROY >>Foo=HASH(0x80f59e8)<<>>BARPtr=SCALAR(0x80f5a30)<<
Use of uninitialized value in print at blib/lib/Foo.pm line 54 during
global destruction.
Foo​::DESTROY >>Foo=HASH(0x80f5a18)<<>><<


I attached a static pointer just to simplify the code, but having
different
pointers doesn't change anything.

I add the 'diff -u' of Foo.pm that generates the output above, just in
case
someone wants to check it out.

François

Start of patch ------------------------------------

Inline Patch
--- Foo.pm.orig	Thu Apr 12 11:09:44 2001
+++ Foo.pm	Thu Apr 12 11:05:21 2001
@@ -34,6 +34,26 @@
 
 # Autoload methods go after =cut, and are processed by the autosplit
program.

+sub new {
+ my($class) = shift;
+ my($self) = { val => undef };
+
+ $self->{val} = _create('a');
+
+ $class = ref($class) || $class;
+
+ bless($self, $class);
+
+ print "Foo​::new >>$self<<>>$self->{val}<<\n";
+ $self
+}
+
+sub DESTROY {
+ my($self) = shift;
+
+ print "Foo​::DESTROY >>", $self, "<<>>", $self->{val}, "<<\n";
+}
+
1;
__END__
# Below is stub documentation for your module. You better edit it!
End of patch --------------------------------------
Line 1 : $x = new Foo()
Line 2 : $y = new Foo()
Line 3 : DESTROY($x)
Line 5 : DESTROY($y)

@p5pRT
Copy link
Author

p5pRT commented Apr 12, 2001

From @timj

Tim Jenness wrote​:

I assume you are using T_PTROBJ in your local typemap?

Oooops yes. The typemap is

BAR * T_PTROBJ

Can you check that you really are getting a different object each time.
You're blessing a static pointer into a perl object so each object
is attached to the same pointer. I imagine the first destroy is killing
the pointer object and the second destroy is simply killing your wrapper
object (the input arg to _create is never used). Print $self->{val}
after each object is created to see whether the objects are different.

It seems that the destruction order is​:

Object $x
Object $x->{val}
Object $y

but you need to check this (I haven't got time to actual build this
module to confirm).

Yes, I get a different object each time and a diffierent scalar for
each 'val' hash entry :

-------------------------------------------------------------------------
Foo​::new >>Foo=HASH(0x80f59e8)<<>>BARPtr=SCALAR(0x80f5a30)<<
Foo​::new >>Foo=HASH(0x80f5a18)<<>>BARPtr=SCALAR(0x8221338)<<
Foo​::DESTROY >>Foo=HASH(0x80f59e8)<<>>BARPtr=SCALAR(0x80f5a30)<<
Use of uninitialized value in print at blib/lib/Foo.pm line 54 during
global destruction.
Foo​::DESTROY >>Foo=HASH(0x80f5a18)<<>><<
-------------------------------------------------------------------------

Add a destructor in package BARPtr.

MODULE = Foo PACKAGE = BARPtr

void
DESTROY( self )
  BAR * self
CODE​:
  printf("In destructor for BARPtr\n");

or something. This will tell you when perl is destroying the pointer
object as well as when it destroys the perl object.

As I said yesterday, it sounds like you are being bitten by the
destruction order of objects (but you need to check).

--
Tim Jenness
JAC software
http​://www.jach.hawaii.edu/~timj

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2001

From [Unknown Contact. See original ticket]

First, I want to thank you very much for trying to help me.

But I'm not bitten by anything : I just submitted this bug report
to let the p5p team know that Perl does weird things in global
destructions, providing them an easy way to reproduce the problem,
as it maybe could hide something more serious.

Actually, my XS module is on its way and doesn't share anything from
the Foo example.

I just came across this insanity and posted only for the sake of
Perl, hoping it can help.

Perlishly yours,

François

Tim Jenness a écrit :

Add a destructor in package BARPtr.

MODULE = Foo PACKAGE = BARPtr

void
DESTROY( self )
BAR * self
CODE​:
printf("In destructor for BARPtr\n");

or something. This will tell you when perl is destroying the pointer
object as well as when it destroys the perl object.

As I said yesterday, it sounds like you are being bitten by the
destruction order of objects (but you need to check).

--
Tim Jenness
JAC software
http​://www.jach.hawaii.edu/~timj

@p5pRT
Copy link
Author

p5pRT commented Nov 30, 2004

From @Signum

Created by @Signum

This is a bug report for perl from email@​christoph-haas.de,
generated with the help of perlbug 1.35 running under perl v5.8.4.

-----------------------------------------------------------------
It seems like in some cases class members disappear during the
garbage collector's global destruction. I have prepared two
test files at http​://workaround.org/perl-problem/ that show
the problem.

The "Helper.pm" declares a simple class that does nothing really
except creating (new) and destroying (DESTROY) an object.
The "helper" script just creates an object and does nothing
else. Even the supplied "sub test" is not called. When running
./helper the creation and destruction of the object is shown.
It looks like this here​:

NEW​:$VAR1 = {
  'cgi' => bless( {
  '.parameters' => [],
  '.charset' => 'ISO-8859-1',
  '.fieldnames' => {},
  'escape' => 1
  }, 'CGI' ),
  'foo' => 42
  };
DESTROY​:$VAR1 = bless( {
  'cgi' => undef,
  'foo' => 42
  }, 'Helper' );

So the CGI object (as a member of the Helper class) is created
correctly. Just when it comes to the global destruction it gets
lost. Other members like 'foo' however stay there. So it seems
like the CGI class is destroyed before the destruction of the
Helper class. This is not what I expected.

When I call $helper->DESTROY then the CGI class is still
existing. So it's not a problem of the DESTROY sub but of
the global destruction that happens automatically at the end
of the program.

Interesting though​: when I declare a lexical (local) variable
in the "sub test" (that is completely unused!) then the CGI
object is also still there. Just if I use $helper from the
global main context it fails.

I could verify that in Perl 5.6.0 this works as expected. Two
other users in the IRC (#perl) verified this problem.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl v5.8.4:

Configured by Debian Project at Wed Sep  1 12:11:29 CEST 2004.

Summary of my perl5 (revision 5 version 8 subversion 4) configuration:
  Platform:
    osname=linux, osvers=2.6.7.fb, archname=i386-linux-thread-multi
    uname='linux downhill 2.6.7.fb #1 sun jul 18 15:42:00 cest 2004 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i386-linux -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.4 -Dsitearch=/usr/local/lib/perl/5.8.4 -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 -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.4 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef 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 -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include'
    ccversion='', gccversion='3.3.4 (Debian 1:3.3.4-9)', 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
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so.5.8.4
    gnulibc_version='2.3.2'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.4:
    /etc/perl
    /usr/local/lib/perl/5.8.4
    /usr/local/share/perl/5.8.4
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.8
    /usr/share/perl/5.8
    /usr/local/lib/site_perl
    .


Environment for perl v5.8.4:
    HOME=/home/chaas
    LANG=de_DE@euro
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin
    PERL_BADLANG (unset)
    SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Nov 30, 2004

From @mjdominus

Created by @mjdominus

The following program manufactures an "Outer" object that contains an
"Inner" subobject. The outer object is destroyed during the global
destruction phase, but at the time that Outer​::DESTOY is called, the
outer object has been corrupted. Its inner object is already gone,
and its value in the outer hash has been set to "undef".

  #!/usr/bin/perl

  use Data​::Dumper;

  sub Outer​::new
  {
  my $self = {subobject => bless(["Hey, where did I go?"], "Inner")};
  bless($self, 'Outer');
  }

  sub Outer​::DESTROY
  {
  warn "Hey, where is my subobject?" unless defined $_[0]{subobject};
  }

  $helper = new Outer;

  sub test
  {
  print STDERR "TEST​: $helper";
  }

Here Outer​::DESTROY is expecting that its subobject will still be
intact at the time the outer object is destroyed. It plaintively asks
about the location of its missing inner object​:

  Hey, where is my subobject? at helper line 13 during global destruction.

The bug is highly peturbable. Removing the (unused) "use
Data​::Dumper" statement makes it go away, as does removing the
(unused) "test" subroutine or the (unused) "print" statement inside of
"test".

This bug was reported by Christoph Haas under 5.8.4; his bug report
should be coming through soon, if it has not already.

Perl Info

Flags:
    category=core
    severity=high

Site configuration information for perl v5.8.0:

Configured by mjd at Thu Apr 17 11:57:37 EDT 2003.

Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.4.2-2, archname=i586-linux
    uname='linux plover.com 2.4.2-2 #1 sun apr 8 19:37:14 edt 2001 i586 unknown '
    config_args='-des'
    hint=previous, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm'
    ccversion='', gccversion='2.96 20000731 (Red Hat Linux 7.1 2.96-81)', 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
    libs=-lnsl -lndbm -lgdbm -ldl -lm -lc -lcrypt -lutil
    perllibs=-lnsl -ldl -lm -lc -lcrypt -lutil
    libc=/lib/libc-2.2.2.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.2.4'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.0:
    /usr/local/lib/perl5/5.8.0/i586-linux
    /usr/local/lib/perl5/5.8.0
    /usr/local/lib/perl5/site_perl/5.8.0/i586-linux
    /usr/local/lib/perl5/site_perl/5.8.0
    /usr/local/lib/perl5/site_perl/5.7.3
    /usr/local/lib/perl5/site_perl/5.7.2
    /usr/local/lib/perl5/site_perl/5.6.1
    /usr/local/lib/perl5/site_perl/5.6.0
    /usr/local/lib/perl5/site_perl
    .


Environment for perl v5.8.0:
    HOME=/home/mjd
    LANG=C
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/lib:/usr/lib:/usr/X11R6/lib
    LOGDIR (unset)
    PATH=/home/mjd/bin:/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin:/usr/games:/sbin:/usr/sbin:/usr/local/bin/X11R6:/usr/local/bin/mh:/data/mysql/bin:/usr/local/bin/pbm:/usr/local/bin/ezmlm:/home/mjd/TPI/bin:/usr/local/teTeX/bin:/usr/local/mysql/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

From @demerphq

Subject​: [perl #32714] Objects destroyed in the wrong order during global
destruction

I thought order of destruction in perl in general was undefined? At least
Perltoot seems to agree with me​:

"Perl's notion of the right time to call a destructor is not well-defined
currently, which is why your destructors should not rely on when they are
called."

There is also something on the subject in perlobj.pod but it doesn't say
that there is a defined order.

Im wondering why Perl doesn't have a counter whose value is incremented and
assigned to each blessed object when they are blessed. Then in global
destruction Perl could ensure that the objects are destroyed in the correct
order (hmmm, would that be oldest first or newest first?). Is it just
because the memory overhead is too much for the gain to be worthwhile?

Yves

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

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

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

From @mjdominus

Subject​: [perl #32714] Objects destroyed in the wrong order during global
destruction

I thought order of destruction in perl in general was undefined? At least
Perltoot seems to agree with me​:

"Perl's notion of the right time to call a destructor is not well-defined
currently, which is why your destructors should not rely on when they are
called."

That is talking about something else that is irrelevant in this case.
That paragraph refers to this situation;

  { my $obj = Class->new;
  ...
  }

  B;

The question here is whether Class​::DESTROY is called immediately at
the end of the block, before B is executed. The paragraph you quote
is warning us that that might not be the case.

However, the problem I am reporting is a different situation. Here, a
piece of data is destroyed prematurely, while there is still a
reference to it. That is not the same thing at all; Perl *does*
guarantee that it will not destroy an object that is reachable from an
executing program.

In my example, Perl's premature execution of the inner object while
there is still a reference to it from the outer object is a bug, and
one that could easily lead to a core dump or other bad behavior.

Im wondering why Perl doesn't have a counter whose value is incremented and
assigned to each blessed object when they are blessed.

It maintains a count of the number of references to each object; this
count is intended to prevent exactly the behavior I am reporting.
That it does not, in this case, indicates that there is a bug.

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

From @demerphq

MJD wrote​:

That is talking about something else that is irrelevant in this case.
That paragraph refers to this situation;

    \{ my $obj = Class\->new;
      \.\.\.
    \}

    B;

The question here is whether Class​::DESTROY is called immediately at
the end of the block, before B is executed. The paragraph you quote
is warning us that that might not be the case.

Well I read that as a general observation that you cant count on DESTROY
being called at any particular time, which to me implys order.

However, the problem I am reporting is a different situation. Here, a
piece of data is destroyed prematurely, while there is still a
reference to it. That is not the same thing at all; Perl *does*
guarantee that it will not destroy an object that is reachable from an
executing program.

Only in normal destruction. It _cant_ make the guarantee that you say it
does or circular structures would never be broken during global destruction​:

  my $x=bless {},'Foo';
  my $y=bless {},'Foo';
  $x->{y}=$y;
  $y->{x}=$x;

  my $z;
  $z=bless \$z,"Foo";

  sub Foo​::Destroy { print "Destroying​: @​_\n" }

Im wondering why Perl doesn't have a counter whose value is incremented
and
assigned to each blessed object when they are blessed.

It maintains a count of the number of references to each object; this
count is intended to prevent exactly the behavior I am reporting.

Not in global destruction though. Refcounting can't help there at all.
That's why I was suggesting the counter approach. At least then you could
know that the objects would be destroyed in a defined order.

yves

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

From @mjdominus

Only in normal destruction. It _cant_ make the guarantee that you say it
does or circular structures would never be broken during global destruction​:

Yes, you're right. But in this case there is no circular structure.

As I mentioned to Christoph Haas, our surprise at this situation might
be the result of our misunderstanding of global destruction semantics.
I am still willing to admit that, but it also seems to me that the
present behavior is unnecessarily bad.

That's why I was suggesting the counter approach. At least then you could
know that the objects would be destroyed in a defined order.

I misunderstood your suggestion. Thanks for explaining it.

It still seems to me that Perl's final destruction should have a first
pass in which objects are destroyed normally, in
reference-count-order, followed by a second pass to take care of any
remaining circularly-referenced objects. Or alternatively, that the
outer object in my example should be destroyed at the end of program
execution, but before the global destruction paqhse is initiated.

@p5pRT
Copy link
Author

p5pRT commented Dec 2, 2004

From @lizmat

At 1​:21 PM -0500 12/2/04, Mark Jason Dominus wrote​:

Only in normal destruction. It _cant_ make the guarantee that you say it
does or circular structures would never be broken during global destruction​:

Yes, you're right. But in this case there is no circular structure.

As I mentioned to Christoph Haas, our surprise at this situation might
be the result of our misunderstanding of global destruction semantics.
I am still willing to admit that, but it also seems to me that the
present behavior is unnecessarily bad.

That's why I was suggesting the counter approach. At least then you could
know that the objects would be destroyed in a defined order.

I misunderstood your suggestion. Thanks for explaining it.

It still seems to me that Perl's final destruction should have a first
pass in which objects are destroyed normally, in
reference-count-order, followed by a second pass to take care of any
remaining circularly-referenced objects. Or alternatively, that the
outer object in my example should be destroyed at the end of program
execution, but before the global destruction paqhse is initiated.

I think it would be possible to create a module (pragma) that would
ensure an orderly (as in lifo) destruction of objects. The only
thing I miss is a way to make sure a specific END code block is
executed as the very last END block. Is there a way to do this?
Possibly in XS?

Liz

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @pjcj

On Thu, Dec 02, 2004 at 10​:03​:59PM +0100, Elizabeth Mattijsen wrote​:

I think it would be possible to create a module (pragma) that would
ensure an orderly (as in lifo) destruction of objects. The only
thing I miss is a way to make sure a specific END code block is
executed as the very last END block. Is there a way to do this?
Possibly in XS?

Devel​::Cover does just this. It's something of an unholy mix of XS and
perl subs for various arcane reasons, at least some of which are
unlikely to be valid any more.

The way it works is that in a CHECK block an XS sub is called which
installs a perl sub as as the first END sub. This is done by unshifting
onto PL_endav. When that sub is called as the first END sub it calls
another XS sub which installs another perl sub as the last END sub by
pushing it onto PL_endav. That newly installed sub should be the last
END sub called. Unless one of the subsequent END subs installs another
END sub.

It's probably easier to look at the code. It's on CPAN.

But if you do this, our two modules will fight. Which means I'll have
to make my final END sub not really the final one any more, but have it
install another END sub which really will be the last ;-)

--
Paul Johnson - paul@​pjcj.net
http​://www.pjcj.net

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @ysth

On Fri, Dec 03, 2004 at 01​:01​:20AM +0100, Paul Johnson <paul@​pjcj.net> wrote​:

Devel​::Cover does just this. It's something of an unholy mix of XS and
perl subs for various arcane reasons, at least some of which are
unlikely to be valid any more.

The way it works is that in a CHECK block an XS sub is called which
installs a perl sub as as the first END sub. This is done by unshifting
onto PL_endav. When that sub is called as the first END sub it calls
another XS sub which installs another perl sub as the last END sub by
pushing it onto PL_endav. That newly installed sub should be the last
END sub called. Unless one of the subsequent END subs installs another
END sub.

Does the first END sub do something else also? Why not skip the CHECK
and just have a regular END sub that inserts a last END sub?

No dedicated XS needed, at least as of 5.8.1​:

perl -we'use B qw/end_av/; END { print "baz" } END {print "bar"; push @​{end_av()->object_2svref}, sub { print "quux" } } END { print "foo" }'

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @lizmat

At 1​:01 AM +0100 12/3/04, Paul Johnson wrote​:

On Thu, Dec 02, 2004 at 10​:03​:59PM +0100, Elizabeth Mattijsen wrote​:

I think it would be possible to create a module (pragma) that would
ensure an orderly (as in lifo) destruction of objects. The only
thing I miss is a way to make sure a specific END code block is
executed as the very last END block. Is there a way to do this?
Possibly in XS?

Devel​::Cover does just this. It's something of an unholy mix of XS and
perl subs for various arcane reasons, at least some of which are
unlikely to be valid any more.

The way it works is that in a CHECK block an XS sub is called which
installs a perl sub as as the first END sub. This is done by unshifting
onto PL_endav. When that sub is called as the first END sub it calls
another XS sub which installs another perl sub as the last END sub by
pushing it onto PL_endav. That newly installed sub should be the last
END sub called. Unless one of the subsequent END subs installs another
END sub.

It's probably easier to look at the code. It's on CPAN.

But if you do this, our two modules will fight. Which means I'll have
to make my final END sub not really the final one any more, but have it
install another END sub which really will be the last ;-)

How about if Devel​::Cover would supply a method/subroutine to install
the "last" END block? Then it would be easy to ensure that
Devel​::Cover's is really the last.

On the other hand, having Devel​::Cover as a prerequisite may be quite
overkill. Maybe this functionality should be put into a seperate
module that would become a prerequisite for both?

Liz

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @lizmat

At 11​:20 PM -0800 12/2/04, Yitzchak Scott-Thoennes wrote​:

On Fri, Dec 03, 2004 at 01​:01​:20AM +0100, Paul Johnson <paul@​pjcj.net> wrote​:

Devel​::Cover does just this. It's something of an unholy mix of XS and
perl subs for various arcane reasons, at least some of which are
unlikely to be valid any more.

The way it works is that in a CHECK block an XS sub is called which
installs a perl sub as as the first END sub. This is done by unshifting
onto PL_endav. When that sub is called as the first END sub it calls
another XS sub which installs another perl sub as the last END sub by
pushing it onto PL_endav. That newly installed sub should be the last
END sub called. Unless one of the subsequent END subs installs another
END sub.

Does the first END sub do something else also? Why not skip the CHECK
and just have a regular END sub that inserts a last END sub?

That would be my plan, yes.

No dedicated XS needed, at least as of 5.8.1​:
perl -we'use B qw/end_av/; END { print "baz" } END {print "bar";
push @​{end_av()->object_2svref}, sub { print "quux" } } END { print
"foo" }'

Wow. But is having B as a prerequisite a good idea?

Liz

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @rgs

Elizabeth Mattijsen wrote​:

Wow. But is having B as a prerequisite a good idea?

Well, B is core :)

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @lizmat

At 11​:54 AM +0100 12/3/04, Rafael Garcia-Suarez wrote​:

Elizabeth Mattijsen wrote​:

Wow. But is having B as a prerequisite a good idea?

Well, B is core :)

Ah, ok!

Well, I've just uploaded this to CPAN​:

=head1 NAME

ogd - ordered global destruction of objects

=head1 SYNOPSIS

  use ogd;

  ogd->register( @​object ); # for objects from XSUBs only

=head1 DESCRIPTION

This module adds ordered destruction of object in LIFO order during global
destruction.

=head1 CLASS METHODS

=head2 register

  ogd->register( @​object ); # only for blessed objects created in XSUBs

Not all blessed objects in Perl are necessarily created with "bless"​: they can
also be created in XSUBs and thereby bypass the registration mechanism that
ogd installs for "bless". For those cases, it is possible to register objects
created in such a manner by calling the "register" class function. Any object
passed to it will be registered.

=head1 REQUIRED MODULES

  B (any)
  Scalar​::Util (any)

=head1 ORDER OF LOADING

The C<ogd> pragma installs its own version of the "bless" system function.
Without that special version of "bless", it can not work (unless you
L<register> your objects yourself). This means that the C<ogd> pragma
needs to be loaded B<before> any modules that you want the special
functionality of C<ogd> to be applied to.

=head1 TODO

Examples should be added.

=head1 AUTHOR

Elizabeth Mattijsen, <liz@​dijkmat.nl>.

Please report bugs to <perlbugs@​dijkmat.nl>.

=head1 ACKNOWLEDGEMENTS

Mark Jason Dominus for the initial impetus. Yitzchak Scott-Thoennes for
the suggestion of using the B module. Inspired by similar work on
L<Thread​::Bless>.

=head1 COPYRIGHT

Copyright (c) 2004 Elizabeth Mattijsen <liz@​dijkmat.nl>. All rights
reserved. This program is free software; you can redistribute it and/or
modify it under the same terms as Perl itself.

=head1 SEE ALSO

L<Thread​::Bless>.

=cut

For those who cannot wait​: it's also available from​:

  http​://www.liz.nl/CPAN/ogd-0.01.tar.gz

Liz

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @ysth

On Fri, Dec 03, 2004 at 04​:03​:42PM +0100, Elizabeth Mattijsen <liz@​dijkmat.nl> wrote​:

At 11​:54 AM +0100 12/3/04, Rafael Garcia-Suarez wrote​:

Elizabeth Mattijsen wrote​:

Wow. But is having B as a prerequisite a good idea?

Well, B is core :)

Ah, ok!

Well, I've just uploaded this to CPAN​:

=head1 NAME

ogd - ordered global destruction of objects

Before 5.8.1, there wasn't a B​::object_2svref, so either put in a
"use 5.008001;", or maybe use Manip​::END for earlier perls.

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2004

From @lizmat

At 11​:06 AM -0800 12/3/04, Yitzchak Scott-Thoennes wrote​:

On Fri, Dec 03, 2004 at 04​:03​:42PM +0100, Elizabeth Mattijsen
<liz@​dijkmat.nl> wrote​:

At 11​:54 AM +0100 12/3/04, Rafael Garcia-Suarez wrote​:

Elizabeth Mattijsen wrote​:

Wow. But is having B as a prerequisite a good idea?

Well, B is core :)

Ah, ok!

Well, I've just uploaded this to CPAN​:

=head1 NAME

ogd - ordered global destruction of objects

Before 5.8.1, there wasn't a B​::object_2svref, so either put in a
"use 5.008001;", or maybe use Manip​::END for earlier perls.

I've put that in the Makefile.PL, to avoid the overhead during
execution. This should prevent it from getting installed.

Liz

@p5pRT
Copy link
Author

p5pRT commented Jul 7, 2016

From @dcollinsn

Based on the above suggestions to make a pragma and upload it to cpan, and the report that Liz has done so, I believe this ticket can be closed? I intend to close resolved in a week unless there is some objection.

@p5pRT
Copy link
Author

p5pRT commented Jul 12, 2016

@dcollinsn - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Jul 12, 2016
@p5pRT
Copy link
Author

p5pRT commented Mar 9, 2017

From @nwellnhof

On Thu, 07 Jul 2016 09​:49​:52 -0700, dcollinsn@​gmail.com wrote​:

Based on the above suggestions to make a pragma and upload it to cpan,
and the report that Liz has done so, I believe this ticket can be
closed? I intend to close resolved in a week unless there is some
objection.

I object to closing this bug. Perl really should run global destruction in two phases​:

1. Decrease the refcount of globals like "our" variables which will run destructors in correct order unless there are circular references or refcount leaks.

2. Only then forcibly destroy the remaining objects.

Ideally, it should be possible to tell which of the two phases is run. Some code may want to skip forced destruction in phase 2.

There's no need for a pragma that adds additional overhead. Besides, "use ogd" doesn't seem to be very useful. In most cases you don't want LIFO order, but make sure that an object that is still referred to isn't destroyed before the referrer.

@p5pRT
Copy link
Author

p5pRT commented Mar 10, 2017

From @Leont

On Thu, Mar 9, 2017 at 9​:55 PM, Nick Wellnhofer via RT <
perlbug-followup@​perl.org> wrote​:

On Thu, 07 Jul 2016 09​:49​:52 -0700, dcollinsn@​gmail.com wrote​:

Based on the above suggestions to make a pragma and upload it to cpan,
and the report that Liz has done so, I believe this ticket can be
closed? I intend to close resolved in a week unless there is some
objection.

I object to closing this bug. Perl really should run global destruction in
two phases​:

1. Decrease the refcount of globals like "our" variables which will run
destructors in correct order unless there are circular references or
refcount leaks.

2. Only then forcibly destroy the remaining objects.

Ideally, it should be possible to tell which of the two phases is run.
Some code may want to skip forced destruction in phase 2.

There's no need for a pragma that adds additional overhead. Besides, "use
ogd" doesn't seem to be very useful. In most cases you don't want LIFO
order, but make sure that an object that is still referred to isn't
destroyed before the referrer.

Destructors can and do depend on package variables, this would not solve
all non-circular cases. Just because the roots of the graph are package
variables doesn't mean all package variables are roots.

The proper solution would be to topologically sort the objects and then
destroy them in that order (ideally even taking into account that the
destruction can add new nodes to the graph), but this is non-trivial to
implement and to execute; and obviously would still not solve circular
references.

Leon

@p5pRT
Copy link
Author

p5pRT commented Mar 10, 2017

From @nwellnhof

On Fri, 10 Mar 2017 02​:16​:13 -0800, LeonT wrote​:

Destructors can and do depend on package variables, this would not solve
all non-circular cases.

Right, but it would solve a large class of cases. Also note that Perl seems to do the right thing for "my" variables with package scope. Only for "our" variables, it seems impossible to get a sane destruction order. If you run the program below, $my1 if always destroyed before $my2, regardless of initialization order. But "our" variables are always destroyed in the DESTRUCT phase without taking references between objects into account.

The proper solution would be to topologically sort the objects and then
destroy them in that order (ideally even taking into account that the
destruction can add new nodes to the graph), but this is non-trivial to
implement and to execute; and obviously would still not solve circular
references.

Just because a solution isn't perfect doesn't mean that it shouldn't be implemented. Anyway, I'm only asking to acknowledge that this bug isn't fixed and to reopen it.

#!/usr/bin/perl
use strict;
use warnings;

package TestObj;

sub new {
  my ( $class, $name ) = @​_;
  return bless { name => $name }, $class;
}

sub set_ref {
  my ( $self, $ref ) = @​_;
  $self->{ref} = $ref;
}

sub DESTROY {
  my $self = shift;
  print "Destroying $self-&gt;{name}, phase is ${^GLOBAL_PHASE}\n";
}

my $my1 = TestObj->new('my1');
my $my2 = TestObj->new('my2');
$my1->set_ref($my2);

our $our1 = TestObj->new('our1');
our $our2 = TestObj->new('our2');
$our1->set_ref($our2);

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2017

From @cpansprout

On Fri, 10 Mar 2017 07​:38​:02 -0800, wellnhofer@​aevum.de wrote​:

On Fri, 10 Mar 2017 02​:16​:13 -0800, LeonT wrote​:

Destructors can and do depend on package variables, this would not
solve
all non-circular cases.

Right, but it would solve a large class of cases. Also note that Perl
seems to do the right thing for "my" variables with package scope.
Only for "our" variables, it seems impossible to get a sane
destruction order. If you run the program below, $my1 if always
destroyed before $my2, regardless of initialization order. But "our"
variables are always destroyed in the DESTRUCT phase without taking
references between objects into account.

The proper solution would be to topologically sort the objects and
then
destroy them in that order (ideally even taking into account that the
destruction can add new nodes to the graph), but this is non-trivial
to
implement and to execute; and obviously would still not solve
circular
references.

Just because a solution isn't perfect doesn't mean that it shouldn't
be implemented. Anyway, I'm only asking to acknowledge that this bug
isn't fixed and to reopen it.

Seems fair enough. I am doing exactly that.

But let me note that simply destroying ‘our’ variables before the general reference sweep is not so straightforward. If you start by deleting everything stored by globs, then you end up deleting the destructors themselves. Again, deleting everything from globs except subs would only work for some code, and not other code. What Leon Timmermans said about a topological order would probably be the only solution.

#!/usr/bin/perl
use strict;
use warnings;

package TestObj;

sub new {
my ( $class, $name ) = @​_;
return bless { name => $name }, $class;
}

sub set_ref {
my ( $self, $ref ) = @​_;
$self->{ref} = $ref;
}

sub DESTROY {
my $self = shift;
print "Destroying $self-&gt;{name}, phase is ${^GLOBAL_PHASE}\n";
}

my $my1 = TestObj->new('my1');
my $my2 = TestObj->new('my2');
$my1->set_ref($my2);

our $our1 = TestObj->new('our1');
our $our2 = TestObj->new('our2');
$our1->set_ref($our2);

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2017

From @avar

On Sat, Mar 11, 2017 at 2​:41 AM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 10 Mar 2017 07​:38​:02 -0800, wellnhofer@​aevum.de wrote​:

On Fri, 10 Mar 2017 02​:16​:13 -0800, LeonT wrote​:

Destructors can and do depend on package variables, this would not
solve
all non-circular cases.

Right, but it would solve a large class of cases. Also note that Perl
seems to do the right thing for "my" variables with package scope.
Only for "our" variables, it seems impossible to get a sane
destruction order. If you run the program below, $my1 if always
destroyed before $my2, regardless of initialization order. But "our"
variables are always destroyed in the DESTRUCT phase without taking
references between objects into account.

The proper solution would be to topologically sort the objects and
then
destroy them in that order (ideally even taking into account that the
destruction can add new nodes to the graph), but this is non-trivial
to
implement and to execute; and obviously would still not solve
circular
references.

Just because a solution isn't perfect doesn't mean that it shouldn't
be implemented. Anyway, I'm only asking to acknowledge that this bug
isn't fixed and to reopen it.

Seems fair enough. I am doing exactly that.

But let me note that simply destroying ‘our’ variables before the general reference sweep is not so straightforward. If you start by deleting everything stored by globs, then you end up deleting the destructors themselves. Again, deleting everything from globs except subs would only work for some code, and not other code. What Leon Timmermans said about a topological order would probably be the only solution.

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

#!/usr/bin/perl
use strict;
use warnings;

package TestObj;

sub new {
my ( $class, $name ) = @​_;
return bless { name => $name }, $class;
}

sub set_ref {
my ( $self, $ref ) = @​_;
$self->{ref} = $ref;
}

sub DESTROY {
my $self = shift;
print "Destroying $self-&gt;{name}, phase is ${^GLOBAL_PHASE}\n";
}

my $my1 = TestObj->new('my1');
my $my2 = TestObj->new('my2');
$my1->set_ref($my2);

our $our1 = TestObj->new('our1');
our $our2 = TestObj->new('our2');
$our1->set_ref($our2);

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: resolved
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=32714

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2017

From @cpansprout

On Sat, 11 Mar 2017 04​:33​:24 -0800, avarab@​gmail.com wrote​:

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?

Well, there is sv_clean_objs. I think it is an API function.

Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

I’m not exactly poking it. I *have* poked it in the past, so I thought I could give some input (which I fear proved no more helpful than what was already said).

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2017

From @Leont

On Sat, Mar 11, 2017 at 1​:32 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

Perl already has a concept of destruct level, current valid values are
0, 1, and 2.

0 destructs much of the interpreter and all objects, this is the
default on unthreaded perls.
1 destructs a bit more of the interpreter, in particular the stashes.
This is the default in an embeded perl with multiplicity enabled.
2 destructs absolutely everything, including non-object cycles and the
global string table. threads.pm sets it to this value.

I'm not sure what the value for PL_perl_destruct_level is in your
particular case, lowering it may help. I can imagine introducing a -1
level that does little more than run END blocks and flush IO buffers.

Leon

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2017

From @iabyn

On Fri, Mar 10, 2017 at 07​:38​:02AM -0800, Nick Wellnhofer via RT wrote​:

Right, but it would solve a large class of cases.

But has just been pointed out, freeing package vars before calling
destructors breaks everything.

Also note that Perl seems to do the right thing for "my" variables with
package scope. Only for "our" variables, it seems impossible to get a
sane destruction order.

file-scoped lexical variables are freed (or rather their reference count
is decreased) when execution falls off the end of the file or eval or
whatever. This is a well-defined point *before* global destruction.
They are also closed over by subs such as destructors, so will continue to
live if they are needed.

Package variables have no such concepts (well, subs tend to have ops that
hold a reference count to the gobs that hold the package vars, so in that
sense they close over package vars).

--
Standards (n). Battle insignia or tribal totems.

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2017

From @nwellnhof

On Mon, 13 Mar 2017 02​:31​:15 -0700, davem wrote​:

But has just been pointed out, freeing package vars before calling
destructors breaks everything.

I realize that this problem is harder than I initially thought. But I think it could be solved by deferring freeing of package vars during global destruction. So if a refcount drops to zero, DESTROY is invoked, the object is not freed, but refcounts of other objects that are referred to are decreased. In other words, you simulate freeing an object just for refcount side effects. Then, in a second pass, DESTROY is invoked on objects that still have a refcount (because of circular references or refcount leaks).

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2017

From @iabyn

On Mon, Mar 13, 2017 at 08​:52​:09AM -0700, Nick Wellnhofer via RT wrote​:

On Mon, 13 Mar 2017 02​:31​:15 -0700, davem wrote​:

But has just been pointed out, freeing package vars before calling
destructors breaks everything.

I realize that this problem is harder than I initially thought. But I
think it could be solved by deferring freeing of package vars during
global destruction. So if a refcount drops to zero, DESTROY is invoked,
the object is not freed, but refcounts of other objects that are
referred to are decreased. In other words, you simulate freeing an
object just for refcount side effects. Then, in a second pass, DESTROY
is invoked on objects that still have a refcount (because of circular
references or refcount leaks).

Can you provide a short, self-contained perl program which demonstrates
the bug, and which would be fixed by why you propose above; just so that
we can be sure we're all discussing the same issue.

--
In economics, the exam questions are the same every year.
They just change the answers.

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2017

From @nwellnhof

On Tue, 14 Mar 2017 01​:46​:17 -0700, davem wrote​:

Can you provide a short, self-contained perl program which demonstrates
the bug, and which would be fixed by why you propose above; just so that
we can be sure we're all discussing the same issue.

I already did so here​:

  https://rt-archive.perl.org/perl5/Ticket/Display.html?id=32714#txn-1453380

I would expect $our1->DESTROY to be run before $our2->DESTROY because $our1 holds a reference to $our2. But (at least with Perl 5.22.1), the DESTROY method is first invoked on $our2.

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2017

From @demerphq

On 11 March 2017 at 22​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Sat, Mar 11, 2017 at 1​:32 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

Perl already has a concept of destruct level, current valid values are
0, 1, and 2.

0 destructs much of the interpreter and all objects, this is the
default on unthreaded perls.
1 destructs a bit more of the interpreter, in particular the stashes.
This is the default in an embeded perl with multiplicity enabled.
2 destructs absolutely everything, including non-object cycles and the
global string table. threads.pm sets it to this value.

I'm not sure what the value for PL_perl_destruct_level is in your
particular case, lowering it may help. I can imagine introducing a -1
level that does little more than run END blocks and flush IO buffers.

Those models dont match the problem Booking is solving. They may match
a different problem but that is a different question.

The issue here is that in a multi-forked process model you want to add
an extra concept​: that is which process "owns" the data, and thus is
responsible for the cleanup of the objects involved.

So consider a scenario where we have a mother process which owns an
object, lets say a string. Now we fork, and have the child process do
something, say write some summaries to disk. When that child process
terminates we don't want to it to try to free or destruct any
variables owned by the parent process that the child process hasn't
touched, in this case the string structure owned by the parent.

What happens in a normal scenario is that the refcount page holding
that string is decremented during cleanup by the child, which un-COWs
the page.

If you are using a fork/exec model, the costs of tearing down all the
structures which are effectively RO to the child process can be very
significant, to the point of being a "forkbomb" in disguise.

So we don't tear down, we just kill everything. Avar would like a more
graceful solution that knows that there are some things that the child
should tear down, and that the rest should not be.

Examples of this abound btw, for instance consider PL_strtab. Every
time a child process exits we free up all those keys....

Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2017

From @avar

On Tue, Mar 14, 2017 at 1​:22 PM, demerphq <demerphq@​gmail.com> wrote​:

On 11 March 2017 at 22​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Sat, Mar 11, 2017 at 1​:32 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

Perl already has a concept of destruct level, current valid values are
0, 1, and 2.

0 destructs much of the interpreter and all objects, this is the
default on unthreaded perls.
1 destructs a bit more of the interpreter, in particular the stashes.
This is the default in an embeded perl with multiplicity enabled.
2 destructs absolutely everything, including non-object cycles and the
global string table. threads.pm sets it to this value.

I'm not sure what the value for PL_perl_destruct_level is in your
particular case, lowering it may help. I can imagine introducing a -1
level that does little more than run END blocks and flush IO buffers.

Those models dont match the problem Booking is solving. They may match
a different problem but that is a different question.

The issue here is that in a multi-forked process model you want to add
an extra concept​: that is which process "owns" the data, and thus is
responsible for the cleanup of the objects involved.

So consider a scenario where we have a mother process which owns an
object, lets say a string. Now we fork, and have the child process do
something, say write some summaries to disk. When that child process
terminates we don't want to it to try to free or destruct any
variables owned by the parent process that the child process hasn't
touched, in this case the string structure owned by the parent.

What happens in a normal scenario is that the refcount page holding
that string is decremented during cleanup by the child, which un-COWs
the page.

If you are using a fork/exec model, the costs of tearing down all the
structures which are effectively RO to the child process can be very
significant, to the point of being a "forkbomb" in disguise.

So we don't tear down, we just kill everything. Avar would like a more
graceful solution that knows that there are some things that the child
should tear down, and that the rest should not be.

Examples of this abound btw, for instance consider PL_strtab. Every
time a child process exits we free up all those keys....

Everything you said makes sense & is a good summary of our situation
and why we just short-circuit destruction via POSIX​::_exit().

But just to clarify​: I'm not particularly looking for a graceful
solution like this myself or for us, I just thought it was interesting
to point out this use-case in general if we're talking about tweaks to
how perl does destruction when embedded.

I.e. at Booking's web serving processes we're perfectly happy to just
not run perl_{destroy,free} on teardown, and just say that you can't
have a DESTROY handler hanging off anything but the $env variable.
I.e. we won't run any package-level DESTROY handlers, which is a
trade-off we're OK with, and we're about to tear down the process
anyway so it's fine to leave the PerlInterpreter in a dirty state.

But this is an interesting question for perl embedding in general.
E.g. you might without multiplicity or multithreading have a 10GB perl
interpreter that you only free to the extent that you can spawn
another one, i.e. intentionally leak memory because you're going to
make the OS take care of the cleanup eventually.

Or perhaps out of those 10GB of variables/hashes/data you'd just like
the package-level DESTROY handlers to fire, which might free an un-COW
500MB of those 10GB, but would cost you way less than full
destruction.

Another thing I didn't have time to look into as well is to what
extent the memory layout could be improved to avoid these DoS
situations, i.e. what specifically is causing the pages to COW. I
assume it's because we go around touching the REFCNT which (I think)
is stored in the same pages adjacent to the rest of the SV header.
Perhaps changing some of that around and storing just the REFCNT in
its own pages distinct from the SV's would be a good tradeoff in some
cases.

@p5pRT
Copy link
Author

p5pRT commented Mar 14, 2017

From @demerphq

On 14 March 2017 at 13​:40, Ævar Arnfjörð Bjarmason <avarab@​gmail.com> wrote​:

On Tue, Mar 14, 2017 at 1​:22 PM, demerphq <demerphq@​gmail.com> wrote​:

On 11 March 2017 at 22​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Sat, Mar 11, 2017 at 1​:32 PM, Ævar Arnfjörð Bjarmason
<avarab@​gmail.com> wrote​:

On a related note. At Booking.com in our web applications we don't run
destruction beyond just destroying the Plack $env, then we
POSIX​::_exit(). See https://github.com/unbit/uwsgi/pull/1392/files &
unbit/uwsgi#1384 for why, but tl;dr​: With
forked processes it creates a CPU/Memory DoS attack on the kernel as
it frantically tries to unshare all the pages whose refcounts change.

So uWSGI has an option now to simply skip the perl_destruct() &
perl_free() phases, which from my looking at the source & testing it
seemed like the sane solution​:
https://github.com/avar/uwsgi/blob/bafc14c80771a2c55063c3c9cc9c9dd0377a0294/plugins/psgi/psgi_plugin.c#L860

I can't think of a use-case for this, but is it a good idea to provide
some alternative to POSIX​::_exit() in this case, i.e. is there some
subset of perl_{destruct,free}() that we could run that wouldn't free
everything, but e.g. just enough to run any DESTROY handlers?
Currently if we have some DESTROY handler hanging off anything but
$env it simply won't execute when we teardown the process.

I don't know if that's possible or even a mad idea, but since you're
poking this code I thought I'd bring this use-case to your attention.

Perl already has a concept of destruct level, current valid values are
0, 1, and 2.

0 destructs much of the interpreter and all objects, this is the
default on unthreaded perls.
1 destructs a bit more of the interpreter, in particular the stashes.
This is the default in an embeded perl with multiplicity enabled.
2 destructs absolutely everything, including non-object cycles and the
global string table. threads.pm sets it to this value.

I'm not sure what the value for PL_perl_destruct_level is in your
particular case, lowering it may help. I can imagine introducing a -1
level that does little more than run END blocks and flush IO buffers.

Those models dont match the problem Booking is solving. They may match
a different problem but that is a different question.

The issue here is that in a multi-forked process model you want to add
an extra concept​: that is which process "owns" the data, and thus is
responsible for the cleanup of the objects involved.

So consider a scenario where we have a mother process which owns an
object, lets say a string. Now we fork, and have the child process do
something, say write some summaries to disk. When that child process
terminates we don't want to it to try to free or destruct any
variables owned by the parent process that the child process hasn't
touched, in this case the string structure owned by the parent.

What happens in a normal scenario is that the refcount page holding
that string is decremented during cleanup by the child, which un-COWs
the page.

If you are using a fork/exec model, the costs of tearing down all the
structures which are effectively RO to the child process can be very
significant, to the point of being a "forkbomb" in disguise.

So we don't tear down, we just kill everything. Avar would like a more
graceful solution that knows that there are some things that the child
should tear down, and that the rest should not be.

Examples of this abound btw, for instance consider PL_strtab. Every
time a child process exits we free up all those keys....

Everything you said makes sense & is a good summary of our situation
and why we just short-circuit destruction via POSIX​::_exit().

But just to clarify​: I'm not particularly looking for a graceful
solution like this myself or for us, I just thought it was interesting
to point out this use-case in general if we're talking about tweaks to
how perl does destruction when embedded.

Ok, fair enough. On the other hand, if someone could offer us a
graceful solution to this we would take it and stop doing the
hard-hammer approach we do now...

I.e. at Booking's web serving processes we're perfectly happy to just
not run perl_{destroy,free} on teardown, and just say that you can't
have a DESTROY handler hanging off anything but the $env variable.
I.e. we won't run any package-level DESTROY handlers, which is a
trade-off we're OK with, and we're about to tear down the process
anyway so it's fine to leave the PerlInterpreter in a dirty state.

But this is an interesting question for perl embedding in general.
E.g. you might without multiplicity or multithreading have a 10GB perl
interpreter that you only free to the extent that you can spawn
another one, i.e. intentionally leak memory because you're going to
make the OS take care of the cleanup eventually.

Or perhaps out of those 10GB of variables/hashes/data you'd just like
the package-level DESTROY handlers to fire, which might free an un-COW
500MB of those 10GB, but would cost you way less than full
destruction.

Another thing I didn't have time to look into as well is to what
extent the memory layout could be improved to avoid these DoS
situations, i.e. what specifically is causing the pages to COW. I
assume it's because we go around touching the REFCNT which (I think)
is stored in the same pages adjacent to the rest of the SV header.
Perhaps changing some of that around and storing just the REFCNT in
its own pages distinct from the SV's would be a good tradeoff in some
cases.

The SvHeader holds the recount. And sv heads are packed into pages. An
sv head holds more than just the refcount however, so we dont get the
maximum density possible, but i think its pretty dense as is.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Mar 16, 2017

From @iabyn

On Tue, Mar 14, 2017 at 05​:20​:38AM -0700, Nick Wellnhofer via RT wrote​:

On Tue, 14 Mar 2017 01​:46​:17 -0700, davem wrote​:

Can you provide a short, self-contained perl program which demonstrates
the bug, and which would be fixed by why you propose above; just so that
we can be sure we're all discussing the same issue.

I already did so here​:

https://rt-archive.perl.org/perl5/Ticket/Display.html?id=32714#txn-1453380

Ah sorry, I completely failed to spot that when I looked.

I would expect $our1->DESTROY to be run before $our2->DESTROY because
$our1 holds a reference to $our2. But (at least with Perl 5.22.1), the
DESTROY method is first invoked on $our2.

I realize that this problem is harder than I initially thought. But I
think it could be solved by deferring freeing of package vars during
global destruction. So if a refcount drops to zero, DESTROY is invoked,
the object is not freed, but refcounts of other objects that are
referred to are decreased. In other words, you simulate freeing an
object just for refcount side effects. Then, in a second pass, DESTROY
is invoked on objects that still have a refcount (because of circular
references or refcount leaks).

Note that the effect demonstrated by your code is not caused by global vars
being freed - in fact during both calls to DESTROY, $our1 and $our2 exist
and have a reference count >= 1.

What is actually happening is that at that point during global
destruction, perl is linearly scanning SV arenas looking for any RV which
points to an object. Each such RV is undeffed (*not* freed), which may
trigger a destructor call on the now not-so-referred-to object.

So its a bit like your code having this in it​:

  END { undef $our1; undef $our2 }

except that the order is non-deterministic, so it could equally as well be

  END { undef $our2; undef $our1 }

and note that its also unrelated to package vars​: it's *any* RV that can
be found (including anonymous ones) which gets zapped, in any order. So
for example​:

  our $x;
  $x->{l1}{l2}{l3}{l4} = 1;

(where each hash is blessed) might have a destruction equivalent to any of
these

  END { undef $x->{l1}{l2}; undef $x }
  END { undef $x }
  END { undef $x->{l1}{l2}{l3}; undef $x->{l1}; undef $x }
  ... etc ...

Now having explained all that, I don't really understand your proposal
above. Given that package vars *aren't* freed at that point during global
destruction, could you update and clarify your proposal?

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

@p5pRT
Copy link
Author

p5pRT commented Mar 17, 2017

From @nwellnhof

On Thu, 16 Mar 2017 04​:09​:36 -0700, davem wrote​:

Note that the effect demonstrated by your code is not caused by global
vars
being freed - in fact during both calls to DESTROY, $our1 and $our2
exist
and have a reference count >= 1.

What is actually happening is that at that point during global
destruction, perl is linearly scanning SV arenas looking for any RV
which
points to an object. Each such RV is undeffed (*not* freed), which may
trigger a destructor call on the now not-so-referred-to object.

Yes, it seems that these RVs are undeffed, but something with the order of DESTROY calls isn't right.

So its a bit like your code having this in it​:

END { undef $our1; undef $our2 }

except that the order is non-deterministic, so it could equally as
well be

END { undef $our2; undef $our1 }

There must be something else going on. With regard to destructors, the order of undeffing shouldn't matter. When inserted into my test program, both of the END blocks above result in the correct order of DESTROY calls.

Now having explained all that, I don't really understand your proposal
above. Given that package vars *aren't* freed at that point during
global
destruction, could you update and clarify your proposal?

I don't care when package vars are freed, I'm mostly concerned with the order in which DESTROY methods are called. I want to avoid DESTROY being invoked on an object that still has references pointing to it, unless it's absolutely necessary because of a reference cycle. Simply setting each package scalar to undef and clearing each package array or hash *should already* result in the correct order, but for some reason it doesn't. (Unless something was fixed since Perl 5.22.1.)

My proposal above only takes it one step further and makes sure that package vars don't lose their content too early when other destructors still might use them. As I understood Leon's ("Destructors can and do depend on package variables, this would not solve all non-circular cases.") and your comment ("But has just been pointed out, freeing package vars before calling destructors breaks everything."), this is desirable. But maybe this was a misunderstanding. As it seems, Perl simply undefs package vars in random order, so a destructor can't rely on their contents anyway. Personally, I don't really care about the exact behavior.

Another useful feature, especially for XS code, would be a mechanism to tell when DESTROY is invoked forcibly on an object that still has references during global destruction. In this case, calling destructors of an external C library could can lead to crashes. Right now, the best heuristic I could come up with is (PL_dirty && SvREFCNT(sv) > 1) which isn't completely reliable in my particular case.

@p5pRT
Copy link
Author

p5pRT commented Mar 17, 2017

From @nwellnhof

On Fri, 17 Mar 2017 14​:09​:51 -0700, wellnhofer@​aevum.de wrote​:

So its a bit like your code having this in it​:

END { undef $our1; undef $our2 }

except that the order is non-deterministic, so it could equally as
well be

END { undef $our2; undef $our1 }

There must be something else going on. With regard to destructors, the
order of undeffing shouldn't matter. When inserted into my test
program, both of the END blocks above result in the correct order of
DESTROY calls.

OK, now I understand. What's happening in my test program is essentially​:

END {
  undef $our1->{ref};
  undef $our2;
  # $our2->DESTROY is called.
  undef $our1;
  # $our1->DESTROY is called.
}

As Dave already explained, in an early stage of global destruction *every* blessed RV is undeffed in random order​: package var, hash value, whatever. So all bets are off. This seems so unpredictable, I wonder why we bother running destructors in the DESTRUCT phase at all.

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2017

From @iabyn

On Fri, Mar 17, 2017 at 02​:09​:52PM -0700, Nick Wellnhofer via RT wrote​:

My proposal above only takes it one step further and makes sure that
package vars don't lose their content too early when other destructors
still might use them.

Yes, but I still don't understand exactly what your proposal is. That is
to say, I read what you wrote, and couldn't, from your description, see
what I would have to change in the perl core to implement your proposal.
Which means I can't comment on whether your proposal is good or not.

--
More than any other time in history, mankind faces a crossroads. One path
leads to despair and utter hopelessness. The other, to total extinction.
Let us pray we have the wisdom to choose correctly.
  -- Woody Allen

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2017

From @demerphq

On 17 March 2017 at 23​:27, Nick Wellnhofer via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 17 Mar 2017 14​:09​:51 -0700, wellnhofer@​aevum.de wrote​:

So its a bit like your code having this in it​:

END { undef $our1; undef $our2 }

except that the order is non-deterministic, so it could equally as
well be

END { undef $our2; undef $our1 }

There must be something else going on. With regard to destructors, the
order of undeffing shouldn't matter. When inserted into my test
program, both of the END blocks above result in the correct order of
DESTROY calls.

OK, now I understand. What's happening in my test program is essentially​:

END {
undef $our1->{ref};
undef $our2;
# $our2->DESTROY is called.
undef $our1;
# $our1->DESTROY is called.
}

As Dave already explained, in an early stage of global destruction *every* blessed RV is undeffed in random order​: package var, hash value, whatever. So all bets are off. This seems so unpredictable, I wonder why we bother running destructors in the DESTRUCT phase at all.

I don't really understand why freeing the stash is any different from
freeing a hash of references. I would have thought that we start by
refcount decrementing the root of the stash, which would then triggers
a refcount cascade. Am I correct in understanding that we do something
different during tear down?

Or is this is a case where there are reference cycles, and we are in
the phase of destruct where everything left after the refcount
decrement cascade is hard killed?

Sorry if I am asking question already answered...

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2017

From @iabyn

On Wed, Mar 22, 2017 at 11​:46​:06AM +0100, demerphq wrote​:

I don't really understand why freeing the stash is any different from
freeing a hash of references. I would have thought that we start by
refcount decrementing the root of the stash, which would then triggers
a refcount cascade. Am I correct in understanding that we do something
different during tear down?

Or is this is a case where there are reference cycles, and we are in
the phase of destruct where everything left after the refcount
decrement cascade is hard killed?

No, we're discussing a relatively early phase of destruction. From
inspection of perl_destruct(), the order of the main actions are​:

* call END blocksd;
* exit the he top-level scope​: this is the implicit one round the
  main program (so "global" lexical vars are released);
* free the CV corresponding to the main program (think of it as the
  "main" eval, and evals are freed immediately after execution);
* set ${^GLOBAL_PHASE} to 'DESTRUCT';
* flush and pop PerlIO handles and layers;
* call destructors for any remaining objects (the subject of this
  discussion);
* disable warn and die hooks;
* call C-level Perl_call_atexit() functions
* Free registered MROs

At this point if PL_destruct_level == 0 and we're not a child thread,
stop; else continue​:

* free PL_regex_padav;
* free PL_stashcache;
* free lots of assorted interpreter bits and pieces;
* free %​::
* free all arena SVs by setting SVf_BREAK and artificially
  SvREFCNT_dec()ing each SV;
* free PL_strtab;
* if DEBUG_LEAKING_SCALARS, display any remaining arena SVs;
* free a few remaining miscellaneous things in the interpreter;
* free all stacks;
* free all SV arenas;

At the point we're discussing, we trigger the calling of DESTROY on all
remaining objects principally by undeffing all SvROK SVss which point to
such objects. This means that when DESTROY() is called, it sees
everything present and correct, except that some SVs may now be undef
rather than holding a pointer. In particular, all stashes, global vars,
global subs etc still exist at this point.

If we instead went for the approach of triggering destructors by freeing
%​::, then individual destructors would see a much more damaged
environment, and much more would be freed under PL_destruct_level == 0,
including most SVs, not just objects.

I *think* the topic of this thread is how to go about undeffing refs to
objects in a better order which undefs higher-level refs first.

--
Lear​: Dost thou call me fool, boy?
Fool​: All thy other titles thou hast given away; that thou wast born with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant