Front page | perl.perl5.porters |
Postings from September 2013
Re: [perl #119897] glob in threads is broken in perl5.18
Thread Previous
|
Thread Next
From:
Brian Fraser
Date:
September 21, 2013 06:45
Subject:
Re: [perl #119897] glob in threads is broken in perl5.18
Message ID:
CA+nL+naMOWHYaK8CUO5xEnCw3_M4sKhoLbDba9D5BfndYHDk_w@mail.gmail.com
On Sat, Sep 21, 2013 at 12:19 AM, Father Chrysostomos via RT <
perlbug-followup@perl.org> wrote:
> On Fri Sep 20 18:05:44 2013, Hugmeir wrote:
> > On Fri, Sep 20, 2013 at 9:16 PM, Father Chrysostomos via RT <
> > perlbug-followup@perl.org> wrote:
> >
> > > On Fri Sep 20 16:31:00 2013, Hugmeir wrote:
> > > > Yes. The problem for both is that x_GLOB_ENTRIES should be
> thread-local,
> > > > but is unintentionally being shared between threads.
> > > >
> > > > This patch solves the issue:
> > > >
> > > > diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs
> > > > index b3705b3..181dbbc 100644
> > > > --- a/ext/File-Glob/Glob.xs
> > > > +++ b/ext/File-Glob/Glob.xs
> > > > @@ -396,6 +396,13 @@ PPCODE:
> > > > iterate(aTHX_ doglob_iter_wrapper);
> > > > SPAGAIN;
> > > >
> > > > +void
> > > > +CLONE(...)
> > > > +CODE:
> > > > + PERL_UNUSED_ARG(items);
> > > > + MY_CXT_CLONE;
> > > > + MY_CXT.x_GLOB_ENTRIES = NULL;
> > > > +
> > > > BOOT:
> > > > {
> > > > #ifndef PERL_EXTERNAL_GLOB
> > > >
> > > >
> > > > But it means that this:
> > > >
> > > > ./perl -Ilib -Mthreads -E 'sub foo {scalar glob("*")} foo(); say
> > > > threads->create(\&foo)->join() for 1..3'
> > > >
> > > > Will return "Artistic" four times, rather than "Artristic" followed
> by
> > > the
> > > > second file thrice. If the latter is the desired behavior, then we
> need
> > > to
> > > > hv_dup(x_GLOB_ENTRIES) in CLONE() as well.
> > >
> > > Yes, the latter. That would be backward-compatible with when
> File::Glob
> > > still worked.
> > >
> >
> > I'm afraid I don't know how to implement this properly! Unlike dup magic,
> > CLONE() doesn't get a CLONE_PARAMS argument, so calling sv_dup() and
> > friends requires manual intervention, and I don't believe that the
> correct
> > procedure is documented anywhere.
> > The only place in the core that does an sv_dup() during CLONE() is
> > threads.xs, and that's likely not a good role model for more normal code;
>
> Ah, but File::Glob is *not* normal code!
>
Heh, fair enough.
>
> > meanwhile, the paradigm in the few CPAN modules that do this[*] is "grab
> > the interpreter during BOOT, use that to construct a fake
> CLONE_PARAMS, and
> > then replace the interpreter with aTHX after MY_CXT_CLONE"
> > Which, on top of the previous patch, becomes this:
>
> It has often bothered me that a CLONE method in pure Perl can’t really
> access both interpreters. I have known about CLONE for a long time, but
> had never actually seen it used (or bothered looking).
>
> If what you have below works, and works reliably (it looks fine to me),
> I say go for it.
>
I wasn't sure if the new tests would work on VMS -- most of the other glob
test files have special cases for it -- so I've pushed this as
smoke-me/hugmeir/dup_glob_state
>
> Is it possible to write this in a way that is binary-compatible (for
> 5.16 and 5.18)? I believe the fact that the struct below is not public
> makes it safe to change it like that.
>
This I do not know.
(I can't find a definition of binary (in)?compatibility perlpolicy or other
pods)
Thread Previous
|
Thread Next