temporary objects created in returning statements are DESTROYed too late #14637

p5pRT · 2015-04-06T19:44:44Z

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

Searchable as RT124248$

p5pRT · 2015-04-06T19:44:44Z

From kazuhooku@gmail.com

Created by kazuhooku@gmail.com

use Scope::Guard;

sub bar {

!! new Scope::Guard(sub { warn "destroyed" });

}

sub baz {

warn "baz";

}

baz(bar());

In the code above, "baz" is printed before "destroyed". In other

words, the temporary object that is created and evaluated while

executing the last statement of subroutine `bar` is not DESTROYed

until the execution of the calling statement (in the caller)

completes.

I believe that this is a bug. The guard object created in `bar`

should be DESTROYed before `baz` gets called.

The issue caused degradations in one of the Perl modules I

maintain, which was reported as
https://rt.cpan.org/Ticket/Display.html?id=103299

and was worked around as
tokuhirom/Test-TCP@be316f1

Perl Info




Flags:

    category=core

    severity=low



Site configuration information for perl 5.16.2:


Configured by _mdnsresponder at Sun Aug 25 01:10:27 PDT 2013.


Summary of my perl5 (revision 5 version 16 subversion 2) configuration:



  Platform:

    osname=darwin, osvers=13.0, archname=darwin-thread-multi-2level

    uname='darwin jackson.apple.com 13.0 darwin kernel version 13.0.0: tue
jul 30 20:52:22 pdt 2013; root:xnu-2422.1.53~3release_x86_64 x86_64 '

    config_args='-ds -e -Dprefix=/usr -Dccflags=-g  -pipe  -Dldflags=
-Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none -Dcc=cc'

    hint=recommended, useposix=true, d_sigaction=define

    useithreads=define, usemultiplicity=define

    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef

    use64bitint=define, use64bitall=define, uselongdouble=undef

    usemymalloc=n, bincompat5005=undef

  Compiler:

    cc='cc', ccflags ='-arch x86_64 -arch i386 -g -pipe -fno-common
-DPERL_DARWIN -fno-strict-aliasing -fstack-protector -I/usr/local/include',

    optimize='-Os',

    cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing
-fstack-protector -I/usr/local/include'

    ccversion='', gccversion='4.2.1 Compatible Apple LLVM 5.0
(clang-500.0.68)', gccosandvers=''

    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678

    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16

    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8

    alignbytes=8, prototype=define

  Linker and Libraries:

    ld='cc -mmacosx-version-min=10.9', ldflags ='-arch x86_64 -arch i386
-fstack-protector -L/usr/local/lib'

    libpth=/usr/local/lib /usr/lib

    libs=

    perllibs=

    libc=, so=dylib, useshrplib=true, libperl=libperl.dylib

    gnulibc_version=''

  Dynamic Linking:

    dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '


Locally applied patches:

    /Library/Perl/Updates/<version> comes before system perl directories

    installprivlib and installarchlib points to the Updates directory

    CVE-2013-1667 hashtable DOS fix




@INC for perl 5.16.2:

    /Library/Perl/5.16/darwin-thread-multi-2level

    /Library/Perl/5.16

    /Network/Library/Perl/5.16/darwin-thread-multi-2level

    /Network/Library/Perl/5.16

    /Library/Perl/Updates/5.16.2/darwin-thread-multi-2level

    /Library/Perl/Updates/5.16.2

    /System/Library/Perl/5.16/darwin-thread-multi-2level

    /System/Library/Perl/5.16

    /System/Library/Perl/Extras/5.16/darwin-thread-multi-2level

    /System/Library/Perl/Extras/5.16

    .




Environment for perl 5.16.2:

    DYLD_LIBRARY_PATH (unset)

    HOME=/Users/oku.kazuho

    LANG=C

    LANGUAGE (unset)

    LD_LIBRARY_PATH (unset)

    LOGDIR (unset)


