On Wed Mar 21 20:31:53 2012, tonyc wrote: > +0.5 as a blocker. > > Either fixing the base problem, In addition to this way of triggering the HEK leak: $ PERL_DESTRUCT_LEVEL=1 ./perl -Ilib -e 'BEGIN { $^H{foo} = bar } our %FIELDS; my main $x; eval q"$x->{foo}"' Unbalanced string table refcount: (1) for "foo" during global destruction. we have these: $ PERL_DESTRUCT_LEVEL=1 ./perl -Ilib -e 'use warnings FATAL => all; BEGIN { $^H{foo} = "bar" } eval q"exec q/foo/; return"' Unbalanced string table refcount: (1) for "foo" during global destruction. $ PERL_DESTRUCT_LEVEL=1 ./perl -Ilib -e 'use warnings FATAL => all; BEGIN { $^H{foo} = "bar" } eval q"foo(); sub foo($){}"' Unbalanced string table refcount: (1) for "foo" during global destruction. Any kind of croak in Perl_finalize_optree causes the entire op tree to leak. Since eval now copies the hint hash into the root of the op tree (in newSTATEOP), we end up leaking HEKs that before a24d89c9b4c1 were not attached to the op tree. So the leak is slightly bigger, but the HEKs are just showing us that the op tree leak was occurring to begin with. This is the third time I’ve wished we had a way to disable savestack items. The other two times I found workarounds (resulting in some very fragile code in pp_entereval). But this time there doesn’t appear to be any. To be less vague, it would be nice if we could say: token = SAVEFREEOP_token(o); finalize_op(o); /* now we are safe */ unsave(token); /* disable/pop savestack items */ We could accomplish the equivalent for now with a temporary plaster in Perl_finalize_optree (tests are still running locally; no failures yet): diff --git a/op.c b/op.c index 2ffe10f..9fe4b84 100644 --- a/op.c +++ b/op.c @@ -1496,14 +1496,17 @@ the tree thread-safe. void Perl_finalize_optree(pTHX_ OP* o) { + I32 i = PL_savestack_ix; PERL_ARGS_ASSERT_FINALIZE_OPTREE; + SAVEFREEOP(o); ENTER; SAVEVPTR(PL_curcop); finalize_op(o); LEAVE; + PL_savestack_ix = i; } STATIC void __END__ which, although a bit of a hack, looks safe enough. However, it causes this to crash: package Foo; use fields qw(a b); sub new { my $class = shift; return fields::new($class); } my Foo $f = Foo->new; $f->{c} = 1; __END__ and I haven’t yet figured out why. It crashes when the SAVEFREEOP is not cancelled out by the PL_savestack_ix assignment. But, if it can be made to work, does this seem like a good idea? > or reducing the likelihood of the > warning back to 5.14 levels (somehow.) That would involve moving the new strict hints in %^H into $^H, which could affect CPAN modules that make assumptions about %^H that have already been updated for 5.16, though that’s unlikely. The unknowns bother me. I still don’t know which is the best way forward. -- Father Chrysostomos --- via perlbug: queue: perl5 status: open https://rt.perl.org:443/rt3/Ticket/Display.html?id=111462Thread Previous | Thread Next