[3.6] bpo-30703: Improve signal delivery (GH-2415) (#2527)
* [3.6] bpo-30703: Improve signal delivery (GH-2415)
* Improve signal delivery
Avoid using Py_AddPendingCall from signal handler, to avoid calling signal-unsafe functions.
* Remove unused function
* Improve comments
* Add stress test
* Adapt for --without-threads
* Add second stress test
* Add NEWS blurb
* Address comments @haypo.
(cherry picked from commit c08177a1ccad2ed0d50898c2731b518c631aed14)
* bpo-30796: Fix failures in signal delivery stress test (#2488)
* bpo-30796: Fix failures in signal delivery stress test
setitimer() can have a poor minimum resolution on some machines,
this would make the test reach its deadline (and a stray signal
could then kill a subsequent test).
* Make sure to clear the itimer after the test
diff --git a/Python/ceval.c b/Python/ceval.c
index ea79f5f..bf9103d 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -195,6 +195,15 @@
do { pending_async_exc = 0; COMPUTE_EVAL_BREAKER(); } while (0)
+/* This single variable consolidates all requests to break out of the fast path
+ in the eval loop. */
+static _Py_atomic_int eval_breaker = {0};
+/* Request for running pending calls. */
+static _Py_atomic_int pendingcalls_to_do = {0};
+/* Request for looking at the `async_exc` field of the current thread state.
+ Guarded by the GIL. */
+static int pending_async_exc = 0;
+
#ifdef WITH_THREAD
#ifdef HAVE_ERRNO_H
@@ -204,16 +213,8 @@
static PyThread_type_lock pending_lock = 0; /* for pending calls */
static long main_thread = 0;
-/* This single variable consolidates all requests to break out of the fast path
- in the eval loop. */
-static _Py_atomic_int eval_breaker = {0};
/* Request for dropping the GIL */
static _Py_atomic_int gil_drop_request = {0};
-/* Request for running pending calls. */
-static _Py_atomic_int pendingcalls_to_do = {0};
-/* Request for looking at the `async_exc` field of the current thread state.
- Guarded by the GIL. */
-static int pending_async_exc = 0;
#include "ceval_gil.h"
@@ -326,9 +327,6 @@
_PyThreadState_DeleteExcept(current_tstate);
}
-#else
-static _Py_atomic_int eval_breaker = {0};
-static int pending_async_exc = 0;
#endif /* WITH_THREAD */
/* This function is used to signal that async exceptions are waiting to be
@@ -403,6 +401,15 @@
#endif
*/
+void
+_PyEval_SignalReceived(void)
+{
+ /* bpo-30703: Function called when the C signal handler of Python gets a
+ signal. We cannot queue a callback using Py_AddPendingCall() since
+ that function is not async-signal-safe. */
+ SIGNAL_PENDING_CALLS();
+}
+
#ifdef WITH_THREAD
/* The WITH_THREAD implementation is thread-safe. It allows
@@ -467,6 +474,8 @@
int i;
int r = 0;
+ assert(PyGILState_Check());
+
if (!pending_lock) {
/* initial allocation of the lock */
pending_lock = PyThread_allocate_lock();
@@ -481,6 +490,16 @@
if (busy)
return 0;
busy = 1;
+ /* unsignal before starting to call callbacks, so that any callback
+ added in-between re-signals */
+ UNSIGNAL_PENDING_CALLS();
+
+ /* Python signal handler doesn't really queue a callback: it only signals
+ that a signal was received, see _PyEval_SignalReceived(). */
+ if (PyErr_CheckSignals() < 0) {
+ goto error;
+ }
+
/* perform a bounded number of calls, in case of recursion */
for (i=0; i<NPENDINGCALLS; i++) {
int j;
@@ -497,20 +516,23 @@
arg = pendingcalls[j].arg;
pendingfirst = (j + 1) % NPENDINGCALLS;
}
- if (pendingfirst != pendinglast)
- SIGNAL_PENDING_CALLS();
- else
- UNSIGNAL_PENDING_CALLS();
PyThread_release_lock(pending_lock);
/* having released the lock, perform the callback */
if (func == NULL)
break;
r = func(arg);
- if (r)
- break;
+ if (r) {
+ goto error;
+ }
}
+
busy = 0;
return r;
+
+error:
+ busy = 0;
+ SIGNAL_PENDING_CALLS(); /* We're not done yet */
+ return -1;
}
#else /* if ! defined WITH_THREAD */
@@ -545,7 +567,6 @@
} pendingcalls[NPENDINGCALLS];
static volatile int pendingfirst = 0;
static volatile int pendinglast = 0;
-static _Py_atomic_int pendingcalls_to_do = {0};
int
Py_AddPendingCall(int (*func)(void *), void *arg)
@@ -579,7 +600,16 @@
if (busy)
return 0;
busy = 1;
+
+ /* unsignal before starting to call callbacks, so that any callback
+ added in-between re-signals */
UNSIGNAL_PENDING_CALLS();
+ /* Python signal handler doesn't really queue a callback: it only signals
+ that a signal was received, see _PyEval_SignalReceived(). */
+ if (PyErr_CheckSignals() < 0) {
+ goto error;
+ }
+
for (;;) {
int i;
int (*func)(void *);
@@ -591,13 +621,16 @@
arg = pendingcalls[i].arg;
pendingfirst = (i + 1) % NPENDINGCALLS;
if (func(arg) < 0) {
- busy = 0;
- SIGNAL_PENDING_CALLS(); /* We're not done yet */
- return -1;
+ goto error:
}
}
busy = 0;
return 0;
+
+error:
+ busy = 0;
+ SIGNAL_PENDING_CALLS(); /* We're not done yet */
+ return -1;
}
#endif /* WITH_THREAD */