PATH=/Users/oku.kazuho/cocos2d-x-3.1/tools/cocos2d-console/bin:/usr/local/gcc-4.9.1/bin:/usr/local/node-0.10.26/bin:/Users/oku.kazuho/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin

    PERL_BADLANG (unset)

    SHELL=/bin/bash



--
Kazuho Oku

p5pRT · 2015-04-06T23:39:04Z

From @jkeenan

On Mon Apr 06 12:44:44 2015, kazuhooku@gmail.com wrote:

This is a bug report for perl from kazuhooku@gmail.com,

generated with the help of perlbug 1.39 running under perl 5.16.2.

-----------------------------------------------------------------

[Please describe your issue here]

use Scope::Guard;

sub bar {

!! new Scope::Guard(sub { warn "destroyed" });

}

sub baz {

warn "baz";

}

baz(bar());

In the code above, "baz" is printed before "destroyed". In other

words, the temporary object that is created and evaluated while

executing the last statement of subroutine `bar` is not DESTROYed

until the execution of the calling statement (in the caller)

completes.

I believe that this is a bug. The guard object created in `bar`

should be DESTROYed before `baz` gets called.

The issue caused degradations in one of the Perl modules I

maintain, which was reported as
https://rt.cpan.org/Ticket/Display.html?id=103299

and was worked around as
https://github.com/tokuhirom/Test-
TCP/commit/be316f1f8dd426afc4491da52b0803fc915405c9

1. Would it be possible for you to try this with perl-5.20 (or, better still, with blead)?

You reported with perl-5.16.2, which is no longer a supported version of perl.

2. Have you excluded the possibility that this is a bug in Scope::Guard? (I see that that distribution has not had a new release in nearly 5 years.)

Thank you very much.

[Please do not change anything below this line]

-----------------------------------------------------------------

---

Flags:

category=core

severity=low

---

Site configuration information for perl 5.16.2:

Configured by _mdnsresponder at Sun Aug 25 01:10:27 PDT 2013.

Summary of my perl5 (revision 5 version 16 subversion 2)
configuration:

Platform:

osname=darwin, osvers=13.0, archname=darwin-thread-multi-2level

uname='darwin jackson.apple.com 13.0 darwin kernel version 13.0.0: tue
jul 30 20:52:22 pdt 2013; root:xnu-2422.1.53~3release_x86_64 x86_64 '

config_args='-ds -e -Dprefix=/usr -Dccflags=-g -pipe -Dldflags=
-Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none
-Dcc=cc'

hint=recommended, useposix=true, d_sigaction=define

useithreads=define, usemultiplicity=define

useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef

use64bitint=define, use64bitall=define, uselongdouble=undef

usemymalloc=n, bincompat5005=undef

Compiler:

cc='cc', ccflags ='-arch x86_64 -arch i386 -g -pipe -fno-common
-DPERL_DARWIN -fno-strict-aliasing -fstack-protector
-I/usr/local/include',

optimize='-Os',

cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing
-fstack-protector -I/usr/local/include'

ccversion='', gccversion='4.2.1 Compatible Apple LLVM 5.0
(clang-500.0.68)', gccosandvers=''

intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678

d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16

ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8

alignbytes=8, prototype=define

Linker and Libraries:

ld='cc -mmacosx-version-min=10.9', ldflags ='-arch x86_64 -arch i386
-fstack-protector -L/usr/local/lib'

libpth=/usr/local/lib /usr/lib

libs=

perllibs=

libc=, so=dylib, useshrplib=true, libperl=libperl.dylib

gnulibc_version=''

Dynamic Linking:

dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '

Locally applied patches:

/Library/Perl/Updates/<version> comes before system perl directories

installprivlib and installarchlib points to the Updates directory

CVE-2013-1667 hashtable DOS fix

---

@INC for perl 5.16.2:

/Library/Perl/5.16/darwin-thread-multi-2level

/Library/Perl/5.16

/Network/Library/Perl/5.16/darwin-thread-multi-2level

/Network/Library/Perl/5.16

/Library/Perl/Updates/5.16.2/darwin-thread-multi-2level

