DESTROY not called on code reference objects #2028
Comments
From [email protected]Created by [email protected]This might be related to my previous report. Consider the following program: #!/opt/perl/bin/perl -w use strict; sub new { DESTROY { my $i = "Hello"; __END__ This will not print anything at all, indicating that the DESTROY method If we change 1) such that new() is called with a closure, for instance my $exit = main -> new (sub {$i}); # 1) DESTROY is called as soon as $exit goes out of scope. Abigail Perl Info |
From @schwern(From perlbug 20000530.008)
-- Michael G. Schwern <schwern@pobox.com> http://www.pobox.com/~schwern/ |
From @simoncozensCreated by [email protected]This works: { my $x = new Flow sub {print "Eat this, Hook::Scope\n" }} package Flow; This doesn't: { my $x = new Flow sub {print "Eat this, Hook::Scope\n" }} package Flow; DESTROY is called in the first place but not in the second. This Perl Info |
From @iabynAbout a year ago, Simon Cozens reported a bug to the effect that { my $x = bless sub { }, 'X'; } Doesn't call the destructor. Having looked at it, the reason is that the anon sub isn't a closure Turning it into a closure makes it work: my $y; Simon mentions that "either the documentation or the code are wrong". Dave. -- |
From @schwernOn Sun, Aug 17, 2003 at 11:07:15PM +0100, Dave Mitchell wrote:
Since it is unnecessarily surprising and inconsistent behavior, and since Don't document bugs as features. You will hate yourself in the morning. -- |
From @ysthOn Sun, Aug 17, 2003 at 05:27:51PM -0700, Michael G Schwern wrote:
My initial reaction is to disagree. On second thought, I'm not so perl -wle'sub cv { sub {} }; my $x = bless cv, 'X'; print ref cv' either the bless should fail or the second call to cv should get a new |
From @simoncozensYitzchak Scott-Thoennes:
My initial reaction is that calling a destructor on a sub that happens -- |
From @iabynOn Sun, Aug 17, 2003 at 05:27:51PM -0700, Michael G Schwern wrote:
Note that I used the word 'misfeature' rather than 'feature'. So I was -- |
From @iabynOn Mon, Aug 18, 2003 at 05:45:29PM +0100, Simon Cozens wrote:
Assuming that we wish to keep the optimisation of sharing non-closure anon -- |
From [email protected]On Aug 18, Dave Mitchell wrote:
It seems very strange to me to say that an anonymous sub with It's great that we have the optimization of not cloning CV's Remember the etymology of "closure." It's from set theory. - Kurt |
From @chipdudeAccording to Simon Cozens:
Because every value of $a = sub { print "hi" } is the same CV. Printing $a will confirm this. The original owner of sub hi { print "hi" } The only difference (WRT GC) is that in the \&hi case the original In contrast, every value of my $foo; is a _new_ CV; printing $a will confirm this. Therefore there is no |
From @nwc10On Mon, Aug 18, 2003 at 05:50:16PM -0400, Chip Salzenberg wrote:
Oh yes. Mmm. Arguably action at a distance: $ perl -le 'sub foo { return sub {}}; $a = foo; $b = foo; print ref $b; bless $a; print ref $b' Nicholas Clark |
From @iabynOn Mon, Aug 18, 2003 at 05:49:16PM -0400, Kurt Starsinic wrote:
Well, I've never been very conversant with set theory, so I've never Dave. PS - I've been having some sendmail problems this afternoon, so replies to -- |
From @chipdudeAccording to Kurt Starsinic:
Etymology doesn't define usage, it merely illuminates its origins. And yes that makes me a total descriptivist. But how can a Perl person |
From @RandalSchwartz
Kurt> It seems very strange to me to say that an anonymous sub with Well, we need a term for that then. I've been adovcating "closure" But if you have a better term (that doesn't take more than three If "all subroutines are closures", then the term loses its usefulness -- |
From [email protected]On Monday, August 18, 2003, at 07:12 pm, Dave Mitchell wrote:
COW? Arthur |
From @iabynOn Tue, Aug 19, 2003 at 05:29:09PM +0100, Arthur Bergman wrote:
AFAIKT, any COW scheme would break the following: my $sub = sub { print "hello" }; -- |
From [email protected]On Aug 19, Randal L. Schwartz wrote:
I suggest that it's a closure when the language supports So it seems to me that, in Perl, a named subroutine is never a
I'm with you 100% there, Randal. - Kurt |
From [email protected]On Wednesday, August 20, 2003, at 04:31 am, Kurt Starsinic wrote:
I would say an anonymous subroutine is just that, an anonymous However, I also think this issue is totally irrelevant since only a Arthur |
From @iabynOn Tue, Aug 19, 2003 at 11:31:48PM -0400, Kurt Starsinic wrote:
But named subs also capture their lexical state at creation time, X.pm: my $x1 = X->new('x1'); print "in main\n"; gizmo [d]$ ./p Here, f captures $x2 and so stops it being destroyed at the end of Similar comments aply to, eg { -- |
From @iabynOn Wed, Aug 20, 2003 at 07:12:22PM +0100, Nick Ing-Simmons wrote:
Run it with -w and you get: Variable "$foo" will not stay shared at /tmp/p line 6. Subs capture their lexical environment at creation time. When fred is D. -- |
From [email protected]Dave Mitchell <davem@fdgroup.com> writes:
Things get confusing: sub harry harry('Ouch'); fred();
|
From [email protected]Created by [email protected]perl -wle 'sub DESTROY { print "DESTROY" } my $a = bless sub {}; $a=undef' The DESTROY never gets called. Perl Info |
From @iabynOn Sun, Oct 17, 2004 at 09:10:49PM -0000, perl-5. 8. 0 @ ton. iguana. be wrote:
This has been discussed before. When the anon sub isn't a closure, the eg $ perl -le'push @a, sub {} for 1..5; print $_ for @a' There doen't seem to be any way of fixing this without impacting the Dave. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @nwc10On Mon, Oct 18, 2004 at 05:02:13PM +0100, Dave Mitchell wrote:
Is it possible to retrospectively change a sub to be shared in the bless Nicholas Clark |
From [email protected]Nicholas Clark <nick@ccl4.org> wrote
Won't that make the reference point to a new CV? Will it break something my $a = sub { ... }; Or is there an extra level of indirection? I certainly think this bug ought to be fixed (and am slightly surprised that Mike Guy |
From @davidnicoli don't think this matters. When do you need to catch destruction of an anonymous coderef that is Objects based on coderefs tend to be closures, so they can have their own We could document the shared aspect in the documentation on DESTROY if On Mon, 18 Oct 2004 18:40:03 +0100, Mike Guy <mjtg@cam.ac.uk> wrote:
-- |
From [email protected]In article <934_64a2041018233558_d94e3@mail.gmail.com>,
The optimization for non-anonymous closures seems sensible, but it |
From @davidnicolHere's i guess what you're asking for, if these lines were added to Imagine adding these lines to perltoot, and wince: Perl's notion of the right time to call a destructor is not On Tue, 19 Oct 2004 08:48:17 +0000 (UTC), Ton Hospel
-- |
From @iabynOn Mon, May 01, 2006 at 09:05:30PM +0300, Yuval Kogman wrote:
I should imagine it's copying the scratchpad that takes most of the time. -- |
From @nwc10On Mon, May 01, 2006 at 06:49:41PM +0100, Dave Mitchell wrote:
So the problem is that there are two pairs of two in: $ perl -lwe 'sub foo { my $q = sub {}; ($q, $q) } my @a = foo(); my @b = foo; print "@a / @b"' but currently it's not clear which they are :-) I like the idea of the real copies but shared scratchpads. I hope it's viable. Nicholas Clark |
From @nothingmuchOn Mon, May 01, 2006 at 20:55:21 +0100, Dave Mitchell wrote:
Why is it copied? I guessed (in my prior email) that it's not -- |
From @iabynOn Tue, May 02, 2006 at 03:23:16AM +0300, Yuval Kogman wrote:
Every sub has its own scratchpad. The scratchpad contains pointers to: The only exception to this are closureless anon subs which share the The problem with splitting off the closure vars from the scratchpad in -- |
From @iabynOn Mon, May 01, 2006 at 07:55:21PM +0100, Dave Mitchell wrote:
I've knocked up a proof-of-concept patch (it fails loads of tests, but The timings I get are as follows: orig perl (share CV and scratchpad): 0m2.292s sub {} for 1..10_000_000 hacked perl (copy CV, but share scratchpad): 0m6.564s sub {} for 1..10_000_000 orig perl, but full closure (CV and scratchpad copied): 0m18.229s my $x; sub {$x} for 1..10_000_000 However, there's a problem with copying the CV while sharing the pad: push @s, sub { my $x=1; $s[0] && &{pop @s}; print "x=$x\n" } for 1,2; which gives: x=1 whereas in normal perl it gives x=1 this is becuase with the CV being shared, they share CvDEPTH and thus -- |
|
@rspier - Status changed from 'open' to 'deleted' |
|
@ikegami - Status changed from 'deleted' to 'open' |
From @ikegamiTicket wasn't reopened after being mistakenly reported as spam. Fixed. |
From @dolmenLe Dim. Ao�. 17 15:07:59 2003, davem a �crit�:
What we are seeing here is that "sub {}" is a kind of shared constant. Blessing multiple times the same value multiple time can only lead to So I think that the proper fix for this issue would be that bless die To mitigate issues with existing code (but I would be interested to see Olivier. |
From [Unknown Contact. See original ticket]Le Dim. Ao�. 17 15:07:59 2003, davem a �crit�:
What we are seeing here is that "sub {}" is a kind of shared constant. Blessing multiple times the same value multiple time can only lead to So I think that the proper fix for this issue would be that bless die To mitigate issues with existing code (but I would be interested to see Olivier. |
From [email protected]Olivier Mengu?? via RT wrote:
No, it has perfectly well-defined behaviour, and can reasonably be
I get "Modification of a read-only value attempted" if I try, say, It would be vaguely sensible for non-closure subs to be marked readonly It's not sensible to change the existing behaviour of blessing subs. -zefram |
From @FGasperCreated by @FGasperConsider the following: =================== use strict; sub new { sub DESTROY { #---------------------------------------------------------------------- package main; use strict; sub finally1 { sub finally2 { finally1();
|
From @cpansproutOn Mon Aug 18 18:29:12 2014, felipe@felipegasper.com wrote:
This is a known longstanding bug (#3306) that is not easy to fix without severe performance impact. (I’m merging this with that ticket.) -- Father Chrysostomos |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutAh! I found it! This is the ticket I was looking for. I discussed this same issue in <20141028053445.4478.qmail@lists-nntp.develooper.com>. The ideas that I thought were my own have already been suggested and discussed here. :-) On Sat May 06 15:55:35 2006, davem@iabyn.com wrote:
With my hacked-up perl on the sprout/padlist-sharing branch (which fails about 20 test scripts), I get: orig perl (share CV and scratchpad): 0m0.980s sub {} for 1..10_000_000 hacked perl (copy CV, but share scratchpad): 0m3.130s sub {} for 1..10_000_000 orig perl, but full (dis)closure (CV and scratchpad copied): 0m9.695s my $x; sub {$x} for 1..10_000_000 So sub{} that returns a new sub each time is only a third of the speed. Is that acceptable? I think the most compelling reason for fixing this is that the behaviour is different under the debugger. If one’s code is not working, but it just magically does the right thing in the debugger, that’s scary action at a distance, making these sorts of problems hard to track down.
That problem I don’t have because I followed your suggestion of putting CvDEPTH in the padlist. -- Father Chrysostomos |
From @iabynOn Wed, Oct 29, 2014 at 09:38:43PM -0700, Father Chrysostomos via RT wrote:
"There is nothing new under the sun" :-)
Well, 'sub {} for 1..10_000_000' is a bit of a contrived benchmark; in the my $r = { foo => 1 }; which represents a fairly minimal non-closure anon sub that does from: 3.339s a slowdown of 20%. I think this would be at the margins of acceptability.
I think the fact that blessing one sub blesses them all is a fairly -- |
From [email protected]On Mon, Nov 3, 2014 at 7:36 AM, Dave Mitchell <davem@iabyn.com> wrote:
|
|
What I do see however is that just about every luminary in the perl community that I know of has commented on it, and/or been bitten by the consequences of this. As have I. @iabyn suggested that if we did fix this it would slow down construction of anonymous subs by about 20%. Someone else suggested we make it so that bless detects it is blessing an anonymous sub that is not enclosing vars and refuse to bless it. ysth pointed out this is problematic, as people often do bless such subs. Multiple people suggested we document it an move on. I think that most people see code like as a constructor for a new sub. However apparently when the sub is not a closure that is not what it does, and instead it ends up being closer to this code: I think we should resolve these tickets once and for all. As far as I can tell we have three options:
My personal feeling is that the best option is 3, I am not convinced that a 20% slowdown in constructing anonymous non-closure subs is a cost anyone should care about. I do not understand the optree well enough yet to make a patch myself. It looks like @cpansprout produced one which had pretty good performance, which is still in the repo (at least in mine) as sprout/padlist-sharing. But i could live with 2. I don't think 1 is a good idea. I think most people would bump into this issue /then/ read the docs and find it. I also kinda wonder if we could "think outside of the box" here, and maybe make a "light" wrapper around true SVt_CODE (maybe via SVt_PVLV or something like it) so that such subs get an extra wrapper, which is what gets blessed. Thus we could keep the optimization but be able to bless the wrapper independently. It really feels to me like we can and should solve this. |
|
BTW, I pushed yves/sprout-padlist-sharing as a rebase of sprout/padlist-sharing. It doesnt pass test, but then i guess that is expected. I would sure love a patch that does fix this (IE make it so anonymous closures and non-closures are treated the same) so we could benchmark it and assess what the impact might be of fixing it. |
|
This hasnt been resolved, so I am reopening. |
Migrated from rt.perl.org#3306 (status was 'open')
Searchable as RT3306$
The text was updated successfully, but these errors were encountered: