Front page | perl.perl5.porters |
Postings from February 2012
Re: [perl #108798] PL_check is not thread-safe
Thread Previous
|
Thread Next
From:
Karl Williamson
Date:
February 8, 2012 08:07
Subject:
Re: [perl #108798] PL_check is not thread-safe
Message ID:
4F329D83.6060204@khwilliamson.com
On 02/08/2012 06:38 AM, Zefram wrote:
> Father Chrysostomos wrote:
>> Options are:
>>
>> * Provide a mutex
>> * Have modules use the op mutex
>> * Make it an interpreter var
>> * Completely new interface
>
> I'm now working on making my check-hooking modules thread safe, so I've
> looked at the issues here in some detail.
>
> Firstly, I've determined that Perl's "global" variables really are
> always once-per-process, despite the fuss about them sometimes being in
> an indirectly-accessed struct. Making PL_check an interpreter variable
> doesn't seem useful. It's not a priori problematic for checkers to be
> process-global, and indeed it's convenient for a module to be able to use
> a static variable to hold the address of the next checker in the chain.
> The performance win of avoiding a complex lookup of per-thread module
> state also shouldn't be overlooked, given how low-level the op chaining
> mechanism is. (Of course, in any case I have to deal with existing
> Perls where PL_check is unavoidably global.)
>
> There are two thread safety issues in hooking an op checker. One is
> the very brief race condition between two threads (running actually
> simultaneously or with preemptive scheduling) that attempt to hook
> at the same time. Fixing this requires a mutex. In the future the
> core should provide a specific mutex for this, but for retrospective
> use on old Perls I'm adopting the op refcount mutex. This is a purely
> conventional choice; it works as long as everyone writing such modules
> adopts the same convention.
>
> The second issue is of a module being bootstrapped more than once
> in a single process. This can be made to happen very reliably, by
> (for example) loading the module in two threads, having not previously
> loaded it in the main program. Naive hooking, such as my modules have
> used so far, results in the checking chain becoming a loop, leading
> to hangs or crashes. I think a couple of my RT bug reports come from
> this cause. To fix it, the BOOT code needs to recognise that it can
> be called more than once per process, and arrange to be idempotent with
> respect to globals. It needs to not hook the checker if it has already
> done so in this process. The easiest test of whether it's already been
> hooked is simply whether the next-checker variable has been filled.
>
> A convenience function for hooking ought to address both of these inherent
> thread safety issues. Here is the code that I've now got in my latest
> dev version of Devel::CallChecker, and which I'm intending to clone into
> each of my op check hooking modules:
>
> #ifndef wrap_op_checker
> # define wrap_op_checker(c,n,o) THX_wrap_op_checker(aTHX_ c,n,o)
> static void THX_wrap_op_checker(pTHX_ Optype opcode,
> Perl_check_t new_checker, Perl_check_t *old_checker_p)
> {
> if(*old_checker_p) return;
> OP_REFCNT_LOCK;
> if(!*old_checker_p) {
> *old_checker_p = PL_check[opcode];
> PL_check[opcode] = new_checker;
> }
> OP_REFCNT_UNLOCK;
> }
> #endif /* !wrap_op_checker */
>
> I propose the adoption of this code for all op check hooking modules, and
> furthermore that the core in future should provide an equivalent function
> with exactly this API (but, of course, using a purpose-specific mutex).
> The #ifndef aspect of the code I'm using will yield a seamless transition
> if the core adopts exactly this API.
>
> If there are no objections, I'd like to add this to the core this week.
> This is the last chance to get it in for 5.16.
>
> -zefram
>
+1
Thread Previous
|
Thread Next