/Library/Perl/Updates/5.16.2

/System/Library/Perl/5.16/darwin-thread-multi-2level

/System/Library/Perl/5.16

/System/Library/Perl/Extras/5.16/darwin-thread-multi-2level

/System/Library/Perl/Extras/5.16

.

---

Environment for perl 5.16.2:

DYLD_LIBRARY_PATH (unset)

HOME=/Users/oku.kazuho

LANG=C

LANGUAGE (unset)

LD_LIBRARY_PATH (unset)

LOGDIR (unset)

PATH=/Users/oku.kazuho/cocos2d-x-3.1/tools/cocos2d-
console/bin:/usr/local/gcc-4.9.1/bin:/usr/local/node-
0.10.26/bin:/Users/oku.kazuho/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin

PERL_BADLANG (unset)

SHELL=/bin/bash

--
Kazuho Oku

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

p5pRT · 2015-04-06T23:39:04Z

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

p5pRT · 2015-04-07T10:41:07Z

From @iabyn

On Mon, Apr 06, 2015 at 04:39:04PM -0700, James E Keenan via RT wrote:

1. Would it be possible for you to try this with perl-5.20 (or, better
still, with blead)?

You reported with perl-5.16.2, which is no longer a supported version of
perl.

2. Have you excluded the possibility that this is a bug in Scope::Guard?
(I see that that distribution has not had a new release in nearly 5
years.)

It can be reproduced in blead with this simple core-only code:

sub DESTROY { warn "destroyed" };
sub bar { !!(my $x = bless []); }
sub baz { warn "baz"; }
baz(bar());

The issue is that that we free temporaries at the end of each statement but
not at the end of of sub, because otherwise we could end up prematurely
freeing values that are being returned

I don't see that this is fixable.

--
Music lesson: a symbiotic relationship whereby a pupil's embellishments
concerning the amount of practice performed since the last lesson are
rewarded with embellishments from the teacher concerning the pupil's
progress over the corresponding period.

p5pRT · 2015-04-07T11:03:14Z

From @ap

* Dave Mitchell <davem@iabyn.com> [2015-04-07 12:45]:

It can be reproduced in blead with this simple core-only code:
sub DESTROY \{ warn "destroyed" \};
sub bar \{ \!\!$my $x = bless \[\]$; \}
sub baz \{ warn "baz"; \}
baz$bar\($\);
The issue is that that we free temporaries at the end of each
statement but not at the end of of sub, because otherwise we could end
up prematurely freeing values that are being returned

I don't see that this is fixable.

But the value returned has nothing to do with the value expected to be
freed. There are two boolean conversions and a reference separating the
two from each other. It seems like it ought to be entirely possible to
keep the value(s) the expression yields without keeping every variable
that was still valid at the beginning of the statement.

Is this another instance of the “stack isn’t refcounted” issue (or maybe
a close equivalent thereof)?

--
Aristotle Pagaltzis // <http://plasmasturm.org/>

p5pRT · 2015-04-07T12:51:55Z

From @Leont

On Tue, Apr 7, 2015 at 1:02 PM, Aristotle Pagaltzis <pagaltzis@gmx.de>
wrote:

The issue is that that we free temporaries at the end of each
statement but not at the end of of sub, because otherwise we could end
up prematurely freeing values that are being returned

I don't see that this is fixable.

But the value returned has nothing to do with the value expected to be
freed. There are two boolean conversions and a reference separating the
two from each other. It seems like it ought to be entirely possible to
keep the value(s) the expression yields without keeping every variable
that was still valid at the beginning of the statement.

