develooper Front page | perl.perl5.porters | Postings from March 2012

[perl #111462] refcount warnings from C<use strict> C<our %FIELDS>

Thread Previous | Thread Next
From:
Father Chrysostomos via RT
Date:
March 26, 2012 16:45
Subject:
[perl #111462] refcount warnings from C<use strict> C<our %FIELDS>
Message ID:
rt-3.6.HEAD-4610-1332805494-224.111462-15-0@perl.org
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=111462

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About