develooper Front page | perl.perl5.porters | Postings from May 2010

Re: [BUG] Re: [perl.git] branch blead, updated. v5.13.1-8-g39f3f7f

From:
Nicholas Clark
Date:
May 25, 2010 14:38
Subject:
Re: [BUG] Re: [perl.git] branch blead, updated. v5.13.1-8-g39f3f7f
Message ID:
20100525213843.GZ2576@plum.flirble.org
On Mon, May 24, 2010 at 06:49:15PM -0400, Jerry D. Hedden wrote:
> This change breaks my module Object::InsideOut.  I have determined the
> cause to be that using 'weaken' in combination with threads and objects
> is broken.  The attached self-contained script replicates the bug.


> > commit 39f3f7f442aed93239540238d19a15f6020da747

> > commit b1faab355535118c9fb99fcc343501012923ece2

> > commit 4cf5eae5e58faebb6ae9e68e493cd85343f7a76c

> > commit 78b7eff23b1757f7fccef902e7e706080a997e2c

Thanks for the short self contained test case. I bisect with this:

    #!/bin/sh
    git clean -dxf
    # If you can use ccache, add -Dcc=ccache\ gcc -Dld=gcc to the Configure line
    # if Encode is not needed for the test, you can speed up the bisect by
    # excluding it from the runs with -Dnoextensions=Encode
    sh Configure -des -Dusedevel -Dusethreads -Doptimize="-g" -Dcc=ccache\ gcc -Dld=gcc -Dnoextensions=Encode
    test -f config.sh || exit 125
    # Correct makefile for newer GNU gcc
    perl -ni -we 'print unless /<(?:built-in|command)/' makefile x2p/makefile
    # if you just need miniperl, replace test_prep with miniperl
    make -j4 test_prep
    [ -x ./perl ] || exit 125
    ./perl -Ilib ../Run/bug_thr_weak.pl
    ret=$?
    [ $ret -gt 127 ] && ret=127
    git clean -dxf
    exit $ret

and it was actually this commit

commit adf8f095c5881bcedf07b8e41072f8125e00b5a6
Author: Nicholas Clark <nick@ccl4.org>
Date:   Fri Feb 26 09:18:44 2010 +0000

    Set PADSTALE on all lexicals at the end of sub creation.
    
    The PADSTALEness of lexicals between the 0th and 1st call to a subroutine is now
    consistent with the state between the nth and (n + 1)th call.
    
    This permits a work around in Perl_padlist_dup() to avoid leaking active pad
    data into a new thread, whilst still correctly bodging the external references
    needed by the current ?{} implementation. Fix that, and this can be removed.



The aim of this (and the preceding refactoring) was to reduce the amount of
stuff in pads needlessly cloned as part of thread creation (but not pseudo
forking). Specifically, the cloning code used to copy all the scalars in pads
that had been entered in the parent thread, despite the fact that a child
thread isn't going to "return" to those subroutines.

I think it had the side effect of no longer cloning the pad that held a
(non-weak) reference to the value in question, which meant that during the
CLONE call, that value had a reference count of 0, and the new code in
e42956688f2e0df9 to better handle things with a reference count of 0 doesn't
do its fixups until after the calls to CLONE.

Hence if code in CLONE takes a reference to that value in CLONE, as was, the
value's reference count went from 0 to 1 and back to 0, at which point all
sorts of cleanup activity triggered, including that SV being put onto the
free list. (And then the fixup code would fix things up, so the SV would also
be referenced from elsewhere. Bad).

Fixed in

commit 04518cc3f43b495f85caf2ec89c8b06540a60f8c
Author: Nicholas Clark <nick@ccl4.org>
Date:   Tue May 25 17:23:10 2010 +0100

    Fix CLONE/weakref bug revealed by adf8f095c5881bce.
    
    The AV unreferenced in the clone_params needs to be reference counted, rather
    than not referenced counted, because the fixup to ensure that all otherwise
    0-reference count scalars have a reference (on the temps stack) happens after
    CLONE is run, and CLONE can run Perl code that causes their reference counts
    to increase from then return to zero, which prematurely triggers sv_free().


Also, it seems that this commit:

commit bb1bc619ea68d9703fbd3fe5bc65ae000f90151f
Author: Father Chrysostomos (via RT) <perlbug-followup@perl.org>
Date:   Sun Jan 17 14:32:24 2010 -0800

    Deref ops ignore get-magic when SvROK(sv)
    
    This is just like bug 68192, except in this case it’s a different set
    of operators that have had this problem for much longer.

causes the TODO test at the end of dist/threads-shared/t/object.t to start
passing:

ok 24 - Main: Object ID 1
ok 25 - Thread: Object ID 1
ok 26 - Thread: Changed object ID 10
ok 27 - Thread: New object ID 2
ok 28 - Main: New object ID 2  # TODO - should be 2

Nicholas Clark



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