Possible? Probably, but not without doing more work on every leavesub. And
even then it would have subtle end-user visible side-effects that may break
something (doesn't it always) because order of destruction of things now
changes.

Is this another instance of the “stack isn’t refcounted” issue (or maybe
a close equivalent thereof)?

Was it that predictable?

Leon

p5pRT · 2015-04-07T14:18:42Z

From @iabyn

On Tue, Apr 07, 2015 at 01:02:04PM +0200, Aristotle Pagaltzis wrote:

* Dave Mitchell <davem@iabyn.com> [2015-04-07 12:45]:
It can be reproduced in blead with this simple core-only code:
sub DESTROY \{ warn "destroyed" \};
sub bar \{ \!\!$my $x = bless \[\]$; \}
sub baz \{ warn "baz"; \}
baz$bar\($\);
The issue is that that we free temporaries at the end of each
statement but not at the end of of sub, because otherwise we could end
up prematurely freeing values that are being returned

I don't see that this is fixable.
But the value returned has nothing to do with the value expected to be
freed. There are two boolean conversions and a reference separating the
two from each other. It seems like it ought to be entirely possible to
keep the value(s) the expression yields without keeping every variable
that was still valid at the beginning of the statement.

Is this another instance of the “stack isn’t refcounted” issue (or maybe
a close equivalent thereof)?

Well its more an issue that we use the temps stack ("mortalised")to keep
things alive in lieu of the perl stack not being ref counted.

Consider the following. (-DR causes items which have a refcount other than
1 to be prefixed with <n> where n is the refcount, and by <T> if the temps
stack holds a reference to the item):

$ ./perl -DstR -e'$x = bless []; $y = $x'

...
(-e:1) anonlist
=> <T>\AV()
(-e:1) bless
=> <T>\AV()
(-e:1) gvsv(main::x)
=> <T>\AV() UNDEF
(-e:1) sassign
=> \<2>AV()
(-e:1) nextstate
=>
(-e:1) gvsv(main::x)
=> \AV()
...

anonlist creates a ref to an anonymous AV. That RV has an initial
refcount of 1, which is held by the temps stack. Later when we assign to
$x, we make $x into an RV, which also points to the AV (so the AV now has
a ref count of 2).

At this point the first RV is only accessible from the tmps stack.
The nextstate clears the temps stack back down to the current water mark,
which frees the RV, which decrements the ref count of the AV by one.
We see this when $x is next accessed: the AV now has a refcount of 1.

Not that all this has nothing to do with what we do with the return value
from bless; the very act of creating an anon array pushes a temp RV onto
the temps stack which will keep the AV alive until the next nextstate is
executed.

This is useful as otherwise something like [] in void context (or more
likely, [] returned by a sub which is called in void context) would leak.

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

p5pRT · 2015-04-07T21:16:08Z

From kazuhooku@gmail.com

Thank you for the quick response!

2015-04-07 8:39 GMT+09:00 James E Keenan via RT <perlbug-followup@perl.org>:

On Mon Apr 06 12:44:44 2015, kazuhooku@gmail.com wrote:

This is a bug report for perl from kazuhooku@gmail.com,

generated with the help of perlbug 1.39 running under perl 5.16.2.

-----------------------------------------------------------------

[Please describe your issue here]

use Scope::Guard;

sub bar {

!! new Scope::Guard(sub { warn "destroyed" });

}

sub baz {

warn "baz";

}

baz(bar());

In the code above, "baz" is printed before "destroyed". In other

words, the temporary object that is created and evaluated while

executing the last statement of subroutine `bar` is not DESTROYed

until the execution of the calling statement (in the caller)

completes.

I believe that this is a bug. The guard object created in `bar`

should be DESTROYed before `baz` gets called.

The issue caused degradations in one of the Perl modules I

maintain, which was reported as
https://rt.cpan.org/Ticket/Display.html?id=103299

and was worked around as
https://github.com/tokuhirom/Test-
TCP/commit/be316f1f8dd426afc4491da52b0803fc915405c9

1. Would it be possible for you to try this with perl-5.20 (or, better
still, with blead)?

You reported with perl-5.16.2, which is no longer a supported version of
perl.

The problem still persists on perl 5.20.2, output of perl -V is
https://gist.github.com/kazuho/2809149d5b716bf74103

2. Have you excluded the possibility that this is a bug in Scope::Guard?
(I see that that distribution has not had a new release in nearly 5 years.)

I believe that the cause of the issue is not Scope::Guard. The issue was
originally found as a TCP error (IO::Socket::IP not being DESTROYed as
expected).

A simple test script using IO::Socket::INET to reproduce the issue can be
found at https://gist.github.com/kazuho/e614ca99ab6699c37346 (fails on
5.16.2 and 5.20.2).

Thank you very much.

[Please do not change anything below this line]

-----------------------------------------------------------------

---

Flags:

category=core

severity=low

---

Site configuration information for perl 5.16.2:

Configured by _mdnsresponder at Sun Aug 25 01:10:27 PDT 2013.

Summary of my perl5 (revision 5 version 16 subversion 2)
configuration:

Platform:

osname=darwin, osvers=13.0, archname=darwin-thread-multi-2level

uname='darwin jackson.apple.com 13.0 darwin kernel version 13.0.0: tue
jul 30 20:52:22 pdt 2013; root:xnu-2422.1.53~3release_x86_64 x86_64 '

config_args='-ds -e -Dprefix=/usr -Dccflags=-g -pipe -Dldflags=
-Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none
-Dcc=cc'

hint=recommended, useposix=true, d_sigaction=define

useithreads=define, usemultiplicity=define

useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef

use64bitint=define, use64bitall=define, uselongdouble=undef

usemymalloc=n, bincompat5005=undef

Compiler:

cc='cc', ccflags ='-arch x86_64 -arch i386 -g -pipe -fno-common
-DPERL_DARWIN -fno-strict-aliasing -fstack-protector
-I/usr/local/include',

optimize='-Os',

cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing
-fstack-protector -I/usr/local/include'

ccversion='', gccversion='4.2.1 Compatible Apple LLVM 5.0
(clang-500.0.68)', gccosandvers=''

intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678

d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16

ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8

alignbytes=8, prototype=define

Linker and Libraries:

ld='cc -mmacosx-version-min=10.9', ldflags ='-arch x86_64 -arch i386
-fstack-protector -L/usr/local/lib'

libpth=/usr/local/lib /usr/lib

libs=

perllibs=

libc=, so=dylib, useshrplib=true, libperl=libperl.dylib

gnulibc_version=''

Dynamic Linking:

dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '

Locally applied patches:

/Library/Perl/Updates/<version> comes before system perl directories

installprivlib and installarchlib points to the Updates directory

CVE-2013-1667 hashtable DOS fix

---

@INC for perl 5.16.2:

/Library/Perl/5.16/darwin-thread-multi-2level

/Library/Perl/5.16

/Network/Library/Perl/5.16/darwin-thread-multi-2level

/Network/Library/Perl/5.16

/Library/Perl/Updates/5.16.2/darwin-thread-multi-2level

/Library/Perl/Updates/5.16.2

/System/Library/Perl/5.16/darwin-thread-multi-2level

/System/Library/Perl/5.16

/System/Library/Perl/Extras/5.16/darwin-thread-multi-2level

/System/Library/Perl/Extras/5.16

.

---

Environment for perl 5.16.2:

DYLD_LIBRARY_PATH (unset)

HOME=/Users/oku.kazuho

LANG=C

LANGUAGE (unset)

LD_LIBRARY_PATH (unset)

LOGDIR (unset)

PATH=/Users/oku.kazuho/cocos2d-x-3.1/tools/cocos2d-
console/bin:/usr/local/gcc-4.9.1/bin:/usr/local/node-

0.10.26/bin:/Users/oku.kazuho/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin

PERL_BADLANG (unset)

SHELL=/bin/bash

--
Kazuho Oku

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

--
Kazuho Oku

p5pRT · 2015-04-09T12:24:31Z

From kazuhooku@gmail.com

Hi,

Thank you all for your help.

2015-04-07 19:41 GMT+09:00 Dave Mitchell via RT <perlbug-followup@perl.org>:

It can be reproduced in blead with this simple core-only code:
sub DESTROY \{ warn "destroyed" \};
sub bar \{ \!\!$my $x = bless \[\]$; \}
sub baz \{ warn "baz"; \}
baz$bar\($\);
The issue is that that we free temporaries at the end of each statement but
not at the end of of sub, because otherwise we could end up prematurely
freeing values that are being returned

I don't see that this is fixable.

As Aristotle pointed out, the object is evaluated and converted to boolean
by the ! operator before going out of `bar`, so theoretically is should be
possible to fix.

I do not believe that this issue should necessarily be fixed, however, it
seems very surprising to me; it is hard to expect such a behavior from how
the document is written. Perlobj says:

When the last reference to an object goes away, the object is destroyed.
If you only have one reference to an object stored in a lexical scalar,
the
object is destroyed when that scalar goes out of scope. If you store the
object in a package global, that object may not go out of scope until the
program exits.

In case of `$x` in your example, I suspect that for a novice user it would
be natural to expect that `$x` gets DESTOYed when going out from `bar`,
since such an object that only "has one reference to an object stored in a
lexical scalar, the object is destroyed when that scalar goes out of scope."

Would it be possible to document how and when objects are pushed/popped
from the temps stack, if this issue is not going to get fixed?

--
Kazuho Oku

p5pRT · 2015-04-09T13:45:42Z

From @iabyn

On Thu, Apr 09, 2015 at 04:38:17PM +0900, Kazuho Oku wrote:

2015-04-07 19:41 GMT+09:00 Dave Mitchell via RT <perlbug-followup@perl.org>:
It can be reproduced in blead with this simple core-only code:
sub DESTROY \{ warn "destroyed" \};
sub bar \{ \!\!$my $x = bless \[\]$; \}
sub baz \{ warn "baz"; \}
baz$bar\($\);
The issue is that that we free temporaries at the end of each statement but
not at the end of of sub, because otherwise we could end up prematurely
freeing values that are being returned

I don't see that this is fixable.
As Aristotle pointed out, the object is evaluated and converted to boolean
by the ! operator before going out of `bar`, so theoretically is should be
possible to fix.

And as I pointed out, an extra ref to the anon array is created and added
to the temps stack *before the assignment to $x, or the execution of the
two negations.

The difficulty is in ensuring that

1) in something like this for example

$x = ([], {})[$i];

that the unused an array or hash doesn't leak;

2) in something like

return [];

that the anon array isn't prematurely freed before it gets a chance to
be returned to the caller and assigned somewhere.

These two goals are more or less in conflict with each other. The temps
stack is the current solution to this general issue.

Many bugs are theoretically fixable. That doesn't necessarily make thm
practical to fix.

I do not believe that this issue should necessarily be fixed, however, it
seems very surprising to me; it is hard to expect such a behavior from how
the document is written. Perlobj says:

When the last reference to an object goes away, the object is destroyed.
If you only have one reference to an object stored in a lexical scalar,
the
object is destroyed when that scalar goes out of scope. If you store the
object in a package global, that object may not go out of scope until the
program exits.

In case of `$x` in your example, I suspect that for a novice user it would
be natural to expect that `$x` gets DESTOYed when going out from `bar`,
since such an object that only "has one reference to an object stored in a
lexical scalar, the object is destroyed when that scalar goes out of scope."

Would it be possible to document how and when objects are pushed/popped
from the temps stack, if this issue is not going to get fixed?

Not really. At a quick grep, there are at least 500 places in the core code
that push things onto the temps stack. Trying to describe all that behaviour
in a simple paragraph would be tricky.

--
I don't want to achieve immortality through my work...
I want to achieve it through not dying.
-- Woody Allen

p5pRT · 2015-04-10T04:05:21Z

From kazuhooku@gmail.com

Dave,

Thank you for the response.

2015-04-09 22:45 GMT+09:00 Dave Mitchell <davem@iabyn.com>:

On Thu, Apr 09, 2015 at 04:38:17PM +0900, Kazuho Oku wrote:
2015-04-07 19:41 GMT+09:00 Dave Mitchell via RT <perlbug-followup@perl.org>:
It can be reproduced in blead with this simple core-only code:
sub DESTROY \{ warn "destroyed" \};
sub bar \{ \!\!$my $x = bless \[\]$; \}
sub baz \{ warn "baz"; \}
baz$bar\($\);
The issue is that that we free temporaries at the end of each statement but
not at the end of of sub, because otherwise we could end up prematurely
freeing values that are being returned

I don't see that this is fixable.
As Aristotle pointed out, the object is evaluated and converted to boolean
by the ! operator before going out of `bar`, so theoretically is should be
possible to fix.
And as I pointed out, an extra ref to the anon array is created and added
to the temps stack *before the assignment to $x, or the execution of the
two negations.

The difficulty is in ensuring that

1) in something like this for example
$x = $\[\]\, \{\}$\[$i\];
that the unused an array or hash doesn't leak;

2) in something like
return \[\];
that the anon array isn't prematurely freed before it gets a chance to
be returned to the caller and assigned somewhere.

These two goals are more or less in conflict with each other. The temps
stack is the current solution to this general issue.

Many bugs are theoretically fixable. That doesn't necessarily make thm
practical to fix.

I agree.

I do not believe that this issue should necessarily be fixed, however, it
seems very surprising to me; it is hard to expect such a behavior from how
the document is written. Perlobj says:

When the last reference to an object goes away, the object is destroyed.
If you only have one reference to an object stored in a lexical scalar,
the
object is destroyed when that scalar goes out of scope. If you store the
object in a package global, that object may not go out of scope until the
program exits.

In case of `$x` in your example, I suspect that for a novice user it would
be natural to expect that `$x` gets DESTOYed when going out from `bar`,
since such an object that only "has one reference to an object stored in a
lexical scalar, the object is destroyed when that scalar goes out of scope."

Would it be possible to document how and when objects are pushed/popped
from the temps stack, if this issue is not going to get fixed?

Not really. At a quick grep, there are at least 500 places in the core code
that push things onto the temps stack. Trying to describe all that behaviour
in a simple paragraph would be tricky.

Thank you for the explanation. If it is hard to describe how it
works, would it be possible to document _when_ there is a guarantee
that the objects would be destroyed?

The reason why I ask is the fact that people tend to use DESTORY for
certain critical tacks (e.g. unlocking a lock or committing/rolling
back a database transaction). It is considered a good approach
because people believe that there is a guarantee that the callback is
called when execution goes out of scope.

However the truth in case of Perl seems to be that there are
exceptions; if so, I believe that it should be documented somehow or
applications should be written without using RAII pattern.

Consider the following example, foo() tries to lock a file, and does
something if successful. To me it is natural for to expect that if
foo() is called subsequently it would succeed in locking the file
under the premise that there are no other processes trying to lock the
file at the same time. However the truth is not the case, in case
foo() is called twice in a single statement more than once, the lock
would fail for the succeeding calls.

sub trylockfn {
my $fn = shift;
open my $fh, '>>', $fn
or die "failed to open file:$fn:$!";
if (! flock $fh, LOCK_EX | LOCK_NB) {
return; # not locked
}
$fh;
}

sub foo {
if (my $lock = trylockfn("/tmp/trylock")) {
warn "lock obtained!";
# do something
return 1;
}
}

sub bar {
...
}

bar(foo(), foo());

The example is indeed artificial and may seem unrealistic, but please
note that I reported this issue because it was a problem in the
real-world code.

--
I don't want to achieve immortality through my work...
I want to achieve it through not dying.
-- Woody Allen

--
Kazuho Oku

p5pRT · 2015-04-20T12:09:03Z

From @iabyn

On Fri, Apr 10, 2015 at 01:04:34PM +0900, Kazuho Oku wrote:

Thank you for the explanation. If it is hard to describe how it
works, would it be possible to document _when_ there is a guarantee
that the objects would be destroyed?

Ok, how about something like the following added to the end of the

=head2 Destructors

section in perlobj?

