Skip Menu |

This queue is for tickets about the namespace-clean CPAN distribution.

Report information
The Basics
Id: 73402
Status: resolved
Priority: 0/
Queue: namespace-clean

People
Owner: Nobody in particular
Requestors: 'spro^^*%*^6ut# [...] &$%*c
ANDK [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 0.22



Subject: Bleadperl v5.15.5-306-gb50b205 breaks RIBASUSHI/namespace-clean-0.21.tar.gz
Subject: Problems with tying %^H
namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes when cloning a tied hint hash: use Devel::Hide 'B::Hooks::EndOfScope'; use sort "stable"; use namespace::clean; use sort "stable"; {;} I haven’t written tests to prove it, but I believe there are other problems with it, too: It does not save the existing values in %^H before tying, so whatever keys are there will appear to vanish from the point of view of other BEGIN blocks in the same scope. Tying hint hashes can’t really work, because tied hashes do not really have elements. When you fetch a hash element, you get a magical scalar that calls methods on the tie object, but that scalar is not really a hash element. Hint hashes work by attaching magic to their elements, so the underlying he chain is modified when the hash is modified. Tie magic intercepts the usual element lookup, so you never get to the magic hint hash element. Hence, anything assigned to the tied hash will fail to ‘stick’ and will not show in caller() output at run time. I don’t think it’s even possible to make tied hint hashes work in any useful way. I suspect that you are using a tied hash instead of an object in one of the keys in order to avoid having string eval hang on to that object. In 5.14, you can solve that by tying a single element of the hint hash (instead of the whole hash). Try running the script below. Notice that even with the eval "" destruction happens before the ‘exited’ warning. Unfortunately, it doesn’t work in 5.12 and earlier, due to a tie bug. I haven’t been able to come up with a way to make it work. package bomb; sub TIESCALAR { $_[1]||bless [] } sub DESTROY { warn "destroying $_[0]" } sub FETCH { $_[0][0] } sub STORE { $_[0][0] = $_[1] } package main; { BEGIN { $^H{foo}=bar; # activate localisation magic tie $^H{foo}, 'bomb'; } BEGIN { warn 'about to exit scope' } eval "" } BEGIN { warn 'exited'; }
Subject: Re: [rt.cpan.org #73402] Problems with tying %^H
Date: Tue, 20 Dec 2011 20:07:37 -0600
To: Father Chrysostomos via RT <bug-namespace-clean [...] rt.cpan.org>
From: Jesse Luehrs <doy [...] tozt.net>
On Tue, Dec 20, 2011 at 08:59:16PM -0500, Father Chrysostomos via RT wrote: Show quoted text
> namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes when cloning a tied hint > hash: > > use Devel::Hide 'B::Hooks::EndOfScope'; > use sort "stable"; > use namespace::clean; > use sort "stable"; > {;} > > I haven’t written tests to prove it, but I believe there are other problems with it, too: It does > not save the existing values in %^H before tying, so whatever keys are there will appear to > vanish from the point of view of other BEGIN blocks in the same scope.
$ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN { warn $^H{foo} }' Warning: something's wrong at -e line 1. $ perl -E'BEGIN { $^H{foo} = 1 } BEGIN { warn $^H{foo} }' 1 at -e line 1. and with B::Hooks::EndOfScope installed: $ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN { warn $^H{foo} }' 1 at -e line 1. So yeah, this is broken. -doy
Subject: Re: [rt.cpan.org #73402] Problems with tying %^H
Date: Tue, 20 Dec 2011 20:16:54 -0600
To: Jesse Luehrs via RT <bug-namespace-clean [...] rt.cpan.org>
From: Jesse Luehrs <doy [...] tozt.net>
On Tue, Dec 20, 2011 at 09:07:46PM -0500, Jesse Luehrs via RT wrote: Show quoted text
> Queue: namespace-clean > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=73402 > > > On Tue, Dec 20, 2011 at 08:59:16PM -0500, Father Chrysostomos via RT wrote:
> > namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes when cloning a tied hint > > hash: > > > > use Devel::Hide 'B::Hooks::EndOfScope'; > > use sort "stable"; > > use namespace::clean; > > use sort "stable"; > > {;} > > > > I haven’t written tests to prove it, but I believe there are other problems with it, too: It does > > not save the existing values in %^H before tying, so whatever keys are there will appear to > > vanish from the point of view of other BEGIN blocks in the same scope.
> > $ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN { warn $^H{foo} }' > Warning: something's wrong at -e line 1. > $ perl -E'BEGIN { $^H{foo} = 1 } BEGIN { warn $^H{foo} }' > 1 at -e line 1. > > and with B::Hooks::EndOfScope installed: > > $ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN { warn $^H{foo} }' > 1 at -e line 1. > > So yeah, this is broken.
And, since 5.15.6 modifies "use strict" to use the hint hash, this will break "use strict; use namespace::clean;", which is a fairly serious issue. -doy
On Tue Dec 20 21:17:02 2011, doy@tozt.net wrote: Show quoted text
> On Tue, Dec 20, 2011 at 09:07:46PM -0500, Jesse Luehrs via RT wrote:
> > Queue: namespace-clean > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=73402 > > > > > On Tue, Dec 20, 2011 at 08:59:16PM -0500, Father Chrysostomos via RT
> wrote:
> > > namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes
> when cloning a tied hint
> > > hash: > > > > > > use Devel::Hide 'B::Hooks::EndOfScope'; > > > use sort "stable"; > > > use namespace::clean; > > > use sort "stable"; > > > {;} > > > > > > I haven’t written tests to prove it, but I believe there are other
> problems with it, too: It does
> > > not save the existing values in %^H before tying, so whatever keys
> are there will appear to
> > > vanish from the point of view of other BEGIN blocks in the same
> scope.
> > > > $ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN {
> warn $^H{foo} }'
> > Warning: something's wrong at -e line 1. > > $ perl -E'BEGIN { $^H{foo} = 1 } BEGIN { warn $^H{foo} }' > > 1 at -e line 1. > > > > and with B::Hooks::EndOfScope installed: > > > > $ perl -E'BEGIN { $^H{foo} = 1 } use namespace::clean; BEGIN {
> warn $^H{foo} }'
> > 1 at -e line 1. > > > > So yeah, this is broken.
> > And, since 5.15.6 modifies "use strict" to use the hint hash, this > will > break "use strict; use namespace::clean;", which is a fairly serious > issue.
That won’t be broken. $^H is still the canonical place used in the core for strictures. If the strict hints are missing from %^H, it means that strict was enabled implicitly by ‘use 5.012’, so it can be disabled implicitly by a lower version declaration.
Subject: Re: [rt.cpan.org #73402] Problems with tying %^H
Date: Wed, 21 Dec 2011 01:20:40 -0600
To: Father Chrysostomos via RT <bug-namespace-clean [...] rt.cpan.org>
From: Jesse Luehrs <doy [...] tozt.net>
On Wed, Dec 21, 2011 at 02:16:25AM -0500, Father Chrysostomos via RT wrote: Show quoted text
> On Tue Dec 20 21:17:02 2011, doy@tozt.net wrote:
> > And, since 5.15.6 modifies "use strict" to use the hint hash, this > > will > > break "use strict; use namespace::clean;", which is a fairly serious > > issue.
> > That won’t be broken. $^H is still the canonical place used in the core for strictures. If the > strict hints are missing from %^H, it means that strict was enabled implicitly by ‘use 5.012’, > so it can be disabled implicitly by a lower version declaration.
Ah, okay. But still, this means that use strict; use namespace::clean; { use 5.010; ...; } breaks, which is still something of a big deal. -doy
On Tue Dec 20 20:59:15 2011, SPROUT wrote: Show quoted text
> In 5.14, you can solve that by tying a single element of the hint hash > (instead of the whole > hash). Try running the script below. Notice that even with the eval > "" destruction happens > before the ‘exited’ warning. Unfortunately, it doesn’t work in 5.12 > and earlier, due to a tie > bug. I haven’t been able to come up with a way to make it work. > > package bomb; > sub TIESCALAR { $_[1]||bless [] } > sub DESTROY { warn "destroying $_[0]" } > sub FETCH { $_[0][0] } > sub STORE { $_[0][0] = $_[1] } > > package main; > > { > BEGIN { > $^H{foo}=bar; # activate localisation magic > tie $^H{foo}, 'bomb'; > } > > BEGIN { warn 'about to exit scope' } > > eval "" > } > BEGIN { warn 'exited'; }
And I tried another method, hoping it would work back to 5.10, but it doesn’t even work on bleadperl, and I don’t know why: use Hash::Util::FieldHash; my %hh; BEGIN { &Hash::Util::FieldHash::fieldhash(\%hh); } package bomb; sub DESTROY { warn "destroying $_[0]" } package main; { BEGIN { $^H{foo}='bar'; # activate localisation magic $hh{\%^H} = bless [], 'bomb'; } BEGIN { warn 'about to exit scope' } eval "" } BEGIN { warn 'exited'; }
Merging with the ticket discussing underlying issue.
RT-Send-CC: doy [...] tozt.net
On Tue Dec 20 20:59:15 2011, SPROUT wrote: Show quoted text
> namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes when > cloning a tied hint > hash: > > use Devel::Hide 'B::Hooks::EndOfScope'; > use sort "stable"; > use namespace::clean; > use sort "stable"; > {;} > > I haven’t written tests to prove it, but I believe there are other > problems with it, too: It does > not save the existing values in %^H before tying, so whatever keys are > there will appear to > vanish from the point of view of other BEGIN blocks in the same scope.
This looks like the actual problem - I simply forgot to do the proper cloning myself. Doing so passes on all 5.10+ perls. The cloner of 5.8.any still dies a fiery death, I am trying to figure out a trick for this. In any case - I think the result is rather satisfactory on 5.10+, and far from "is broken/can't be made to work". What do you guys think? :) diff --git a/lib/namespace/clean.pm b/lib/namespace/clean.pm index 84d203e..65f9567 100644 --- a/lib/namespace/clean.pm +++ b/lib/namespace/clean.pm @@ -19,7 +19,7 @@ BEGIN { # when changing also change in Makefile.PL my $b_h_eos_req = '0.07'; - if (eval { + if (! $ENV{NAMESPACE_CLEAN_USE_PP} and eval { require B::Hooks::EndOfScope; B::Hooks::EndOfScope->VERSION($b_h_eos_req); 1 @@ -80,7 +80,9 @@ EOE push @$stack, namespace::clean::_ScopeGuard->arm(shift); } else { + my %old_contents = %^H; tie( %^H, 'namespace::clean::_TieHintHash', namespace::clean::_ScopeGuard->arm(shift) ); + %^H = %old_contents if %old_contents; } } diff --git a/t/09-fiddle-hinthash.t b/t/09-fiddle-hinthash.t new file mode 100644 index 0000000..c37112b --- /dev/null +++ b/t/09-fiddle-hinthash.t @@ -0,0 +1,36 @@ +use strict; +use warnings; + +use Test::More 0.88; + +{ + package Bar; + use sort 'stable'; + use namespace::clean; + use sort 'stable'; + { + 1; + } + + Test::More::pass('no segfault'); +} + +{ + package Foo; + BEGIN { + $^H{'foo'} = 'bar'; + } + + use namespace::clean; + + BEGIN { + Test::More::is( $^H{'foo'}, 'bar', 'hinthash intact' ); + } + + { + 1; + } +} + + +done_testing;
RT-Send-CC: doy [...] tozt.net
In fact I was trivially able to stop perl 5.8 from segfaulting too. Shipped n::c 0.21_01 for smoke trials, if no objections will ship this as 0.22 proper within couple of days. Relevant changes and test: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace-clean.git;a=commitdiff;h=656ec55b51c454ca3d8a06a728a4f6c06d6db5d5
On Wed Dec 21 05:16:28 2011, RIBASUSHI wrote: Show quoted text
> On Tue Dec 20 20:59:15 2011, SPROUT wrote:
> > namespace::clean tickles a bug in perl 5.10-5.15.6, which crashes when > > cloning a tied hint > > hash: > > > > use Devel::Hide 'B::Hooks::EndOfScope'; > > use sort "stable"; > > use namespace::clean; > > use sort "stable"; > > {;} > > > > I haven’t written tests to prove it, but I believe there are other > > problems with it, too: It does > > not save the existing values in %^H before tying, so whatever keys are > > there will appear to > > vanish from the point of view of other BEGIN blocks in the same scope.
> > This looks like the actual problem - I simply forgot to do the proper > cloning myself. Doing so passes on all 5.10+ perls. The cloner of > 5.8.any still dies a fiery death, I am trying to figure out a trick for > this. > > In any case - I think the result is rather satisfactory on 5.10+, and > far from "is broken/can't be made to work". What do you guys think? :)
It’s a lot better, but there’s still a snag. Due to another perl bug, still present in blead, the hints become invisible to inner scopes at compile time, but not run time. BTW, I was wrong about ties preventing the hint magic from coming into play. Devel::Peek shows that the magically generated elements have both tie *and* hint magic. use Data::Dumper; sub dump_runtime_hints { print Dumper +(caller 0)[10]; warn ' ' } BEGIN { $ENV{NAMESPACE_CLEAN_USE_PP} = 1; %^H = 'a'..'h'; } use namespace::clean; BEGIN { print Dumper \%^H; warn ' '; } dump_runtime_hints; { BEGIN { print Dumper \%^H; warn ' '; } dump_runtime_hints; } __END__ $VAR1 = { 'e' => 'f', 'c' => 'd', 'a' => 'b', 'g' => 'h' }; at - line 14. $VAR1 = {}; at - line 20. $VAR1 = { 'e' => 'f', 'c' => 'd', 'a' => 'b', 'g' => 'h' }; at - line 4. $VAR1 = { 'e' => 'f', 'c' => 'd', 'a' => 'b', 'g' => 'h' }; at - line 4.
On Wed Dec 21 11:39:11 2011, SPROUT wrote: Show quoted text
> > It’s a lot better, but there’s still a snag. Due to another perl bug, > still present in blead, the > hints become invisible to inner scopes at compile time, but not run > time.
I am confused... isn't this the *documented* behavior of %^H? Can you perhaps make an addition to t/09-fiddle-hinthash.t that illustrates the problem better? Show quoted text
> use Data::Dumper; > sub dump_runtime_hints { > print Dumper +(caller 0)[10]; warn ' ' > } > > BEGIN { > $ENV{NAMESPACE_CLEAN_USE_PP} = 1; > %^H = 'a'..'h'; > } > use namespace::clean; > BEGIN { > print Dumper \%^H; ***1*** > warn ' '; > } > dump_runtime_hints; ***3*** > { > BEGIN { > print Dumper \%^H; ***2*** > warn ' '; > } > dump_runtime_hints; ***4*** > } > __END__ > $VAR1 = { > 'e' => 'f', > 'c' => 'd', > 'a' => 'b', > 'g' => 'h' > }; > at - line 14. > $VAR1 = {}; > at - line 20. > $VAR1 = { > 'e' => 'f', > 'c' => 'd', > 'a' => 'b', > 'g' => 'h' > }; > at - line 4. > $VAR1 = { > 'e' => 'f', > 'c' => 'd', > 'a' => 'b', > 'g' => 'h' > }; > at - line 4.
On Wed Dec 21 12:08:16 2011, RIBASUSHI wrote: Show quoted text
> On Wed Dec 21 11:39:11 2011, SPROUT wrote:
> > > > It’s a lot better, but there’s still a snag. Due to another perl bug, > > still present in blead, the > > hints become invisible to inner scopes at compile time, but not run > > time.
> > I am confused... isn't this the *documented* behavior of %^H?
I’m afraid I know this so well at this point that I’m confused as to what you could be confused about. :-) The hints set in the BEGIN block or in any code it calls apply to the scope surrounding it. Any inner compile-time scopes in *that* scope obviously have to inherit hints, otherwise this wouldn’t work: BEGIN { require "sort.pm"; sort::->import("stable") } # stable sort here { # stable sort here, too } Therefore, since BEGIN sees the compile-time hints of its surrounding scope in %^H, the second BEGIN block here should see the hints set in the first: BEGIN { require "sort.pm"; sort::->import("stable") } # stable sort here { # stable sort here, too BEGIN { use Data::Dumper; print Dumper \%^H } } __END__ $VAR1 = { 'sort' => 256 }; Show quoted text
> Can you > perhaps make an addition to t/09-fiddle-hinthash.t that illustrates the > problem better?
See the attachment. Where ‘perl’ is 5.10.1 (later versions should produce the same results): $ perl -Ilib t/09-fiddle-hinthash.t ok 1 - hinthash intact ok 2 - compile-time hinthash intact in inner scope ok 3 - hinthash values visible in caller ok 4 - no segfault 1..4 $ NAMESPACE_CLEAN_USE_PP=1 perl -Ilib t/09-fiddle-hinthash.t ok 1 - hinthash intact not ok 2 - compile-time hinthash intact in inner scope # Failed test 'compile-time hinthash intact in inner scope' # at t/09-fiddle-hinthash.t line 32. # got: undef # expected: 'bar' ok 3 - hinthash values visible in caller ok 4 - no segfault 1..4 # Looks like you failed 1 test of 4. Again, I don’t know of a way to fix this in 5.12-, but tying a single element will fix it in 5.14. If I get to it, your current approach will work in 5.15.7.
Subject: open_qg0dExGk.txt
diff -rup namespace-clean-0.21_01-pGIR9v/t/09-fiddle-hinthash.t namespace-clean-0.21_01-pGIR9v-copy/t/09-fiddle-hinthash.t --- namespace-clean-0.21_01-pGIR9v/t/09-fiddle-hinthash.t 2011-12-21 03:33:16.000000000 -0800 +++ namespace-clean-0.21_01-pGIR9v-copy/t/09-fiddle-hinthash.t 2011-12-21 09:50:30.000000000 -0800 @@ -28,6 +28,11 @@ use Test::More 0.88; } { + BEGIN { + Test::More::is( + $^H{'foo'}, 'bar', 'compile-time hinthash intact in inner scope' + ); + } 1; }
On Wed Dec 21 13:00:33 2011, SPROUT wrote: Show quoted text
> Again, I don’t know of a way to fix this in 5.12-, but tying a single > element will fix it in 5.14. > If I get to it, your current approach will work in 5.15.7.
It’s fixed in blead with cb1f05e, so namespace::clean as it is currently will work in 5.15.7 without B::Hooks::EndOfScope installed, but not in any earlier version. (I havent’t actually tested 5.8, so I could be wrong.) Perl_hv_copy_hints_hv was checking for the number of real keys in the hash (i.e., the number it had before it was tied), which doesn’t apply to tied hashes. It was skipping the copying for apparently empty hashes. So by putting %^H = () before the tie, you were preventing the values from propagating to inner scopes. But the underlying he-chain structure, which is what Perl actually uses at run time, was being copied. The two (the hh and the he-chain) were getting out of sync. So, for 5.14, you can tie a single element of the hint hash, as I demonstrated earlier. For 5.15.7, you don’t have to make any changes. Tying either a single element or the whole thing will work. For 5.12- you have to choose the least of several weevils: • Require B::Hooks::EndOfScope. • Allow string eval to break end-of-scope detection ($^H{foo} = $guard). • Allow the hh and the he-chain to get out of sync, which will work *most* of the time, as most hint-manipulation code just sets or deletes things, without reading them. • Warst ava: Use a source filter.
On Wed Dec 21 02:26:43 2011, SPROUT wrote: Show quoted text
> And I tried another method, hoping it would work back to 5.10, but it > doesn’t even work on > bleadperl, and I don’t know why: > > use Hash::Util::FieldHash; > > my %hh; > BEGIN { &Hash::Util::FieldHash::fieldhash(\%hh); } > > package bomb; > sub DESTROY { warn "destroying $_[0]" } > > package main; > > { > BEGIN { > $^H{foo}='bar'; # activate localisation magic > $hh{\%^H} = bless [], 'bomb'; > } > > BEGIN { warn 'about to exit scope' } > > eval "" > } > BEGIN { warn 'exited'; }
You’re not going to believe what I’ve discovered. This is even crazier than tying the hint hash to begin with. Hash::Util::FieldHash is not deleting elements in void context. When you call delete() in non- void context, a mortal scalar is returned. A mortal scalar is one whose reference count decreases at the end of the current statement. During scope exit, ‘statement’ is not clearly defined, so more scope unwinding could happen before the mortal gets freed. (As long as the stack is not reference-counted, I don’t think we can fix that safely, as there could be code relying on it.) By forcing the deletion into void context, one can overcome that problem, so this approach works back to 5.10: package hhfh; use Tie::Hash; BEGIN { @ISA = Tie::StdHash;} sub DELETE { SUPER::DELETE{@_}; # didn’t know this trick, did you? 1; # put the preceding statement in void context } use Hash::Util::FieldHash; my %hh; BEGIN { &Hash::Util::FieldHash::fieldhash(\%hh); tie %hh, 'hhfh'; } package bomb; sub DESTROY { warn "destroying $_[0]" } package main; { BEGIN { $^H{foo}='bar'; # activate localisation magic $hh{\%^H} = bless [], 'bomb'; } BEGIN { warn 'about to exit scope' } eval "" } BEGIN { warn 'exited'; } __END__ $ pbpaste|perl about to exit scope at - line 26. destroying bomb=ARRAY(0x82b2e0) at - line 15. exited at - line 30.
On Thu Dec 22 02:03:19 2011, SPROUT wrote: Show quoted text
> On Wed Dec 21 02:26:43 2011, SPROUT wrote:
> > And I tried another method, hoping it would work back to 5.10, but
> it
> > doesn’t even work on > > bleadperl, and I don’t know why: > > > > use Hash::Util::FieldHash; > > > > my %hh; > > BEGIN { &Hash::Util::FieldHash::fieldhash(\%hh); } > > > > package bomb; > > sub DESTROY { warn "destroying $_[0]" } > > > > package main; > > > > { > > BEGIN { > > $^H{foo}='bar'; # activate localisation magic > > $hh{\%^H} = bless [], 'bomb'; > > } > > > > BEGIN { warn 'about to exit scope' } > > > > eval "" > > } > > BEGIN { warn 'exited'; }
> > You’re not going to believe what I’ve discovered. This is even > crazier than tying the hint hash > to begin with. > > Hash::Util::FieldHash is not deleting elements in void context. When > you call delete() in non- > void context, a mortal scalar is returned. A mortal scalar is one > whose reference count > decreases at the end of the current statement. During scope exit, > ‘statement’ is not clearly > defined, so more scope unwinding could happen before the mortal gets > freed. > > (As long as the stack is not reference-counted, I don’t think we can > fix that safely, as there > could be code relying on it.) > > By forcing the deletion into void context, one can overcome that > problem, so this approach > works back to 5.10: > > package hhfh; > use Tie::Hash; > BEGIN { @ISA = Tie::StdHash;} > sub DELETE { > SUPER::DELETE{@_}; # didn’t know this trick, did you? > 1; # put the preceding statement in void context > } > > use Hash::Util::FieldHash; > my %hh; > BEGIN { > &Hash::Util::FieldHash::fieldhash(\%hh); > tie %hh, 'hhfh'; > } > > package bomb; > sub DESTROY { warn "destroying $_[0]" } > > package main; > > { > BEGIN { > $^H{foo}='bar'; # activate localisation magic > $hh{\%^H} = bless [], 'bomb'; > } > > BEGIN { warn 'about to exit scope' } > > eval "" > } > BEGIN { warn 'exited'; } > __END__ > > $ pbpaste|perl > about to exit scope at - line 26. > destroying bomb=ARRAY(0x82b2e0) at - line 15. > exited at - line 30. >
More good news: %^H doesn’t propagate into string eval in 5.8, so the classic object-in-the- hint-hash approach works. Behold, a patch attached hereto!
Subject: big patch.txt
diff -rup namespace-clean-0.21_01-pGIR9v/lib/namespace/clean.pm namespace-clean-0.21_01-pGIR9v-copy/lib/namespace/clean.pm --- namespace-clean-0.21_01-pGIR9v/lib/namespace/clean.pm 2011-12-21 03:36:49.000000000 -0800 +++ namespace-clean-0.21_01-pGIR9v-copy/lib/namespace/clean.pm 2011-12-21 23:28:09.000000000 -0800 @@ -26,70 +26,86 @@ BEGIN { } ) { B::Hooks::EndOfScope->import('on_scope_end'); } - else { + elsif ($] < 5.009_999_9) { eval <<'PP' or die $@; - use Tie::Hash (); - { - package namespace::clean::_TieHintHash; + package namespace::clean::_ScopeGuard; use warnings; use strict; - use base 'Tie::ExtraHash'; + sub arm { bless [ $_[1] ] } + + sub DESTROY { $_[0]->[0]->() } } + + sub on_scope_end (&) { + $^H |= 0x020000; + + if( my $stack = $^H{'namespace::clean/g'} ) { + push @$stack, namespace::clean::_ScopeGuard->arm(shift); + } + else { + $^H{'namespace::clean/g'} = + namespace::clean::_ScopeGuard->arm(shift); + } + } + + 1; + +PP + + } + else { + eval <<'PP10' or die $@; + + use Tie::Hash (); + use Hash::Util::FieldHash (); + { - package namespace::clean::_ScopeGuard; + # Hash::Util::FieldHash does not delete elements in void context, so + # they live on a bit longer on the mortals stack. By tying it and + # overriding DELETE, we can force the deletion into void context. + package namespace::clean::_TieHintHashFieldHash; + use base 'Tie::StdHash'; + sub DELETE { + SUPER::DELETE{@_}; + 1; # put the preceding statement in void context + } + } - use warnings; - use strict; + { + package namespace::clean::_ScopeGuard; sub arm { bless [ $_[1] ] } sub DESTROY { $_[0]->[0]->() } } + Hash::Util::FieldHash::fieldhash my %hh; sub on_scope_end (&) { $^H |= 0x020000; - if( my $stack = tied( %^H ) ) { - if ( (my $c = ref $stack) ne 'namespace::clean::_TieHintHash') { - die <<EOE; -======================================================================== - !!! F A T A L E R R O R !!! - - foreign tie() of %^H detected -======================================================================== - -namespace::clean is currently operating in pure-perl fallback mode, because -your system is lacking the necessary dependency B::Hooks::EndOfScope $b_h_eos_req. -In this mode namespace::clean expects to be able to tie() the hinthash %^H, -however it is apparently already tied by means unknown to the tie-class -$c - -Since this is a no-win situation execution will abort here and now. Please -try to find out which other module is relying on hinthash tie() ability, -and file a bug for both the perpetrator and namespace::clean, so that the -authors can figure out an acceptable way of moving forward. + if (!tied %hh) { + tie(%hh, 'namespace::clean::_TieHintHashFieldHash'); + } -EOE - } - push @$stack, namespace::clean::_ScopeGuard->arm(shift); + my $guard = namespace::clean::_ScopeGuard->arm(shift); + + if( my $stack = $hh{\%^H} ) { + push @$stack, $guard; } else { - my %old_contents = %^H; - %^H = (); - tie( %^H, 'namespace::clean::_TieHintHash', namespace::clean::_ScopeGuard->arm(shift) ); - $^H{$_} = $old_contents{$_} for keys %old_contents; + $hh{\%^H} = $guard; } } 1; -PP +PP10 } } diff -rup namespace-clean-0.21_01-pGIR9v/t/09-fiddle-hinthash.t namespace-clean-0.21_01-pGIR9v-copy/t/09-fiddle-hinthash.t --- namespace-clean-0.21_01-pGIR9v/t/09-fiddle-hinthash.t 2011-12-21 03:33:16.000000000 -0800 +++ namespace-clean-0.21_01-pGIR9v-copy/t/09-fiddle-hinthash.t 2011-12-21 23:25:18.000000000 -0800 @@ -28,6 +28,11 @@ use Test::More 0.88; } { + BEGIN { + Test::More::is( + $^H{'foo'}, 'bar', 'compile-time hinthash intact in inner scope' + ); + } 1; }
On Thu Dec 22 02:31:10 2011, SPROUT wrote: Show quoted text
> More good news: %^H doesn’t propagate into string eval in 5.8, so the > classic object-in-the- > hint-hash approach works.
Shipped https://metacpan.org/release/RIBASUSHI/namespace-clean-0.21_02 with all proposed tricks included. I factored things apart a little, so mere mortals can actually read this stuff ;) Also added you to authors, the current incarnation of n::c would not have been possible without your help. Thank you very much for getting to the bottom of this! http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace-clean.git;a=commitdiff;h=b2e54862c91ba576736e1889a84733d11cb1b675 Cheers
Once I finally wrapped my head around your patch I simplified it to arrive at this: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace-clean.git;a=blob;f=lib/namespace/clean/_PP_OSE.pm Please check the comment and code for correctness when time permits. Cheers!
On Thu Dec 22 18:31:06 2011, RIBASUSHI wrote: Show quoted text
> Once I finally wrapped my head around your patch I simplified it to > arrive at this: > > http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace- > clean.git;a=blob;f=lib/namespace/clean/_PP_OSE.pm > > Please check the comment and code for correctness when time permits.
Local firewalls prevent me from looking at that URL. If the code is what is in RIBASUSHI/namespace-clean-0.21_02.tar.gz, it looks correct. When I wrote the patch, I was trying to preserve the existing coding style at least somewhat, and I wondered at the complexity of the code, not realising it was because the thing behind tied(%^H) has to be an object, not just a plain array. Your simplified version in the tarball is a lot clearer. The comments in the tarball are just copied from what I wrote. I don’t know whether you have changed them since, but there is a slight mistake in what I wrote: # Hash::Util::FieldHash is not deleting elements in void context. When # you call delete() in non-void context, a mortal scalar is returned. A # mortal scalar is one whose reference count decreases at the end of the # current statement. During scope exit, ‘statement’ is not clearly # defined, so more scope unwinding could happen before the mortal gets # freed. I don’t know for certain, but my observations suggest that an entire compilation unit counts as one ‘statement’, as far as the mortals stack goes. Items mortalised during *compile-time* scope exit are local to that compilation unit, so they are not freed until that code’s run time (or probably UNITCHECK time, but I haven’t checked). If you change the comment to say the following, I think it will be accurate enough: ‘At compile time, an entire compilation unit counts as one “statement”, so the mortal will not be freed until the current file or eval finishes compiling.’
On Thu Dec 22 19:08:19 2011, SPROUT wrote: Show quoted text
> On Thu Dec 22 18:31:06 2011, RIBASUSHI wrote:
> > Once I finally wrapped my head around your patch I simplified it to > > arrive at this: > > > > http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace- > > clean.git;a=blob;f=lib/namespace/clean/_PP_OSE.pm > > > > Please check the comment and code for correctness when time permits.
> > Local firewalls prevent me from looking at that URL. If the code is > what is in > RIBASUSHI/namespace-clean-0.21_02.tar.gz, it looks correct.
Actually no, I reduced it even further throwing away the destructor entirely. Let me know if this one is just as sane (it passes smokes): package # hide from the pauses namespace::clean::_PP_OSE; use warnings; use strict; use Tie::Hash; use Hash::Util::FieldHash 'fieldhash'; # Here we rely on a combination of several behaviors: # # * %^H is deallocated on scope exit, so any references to it disappear # * A lost weakref in a fieldhash causes the corresponding key to be deleted # * Deletion of a key on a tied hash triggers DELETE # # Therefore the DELETE of a tied fieldhash containing a %^H reference will # be the hook to fire all our callbacks. fieldhash my %hh; { package # hide from pause too namespace::clean::_TieHintHashFieldHash; use base 'Tie::StdHash'; sub DELETE { my $ret = shift->SUPER::DELETE(@_); $_->() for @$ret; $ret; } } sub on_scope_end (&) { $^H |= 0x020000; tie(%hh, 'namespace::clean::_TieHintHashFieldHash') unless tied %hh; push @{ $hh{\%^H} ||= [] }, shift; } 1;
On Fri Dec 23 06:31:40 2011, RIBASUSHI wrote: Show quoted text
> On Thu Dec 22 19:08:19 2011, SPROUT wrote:
> > On Thu Dec 22 18:31:06 2011, RIBASUSHI wrote:
> > > Once I finally wrapped my head around your patch I simplified it to > > > arrive at this: > > > > > > http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=p5sagit/namespace- > > > clean.git;a=blob;f=lib/namespace/clean/_PP_OSE.pm > > > > > > Please check the comment and code for correctness when time permits.
> > > > Local firewalls prevent me from looking at that URL. If the code is > > what is in > > RIBASUSHI/namespace-clean-0.21_02.tar.gz, it looks correct.
> > Actually no, I reduced it even further throwing away the destructor > entirely. Let me know if this one is just as sane (it passes smokes):
Well, now I’m having a ‘why didn’t I think of that?’ moment. I was so close to the problem (trying to trigger destructors) that I didn’t see the obvious. What you have should certainly work. Show quoted text
> > package # hide from the pauses > namespace::clean::_PP_OSE; > > use warnings; > use strict; > > use Tie::Hash; > use Hash::Util::FieldHash 'fieldhash'; > > # Here we rely on a combination of several behaviors: > # > # * %^H is deallocated on scope exit, so any references to it disappear > # * A lost weakref in a fieldhash causes the corresponding key to be deleted > # * Deletion of a key on a tied hash triggers DELETE > # > # Therefore the DELETE of a tied fieldhash containing a %^H reference will > # be the hook to fire all our callbacks. > > fieldhash my %hh; > { > package # hide from pause too > namespace::clean::_TieHintHashFieldHash; > use base 'Tie::StdHash'; > sub DELETE { > my $ret = shift->SUPER::DELETE(@_); > $_->() for @$ret; > $ret; > } > } > > sub on_scope_end (&) { > $^H |= 0x020000; > > tie(%hh, 'namespace::clean::_TieHintHashFieldHash') > unless tied %hh; > > push @{ $hh{\%^H} ||= [] }, shift; > } > > 1;