Skip to content

Commit 7c89eeb

Browse files
author
kristjan.jonsson
committed
Issue 4293: Make Py_AddPendingCall() thread safe
git-svn-id: http://svn.python.org/projects/python/trunk@68460 6015fed2-1504-0410-9fe1-9d1591cc4771
1 parent 7768a71 commit 7c89eeb

1 file changed

Lines changed: 146 additions & 22 deletions

File tree

‎Python/ceval.c‎

Lines changed: 146 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ PyEval_GetCallStats(PyObject *self)
213213
#include "pythread.h"
214214

215215
static PyThread_type_lock interpreter_lock = 0; /* This is the GIL */
216+
static PyThread_type_lock pending_lock = 0; /* for pending calls */
216217
static long main_thread = 0;
217218

218219
int
@@ -284,6 +285,7 @@ PyEval_ReInitThreads(void)
284285
adding a new function to each thread_*.h. Instead, just
285286
create a new lock and waste a little bit of memory */
286287
interpreter_lock = PyThread_allocate_lock();
288+
pending_lock = PyThread_allocate_lock();
287289
PyThread_acquire_lock(interpreter_lock, 1);
288290
main_thread = PyThread_get_thread_ident();
289291

@@ -356,19 +358,145 @@ PyEval_RestoreThread(PyThreadState *tstate)
356358
#ifdef WITH_THREAD
357359
Any thread can schedule pending calls, but only the main thread
358360
will execute them.
361+
There is no facility to schedule calls to a particular thread, but
362+
that should be easy to change, should that ever be required. In
363+
that case, the static variables here should go into the python
364+
threadstate.
359365
#endif
366+
*/
367+
368+
#ifdef WITH_THREAD
369+
370+
/* The WITH_THREAD implementation is thread-safe. It allows
371+
scheduling to be made from any thread, and even from an executing
372+
callback.
373+
*/
374+
375+
#define NPENDINGCALLS 32
376+
static struct {
377+
int (*func)(void *);
378+
void *arg;
379+
} pendingcalls[NPENDINGCALLS];
380+
static int pendingfirst = 0;
381+
static int pendinglast = 0;
382+
static volatile int pendingcalls_to_do = 1; /* trigger initialization of lock */
383+
static char pendingbusy = 0;
384+
385+
int
386+
Py_AddPendingCall(int (*func)(void *), void *arg)
387+
{
388+
int i, j, result=0;
389+
PyThread_type_lock lock = pending_lock;
390+
391+
/* try a few times for the lock. Since this mechanism is used
392+
* for signal handling (on the main thread), there is a (slim)
393+
* chance that a signal is delivered on the same thread while we
394+
* hold the lock during the Py_MakePendingCalls() function.
395+
* This avoids a deadlock in that case.
396+
* Note that signals can be delivered on any thread. In particular,
397+
* on Windows, a SIGINT is delivered on a system-created worker
398+
* thread.
399+
* We also check for lock being NULL, in the unlikely case that
400+
* this function is called before any bytecode evaluation takes place.
401+
*/
402+
if (lock != NULL) {
403+
for (i = 0; i<100; i++) {
404+
if (PyThread_acquire_lock(lock, NOWAIT_LOCK))
405+
break;
406+
}
407+
if (i == 100)
408+
return -1;
409+
}
410+
411+
i = pendinglast;
412+
j = (i + 1) % NPENDINGCALLS;
413+
if (j == pendingfirst) {
414+
result = -1; /* Queue full */
415+
} else {
416+
pendingcalls[i].func = func;
417+
pendingcalls[i].arg = arg;
418+
pendinglast = j;
419+
}
420+
/* signal main loop */
421+
_Py_Ticker = 0;
422+
pendingcalls_to_do = 1;
423+
if (lock != NULL)
424+
PyThread_release_lock(lock);
425+
return result;
426+
}
427+
428+
int
429+
Py_MakePendingCalls(void)
430+
{
431+
int i;
432+
int r = 0;
360433

361-
XXX WARNING! ASYNCHRONOUSLY EXECUTING CODE!
434+
if (!pending_lock) {
435+
/* initial allocation of the lock */
436+
pending_lock = PyThread_allocate_lock();
437+
if (pending_lock == NULL)
438+
return -1;
439+
}
440+
441+
/* only service pending calls on main thread */
442+
if (main_thread && PyThread_get_thread_ident() != main_thread)
443+
return 0;
444+
/* don't perform recursive pending calls */
445+
if (pendingbusy)
446+
return 0;
447+
pendingbusy = 1;
448+
/* perform a bounded number of calls, in case of recursion */
449+
for (i=0; i<NPENDINGCALLS; i++) {
450+
int j;
451+
int (*func)(void *);
452+
void *arg;
453+
454+
/* pop one item off the queue while holding the lock */
455+
PyThread_acquire_lock(pending_lock, WAIT_LOCK);
456+
j = pendingfirst;
457+
if (j == pendinglast) {
458+
func = NULL; /* Queue empty */
459+
} else {
460+
func = pendingcalls[j].func;
461+
arg = pendingcalls[j].arg;
462+
pendingfirst = (j + 1) % NPENDINGCALLS;
463+
}
464+
pendingcalls_to_do = pendingfirst != pendinglast;
465+
PyThread_release_lock(pending_lock);
466+
/* having released the lock, perform the callback */
467+
if (func == NULL)
468+
break;
469+
r = func(arg);
470+
if (r)
471+
break;
472+
}
473+
pendingbusy = 0;
474+
return r;
475+
}
476+
477+
#else /* if ! defined WITH_THREAD */
478+
479+
/*
480+
WARNING! ASYNCHRONOUSLY EXECUTING CODE!
481+
This code is used for signal handling in python that isn't built
482+
with WITH_THREAD.
483+
Don't use this implementation when Py_AddPendingCalls() can happen
484+
on a different thread!
485+
362486
There are two possible race conditions:
363-
(1) nested asynchronous registry calls;
364-
(2) registry calls made while pending calls are being processed.
365-
While (1) is very unlikely, (2) is a real possibility.
487+
(1) nested asynchronous calls to Py_AddPendingCall()
488+
(2) AddPendingCall() calls made while pending calls are being processed.
489+
490+
(1) is very unlikely because typically signal delivery
491+
is blocked during signal handling. So it should be impossible.
492+
(2) is a real possibility.
366493
The current code is safe against (2), but not against (1).
367494
The safety against (2) is derived from the fact that only one
368-
thread (the main thread) ever takes things out of the queue.
369-
370-
XXX Darn! With the advent of thread state, we should have an array
371-
of pending calls per thread in the thread state! Later...
495+
thread is present, interrupted by signals, and that the critical
496+
section is protected with the "busy" variable. On Windows, which
497+
delivers SIGINT on a system thread, this does not hold and therefore
498+
Windows really shouldn't use this version.
499+
The two threads could theoretically wiggle around the "busy" variable.
372500
*/
373501

