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

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

Thread Previous | Thread Next
February 8, 2012 05:38
Re: [perl #108798] PL_check is not thread-safe
Message ID:
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.


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