FAQ
This patch for now fixes the problem of segfaulting when using
async() in 5.8.1. It also removes some cruft (what on earth would
"use threads::Shared" mean in that context?) and simplifies the
equal() subroutine as well.

All tests pass as before.


Liz

Search Discussions

  • Abhijit Menon-Sen at Oct 10, 2003 at 8:22 pm

    At 2003-10-10 16:37:55 +0200, liz@dijkmat.nl wrote:
    This patch for now fixes the problem of segfaulting when using async()
    in 5.8.1.
    Thanks, applied. (#21436)

    -- ams
  • Enache Adrian at Oct 10, 2003 at 8:37 pm

    On Sat, Oct 11, 2003 a.d., Abhijit Menon-Sen wrote:
    At 2003-10-10 16:37:55 +0200, liz@dijkmat.nl wrote:

    This patch for now fixes the problem of segfaulting when using async()
    in 5.8.1.
    Thanks, applied. (#21436)
    However, please don't close the ticket. The bug is still there and
    should be fixed :-)

    $ perl -Mthreads -e 'sub a{threads->new(shift)} $t = a sub{}; $t->tid; $t->join; $t->tid'
    Segmentation fault

    Regards,
    Adi
  • Elizabeth Mattijsen at Oct 10, 2003 at 8:47 pm

    At 23:35 +0300 10/10/03, Enache Adrian wrote:
    On Sat, Oct 11, 2003 a.d., Abhijit Menon-Sen wrote:
    At 2003-10-10 16:37:55 +0200, liz@dijkmat.nl wrote:

    This patch for now fixes the problem of segfaulting when using async()
    in 5.8.1.
    Thanks, applied. (#21436)
    However, please don't close the ticket. The bug is still there and
    should be fixed :-)
    $ perl -Mthreads -e 'sub a{threads->new(shift)} $t = a sub{};
    $t->tid; $t->join; $t->tid'
    Segmentation fault
    Indeed, which was implied by my "for now". It's a workaround for the
    async problem.

    Looking at the stack trace (which I posted yesterday to p5p), I think
    the problem is pad related. And since there have been pad related
    fixes between 5.8.0 and 5.8.1, I think we need to look for the final
    fix of the problem there. But that is defenitely out of my league.


    Liz
  • Dave Mitchell at Oct 16, 2003 at 9:37 pm

    On Fri, Oct 10, 2003 at 10:46:17PM +0200, Elizabeth Mattijsen wrote:
    At 23:35 +0300 10/10/03, Enache Adrian wrote:
    On Sat, Oct 11, 2003 a.d., Abhijit Menon-Sen wrote:
    At 2003-10-10 16:37:55 +0200, liz@dijkmat.nl wrote:

    This patch for now fixes the problem of segfaulting when using async()
    in 5.8.1.
    Thanks, applied. (#21436)
    However, please don't close the ticket. The bug is still there and
    should be fixed :-)
    $ perl -Mthreads -e 'sub a{threads->new(shift)} $t = a sub{};
    $t->tid; $t->join; $t->tid'
    Segmentation fault
    Indeed, which was implied by my "for now". It's a workaround for the
    async problem.

    Looking at the stack trace (which I posted yesterday to p5p), I think
    the problem is pad related. And since there have been pad related
    fixes between 5.8.0 and 5.8.1, I think we need to look for the final
    fix of the problem there. But that is defenitely out of my league.
    The problem is that when threads call perl_destruct(), PL_comppad and
    PL_curpad point to the pad of the CV from where they were created, rather
    than to the pad of PL_main_cv. When the last thread exits, PL_main_root
    is freed, and coredumps occur as op_free() tries to do things with the pad
    slots pointed to by various ops.

    In this particular case, when the main program exits it calls
    perl_destruct(), which decrements the refcount of PL_main_root; it then
    frees $t, which invokes perl_destruct() on the (previously joined) child
    thread, which then causes PL_main_root to be freed while PL_comppad
    points to the wrong place.

    The following patch, applied as #21470, fixes the immediate problem (*).

    I must confess that I was suprised to find that perl_destruct isn't
    called on behalf of a thread immediately after it is joined, but rather
    only after the thread ref is freed.

    Dave.

    (*) Whoo hoo, my first commit :-)

    --
    Please note that ash-trays are provided for the use of smokers,
    whereas the floor is provided for the use of all patrons.
    - Bill Royston


    Change 21470 by davem@davem-percy on 2003/10/16 20:03:44

    Ensure PL_comppad/curpad point to PL_main_cv's padlist when
    PL_main_root is freed; this may not have been be the case if a
    thread other than the main one is the last to be destroyed

    Affected files ...

    ... //depot/perl/ext/threads/t/thread.t#10 edit
    ... //depot/perl/pad.h#9 edit
    ... //depot/perl/perl.c#526 edit

    Differences ...

    ==== //depot/perl/ext/threads/t/thread.t#10 (text) ====

    @@ -12,7 +12,7 @@

    use ExtUtils::testlib;
    use strict;
    -BEGIN { $| = 1; print "1..25\n" };
    +BEGIN { $| = 1; print "1..26\n" };
    use threads;
    use threads::shared;

    @@ -153,5 +153,10 @@
    ok((keys %rand == 25), "Check that rand works after a new thread");
    }

    +# bugid #24165
    +
    +run_perl(prog =>
    + 'use threads; sub a{threads->new(shift)} $t = a sub{}; $t->tid; $t->join; $t->tid');
    +is($?, 0, 'coredump in global destruction');



    ==== //depot/perl/pad.h#9 (text) ====

    @@ -105,6 +105,9 @@
    Set the current pad to be pad C<n> in the padlist, saving
    the previous current pad.

    +=for apidoc m|void|PAD_SET_CUR_NOSAVE |PADLIST padlist|I32 n
    +like PAD_SET_CUR, but without the save
    +
    =for apidoc m|void|PAD_SAVE_SETNULLPAD
    Save the current pad then set it to null.

    @@ -133,8 +136,7 @@
    ? AvARRAY((AV*)(AvARRAY(padlist)[1]))[po] : Nullsv;


    -#define PAD_SET_CUR(padlist,n) \
    - SAVECOMPPAD(); \
    +#define PAD_SET_CUR_NOSAVE(padlist,n) \
    PL_comppad = (PAD*) (AvARRAY(padlist)[n]); \
    PL_curpad = AvARRAY(PL_comppad); \
    DEBUG_Xv(PerlIO_printf(Perl_debug_log, \
    @@ -142,6 +144,11 @@
    PTR2UV(PL_comppad), PTR2UV(PL_curpad), (int)(n)));


    +#define PAD_SET_CUR(padlist,n) \
    + SAVECOMPPAD(); \
    + PAD_SET_CUR_NOSAVE(padlist,n);
    +
    +
    #define PAD_SAVE_SETNULLPAD() SAVECOMPPAD(); \
    PL_comppad = Null(PAD*); PL_curpad = Null(SV**); \
    DEBUG_Xv(PerlIO_printf(Perl_debug_log, "Pad set_null\n"));

    ==== //depot/perl/perl.c#526 (text) ====

    @@ -354,6 +354,10 @@

    /* Destroy the main CV and syntax tree */
    if (PL_main_root) {
    + /* ensure comppad/curpad to refer to main's pad */
    + if (CvPADLIST(PL_main_cv)) {
    + PAD_SET_CUR_NOSAVE(CvPADLIST(PL_main_cv), 1);
    + }
    op_free(PL_main_root);
    PL_main_root = Nullop;
    }
  • Nicholas Clark at Oct 19, 2003 at 4:58 pm
    Does this (21470):
    On Thu, Oct 16, 2003 at 10:37:11PM +0100, Dave Mitchell wrote:

    The following patch, applied as #21470, fixes the immediate problem (*).

    I must confess that I was suprised to find that perl_destruct isn't
    called on behalf of a thread immediately after it is joined, but rather
    only after the thread ref is freed.

    Dave.

    (*) Whoo hoo, my first commit :-)

    --
    Please note that ash-trays are provided for the use of smokers,
    whereas the floor is provided for the use of all patrons.
    - Bill Royston


    Change 21470 by davem@davem-percy on 2003/10/16 20:03:44

    Ensure PL_comppad/curpad point to PL_main_cv's padlist when
    PL_main_root is freed; this may not have been be the case if a
    thread other than the main one is the last to be destroyed

    Affected files ...

    ... //depot/perl/ext/threads/t/thread.t#10 edit
    ... //depot/perl/pad.h#9 edit
    ... //depot/perl/perl.c#526 edit
    mean that this (21436):
    On Fri, Oct 10, 2003 at 04:37:55PM +0200, Elizabeth Mattijsen wrote:
    This patch for now fixes the problem of segfaulting when using
    async() in 5.8.1. It also removes some cruft (what on earth would
    "use threads::Shared" mean in that context?) and simplifies the
    equal() subroutine as well.

    All tests pass as before.
    ==== //depot/perl/ext/threads/threads.pm#34 (xtext) ====

    @@ -31,8 +31,6 @@
    '==' => \&equal,
    'fallback' => 1;

    -#use threads::Shared;
    -
    BEGIN {
    warn "Warning, threads::shared has already been loaded. ".
    "To enable shared variables for these modules 'use threads' ".
    @@ -55,15 +53,11 @@
    our $VERSION = '1.00';


    -sub equal {
    - return 1 if($_[0]->tid() == $_[1]->tid());
    - return 0;
    -}
    +# || 0 to ensure compatibility with previous versions
    +sub equal { ($_[0]->tid == $_[1]->tid) || 0 }

    -sub async (&;@) {
    - my $cref = shift;
    - return threads->new($cref,@_);
    -}
    +# use "goto" trick to avoid pad problems from 5.8.1, should also be faster
    +sub async (&;@) { unshift @_,'threads'; goto &new }

    sub object {
    return undef unless @_ > 1;


    Is no longer needed? Or is it still worthwhile in its own right?

    Nicholas Clark
  • Elizabeth Mattijsen at Oct 19, 2003 at 5:02 pm

    At 17:57 +0100 10/19/03, Nicholas Clark wrote:
    Does this (21470):
    +# use "goto" trick to avoid pad problems from 5.8.1, should also be faster
    +sub async (&;@) { unshift @_,'threads'; goto &new }
    Is no longer needed? Or is it still worthwhile in its own right?
    The comment might need adaptation, but I think it is worthwhile in
    its own right. Similar patches were on my lists in late 5.8.0, but I
    decided not to bother Jarkko about it anymore... ;-)



    Liz

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupperl5-porters @
categoriesperl
postedOct 10, '03 at 2:38p
activeOct 19, '03 at 5:02p
posts7
users5
websiteperl.org

People

Translate

site design / logo © 2022 Grokbase