Note that perl often creates temporary references to newly created
things to avoid them being prematurely freed. For example in
C<$a = []>, a temporary reference to the anonymous array is created so
that it isn't freed before it gets assigned to $a. Temporary
references created in the current scope are normally freed at
statement boundaries. So in C<$a = []; ...>, the array is created with
a temporary reference count of 1; the assignment creates a second
reference to it; then just after the assignment statement completes,
temporaries are freed and the recount is reduced to 1.

This becomes significant where there isn't a statement boundary, such
as the last statement of a subroutine (or a return statement). Here
the temporary reference may live on until the next statement boundary
in the caller.

--
Wesley Crusher gets beaten up by his classmates for being a smarmy git,
and consequently has a go at making some friends of his own age for a
change.
-- Things That Never Happen in "Star Trek" #18

p5pRT · 2016-02-03T15:22:04Z

From @iabyn

On Thu, Apr 09, 2015 at 02:45:02PM +0100, Dave Mitchell wrote:

The difficulty is in ensuring that

1) in something like this for example
$x = $\[\]\, \{\}$\[$i\];
that the unused an array or hash doesn't leak;

2) in something like
return \[\];
that the anon array isn't prematurely freed before it gets a chance to
be returned to the caller and assigned somewhere.

These two goals are more or less in conflict with each other. The temps
stack is the current solution to this general issue.

Many bugs are theoretically fixable. That doesn't necessarily make thm
practical to fix.

But I've fixed it anyway :-)

Commit f7a874b, part of my work on revamping the context system.
Will appear in 5.23.8 and then 5.24.0. Not suitable for backporting.

--
Overhead, without any fuss, the stars were going out.
-- Arthur C Clarke

p5pRT · 2016-02-03T15:23:41Z

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

p5pRT · 2016-05-13T04:02:49Z

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.

With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

p5pRT · 2016-05-13T04:02:50Z

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

p5pRT closed this as completed May 13, 2016

p5pRT added Severity Low distro-darwin lexical/scope/closure labels Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

temporary objects created in returning statements are DESTROYed too late #14637

temporary objects created in returning statements are DESTROYed too late #14637

p5pRT commented Apr 6, 2015

p5pRT commented Apr 6, 2015

p5pRT commented Apr 6, 2015

p5pRT commented Apr 6, 2015

p5pRT commented Apr 7, 2015

p5pRT commented Apr 7, 2015

p5pRT commented Apr 7, 2015

p5pRT commented Apr 7, 2015

p5pRT commented Apr 7, 2015

p5pRT commented Apr 9, 2015

p5pRT commented Apr 9, 2015

p5pRT commented Apr 10, 2015

p5pRT commented Apr 20, 2015

p5pRT commented Feb 3, 2016

p5pRT commented Feb 3, 2016

p5pRT commented May 13, 2016

p5pRT commented May 13, 2016

temporary objects created in returning statements are DESTROYed too late #14637

temporary objects created in returning statements are DESTROYed too late #14637

Comments

p5pRT commented Apr 6, 2015

p5pRT commented Apr 6, 2015

From kazuhooku@gmail.com

Created by kazuhooku@gmail.com

p5pRT commented Apr 6, 2015

From @jkeenan

p5pRT commented Apr 6, 2015

p5pRT commented Apr 7, 2015

From @iabyn

p5pRT commented Apr 7, 2015

From @ap

p5pRT commented Apr 7, 2015

From @Leont

p5pRT commented Apr 7, 2015

From @iabyn

p5pRT commented Apr 7, 2015

From kazuhooku@gmail.com

p5pRT commented Apr 9, 2015

From kazuhooku@gmail.com

p5pRT commented Apr 9, 2015

From @iabyn

p5pRT commented Apr 10, 2015

From kazuhooku@gmail.com

p5pRT commented Apr 20, 2015

From @iabyn

p5pRT commented Feb 3, 2016

From @iabyn

p5pRT commented Feb 3, 2016

p5pRT commented May 13, 2016

From @khwilliamson

p5pRT commented May 13, 2016