374502
#define NPENDINGCALLS 32
@@ -378,16 +506,14 @@ static struct {
378506
} pendingcalls[NPENDINGCALLS];
379507
static volatile int pendingfirst = 0;
380508
static volatile int pendinglast = 0;
381-
static volatile int things_to_do = 0;
509+
static volatile int pendingcalls_to_do = 0;
382510

383511
int
384512
Py_AddPendingCall(int (*func)(void *), void *arg)
385513
{
386514
static volatile int busy = 0;
387515
int i, j;
388516
/* XXX Begin critical section */
389-
/* XXX If you want this to be safe against nested
390-
XXX asynchronous calls, you'll have to work harder! */
391517
if (busy)
392518
return -1;
393519
busy = 1;
@@ -402,7 +528,7 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
402528
pendinglast = j;
403529

404530
_Py_Ticker = 0;
405-
things_to_do = 1; /* Signal main loop */
531+
pendingcalls_to_do = 1; /* Signal main loop */
406532
busy = 0;
407533
/* XXX End critical section */
408534
return 0;
@@ -412,14 +538,10 @@ int
412538
Py_MakePendingCalls(void)
413539
{
414540
static int busy = 0;
415-
#ifdef WITH_THREAD
416-
if (main_thread && PyThread_get_thread_ident() != main_thread)
417-
return 0;
418-
#endif
419541
if (busy)
420542
return 0;
421543
busy = 1;
422-
things_to_do = 0;
544+
pendingcalls_to_do = 0;
423545
for (;;) {
424546
int i;
425547
int (*func)(void *);
@@ -432,14 +554,16 @@ Py_MakePendingCalls(void)
432554
pendingfirst = (i + 1) % NPENDINGCALLS;
433555
if (func(arg) < 0) {
434556
busy = 0;
435-
things_to_do = 1; /* We're not done yet */
557+
pendingcalls_to_do = 1; /* We're not done yet */
436558
return -1;
437559
}
438560
}
439561
busy = 0;
440562
return 0;
441563
}
442564

565+
#endif /* WITH_THREAD */
566+
443567

444568
/* The interpreter's recursion limit */
445569

@@ -514,7 +638,7 @@ static int _Py_TracingPossible = 0;
514638
/* for manipulating the thread switch and periodic "stuff" - used to be
515639
per thread, now just a pair o' globals */
516640
int _Py_CheckInterval = 100;
517-
volatile int _Py_Ticker = 100;
641+
volatile int _Py_Ticker = 0; /* so that we hit a "tick" first thing */
518642

519643
PyObject *
520644
PyEval_EvalCode(PyCodeObject *co, PyObject *globals, PyObject *locals)
@@ -832,7 +956,7 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
832956
/* Do periodic things. Doing this every time through
833957
the loop would add too much overhead, so we do it
834958
only every Nth instruction. We also do it if
835-
``things_to_do'' is set, i.e. when an asynchronous
959+
``pendingcalls_to_do'' is set, i.e. when an asynchronous
836960
event needs attention (e.g. a signal handler or
837961
async I/O handler); see Py_AddPendingCall() and
838962
Py_MakePendingCalls() above. */
@@ -848,12 +972,12 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
848972
#ifdef WITH_TSC
849973
ticked = 1;
850974
#endif
851-
if (things_to_do) {
975+
if (pendingcalls_to_do) {
852976
if (Py_MakePendingCalls() < 0) {
853977
why = WHY_EXCEPTION;
854978
goto on_error;
855979
}
856-
if (things_to_do)
980+
if (pendingcalls_to_do)
857981
/* MakePendingCalls() didn't succeed.
858982
Force early re-execution of this
859983
"periodic" code, possibly after

0 commit comments

Comments
 (0)