develooper Front page | perl.perl5.porters | Postings from February 2012

Re: [perl #108798] PL_check is not thread-safe

Thread Previous | Thread Next
Karl Williamson
February 8, 2012 08:07
Re: [perl #108798] PL_check is not thread-safe
Message ID:
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;
> 		if(!*old_checker_p) {
> 			*old_checker_p = PL_check[opcode];
> 			PL_check[opcode] = new_checker;
> 		}
> 	}
> 	#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


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About