develooper Front page | perl.perl5.porters | Postings from April 2006

[perl #8902] REFCNT wrong on tied hash object following assignment to keys()

From:
Steve Peters via RT
Date:
April 17, 2006 11:37
Subject:
[perl #8902] REFCNT wrong on tied hash object following assignment to keys()
Message ID:
rt-3.0.11-8902-132332.17.709662826019@perl.org
> [RT_System - Wed Apr 03 02:00:26 2002]:
> 
> 
> It turns out that following lvalue 'keys', the REFCNT is wrong on
> hashes even when they are not tied or objects.  For example:
> 
>         sub D::new { bless [] => 'D' }
>         sub D::DESTROY { print "Destroying $_[0]\n" }
> 
>         {
>                 my %h = (0 => D->new);
>         #       keys %h = 64;         
>         }
>         print "%h and D should have been destroyed by now.\n";
> 
>         END { print "Start of global destruction phase\n" }
> 
> With 5.7.3, the output here is 
> 
>         Destroying D=ARRAY(0x80f7b0c)
>         %h and D should have been destroyed by now.
>         Start of global destruction phase
> 
> as you would expect.  But if you uncomment the 'keys' assignment, you
> get different results:
> 
>         %h and D should have been destroyed by now.
>         Start of global destruction phase
>         Destroying D=ARRAY(0x80f7b0c)
> 
> %h is not cleaned up at the end of the block as it should be; instead,
> it lives on until the interpreter thread exits and performs a global
> destruction.  
> 
> The reason is simple:  Doing an lvalue 'keys' on a hash increments the
> reference count on the hash.  Why?  I don't know.  I took out the
> REFCNT_inc: 
> 
>     --- doop.c	2002/04/03 17:22:42	1.1
>     +++ doop.c	2002/04/03 17:33:33
>     @@ -1314,7 +1314,7 @@
>                 if (LvTARG(TARG) != (SV*)keys) {
>                     if (LvTARG(TARG))
>                         SvREFCNT_dec(LvTARG(TARG));
>     -		LvTARG(TARG) = SvREFCNT_inc(keys);
>     +		LvTARG(TARG) = keys;
>                 }
>                 PUSHs(TARG);
>                 RETURN;
> 
> and now the bug is gone and all the tests still pass.
> 
> But I'm worried.  Clearly someone put that SvREFCNT_inc() in for a
> reason, and maybe it's a reason I don't understand.  I couldn't find
> the patch that did this; I only have patches back to @9671.  The
> change was introduced sometime between 5.004_04 and 5.005_02.  Can
> anyone think of what it's doing?
> 
> If not, I suggest that it be removed; as I mentioned, all the other
> tests still pass.  Patch below.
> 
> 
> --- t/op/ref.t	2002/04/03 17:50:46	1.1
> +++ t/op/ref.t	2002/04/03 17:56:44
> @@ -5,7 +5,7 @@
>      @INC = qw(. ../lib);
>  }
>  
> -print "1..62\n";
> +print "1..63\n";
>  
>  require 'test.pl';
>  
> @@ -315,9 +315,19 @@
>    print "# expected \"$expect\", got \"$result\"\n";
>  }
>  
> +# test bug in lvalue 'keys' which incorrectly incremented the refcount
> +# in the hash [ID 20020402.001] REFCNT wrong on tied hash object
> +# following assignment to keys()
> +# 20020403 mjd-perl-patch+@plover.com
> +sub mjd::DESTROY {$mjd::ok = 1}
> +{ my %h = (0 => (bless [] => 'mjd'));
> +  keys %h = 64;
> +} 
> +print $mjd::ok ? "ok 60\n" : "not ok 60\n";
> +
>  # test global destruction
>  
> -my $test = 60;
> +my $test = 61;
>  my $test1 = $test + 1;
>  my $test2 = $test + 2;
>  
> --- doop.c	2002/04/03 17:22:42	1.1
> +++ doop.c	2002/04/03 17:33:33
> @@ -1314,7 +1314,7 @@
>  	    if (LvTARG(TARG) != (SV*)keys) {
>  		if (LvTARG(TARG))
>  		    SvREFCNT_dec(LvTARG(TARG));
> -		LvTARG(TARG) = SvREFCNT_inc(keys);
> +		LvTARG(TARG) = keys;
>  	    }
>  	    PUSHs(TARG);
>  	    RETURN;
> 
> 

Following up later, you wrote...

>
>>http://www.rosat.mpe-garching.mpg.de/mailing-lists/perl5-porters/1998-04/msg00035.html

>Thanks. With Sarathy's help, I was able to find an example where my
>patch failed. I'm now working on a different approach.
>OK, this is way after the fact, but do you know if somewhere you might
>please have an example where your patch failed?  Adding this as a test
>case would help anyone who decided to follow up on this problem.

I know this is *way* after the fact, but do you have examples of where
your patch failed.  These would be very helpful to someone wanting to
take up where this ticket was left off. 



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