Skip to content

bpo-35423: Stop using the "pending calls" machinery for signals. #10972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
2 changes: 2 additions & 0 deletions Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ struct _ceval_runtime_state {
/* Request for dropping the GIL */
_Py_atomic_int gil_drop_request;
struct _pending_calls pending;
/* Request for checking signals. */
_Py_atomic_int signals_pending;
struct _gil_runtime_state gil;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Separate the signal handling trigger in the eval loop from the "pending
calls" machinery. There is no semantic change and the difference in
performance is insignificant.
6 changes: 6 additions & 0 deletions Modules/signalmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,15 @@ It raises KeyboardInterrupt.");
static int
report_wakeup_write_error(void *data)
{
PyObject *exc, *val, *tb;
int save_errno = errno;
errno = (int) (intptr_t) data;
PyErr_Fetch(&exc, &val, &tb);
PyErr_SetFromErrno(PyExc_OSError);
PySys_WriteStderr("Exception ignored when trying to write to the "
"signal wakeup fd:\n");
PyErr_WriteUnraisable(NULL);
PyErr_Restore(exc, val, tb);
errno = save_errno;
return 0;
}
Expand All @@ -216,13 +219,16 @@ report_wakeup_write_error(void *data)
static int
report_wakeup_send_error(void* data)
{
PyObject *exc, *val, *tb;
PyErr_Fetch(&exc, &val, &tb);
/* PyErr_SetExcFromWindowsErr() invokes FormatMessage() which
recognizes the error codes used by both GetLastError() and
WSAGetLastError */
PyErr_SetExcFromWindowsErr(PyExc_OSError, (int) (intptr_t) data);
PySys_WriteStderr("Exception ignored when trying to send to the "
"signal wakeup fd:\n");
PyErr_WriteUnraisable(NULL);
PyErr_Restore(exc, val, tb);
return 0;
}
#endif /* MS_WINDOWS */
Expand Down
108 changes: 83 additions & 25 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ static long dxp[256];
_Py_atomic_store_relaxed( \
&_PyRuntime.ceval.eval_breaker, \
GIL_REQUEST | \
_Py_atomic_load_relaxed(&_PyRuntime.ceval.signals_pending) | \
_Py_atomic_load_relaxed(&_PyRuntime.ceval.pending.calls_to_do) | \
_PyRuntime.ceval.pending.async_exc)

Expand Down Expand Up @@ -128,6 +129,18 @@ static long dxp[256];
COMPUTE_EVAL_BREAKER(); \
} while (0)

#define SIGNAL_PENDING_SIGNALS() \
do { \
_Py_atomic_store_relaxed(&_PyRuntime.ceval.signals_pending, 1); \
_Py_atomic_store_relaxed(&_PyRuntime.ceval.eval_breaker, 1); \
} while (0)

#define UNSIGNAL_PENDING_SIGNALS() \
do { \
_Py_atomic_store_relaxed(&_PyRuntime.ceval.signals_pending, 0); \
COMPUTE_EVAL_BREAKER(); \
} while (0)

#define SIGNAL_ASYNC_EXC() \
do { \
_PyRuntime.ceval.pending.async_exc = 1; \
Expand Down Expand Up @@ -306,7 +319,7 @@ _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();
SIGNAL_PENDING_SIGNALS();
}

/* This implementation is thread-safe. It allows
Expand Down Expand Up @@ -356,44 +369,57 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
return result;
}

int
Py_MakePendingCalls(void)
static int
handle_signals(void)
{
static int busy = 0;
int i;
int r = 0;

assert(PyGILState_Check());
/* Only handle signals on main thread. */
if (_PyRuntime.ceval.pending.main_thread &&
PyThread_get_thread_ident() != _PyRuntime.ceval.pending.main_thread)
{
return 0;
}

if (!_PyRuntime.ceval.pending.lock) {
/* initial allocation of the lock */
_PyRuntime.ceval.pending.lock = PyThread_allocate_lock();
if (_PyRuntime.ceval.pending.lock == NULL)
return -1;
UNSIGNAL_PENDING_SIGNALS();
if (PyErr_CheckSignals() < 0) {
SIGNAL_PENDING_SIGNALS(); /* We're not done yet */
return -1;
}
return 0;
}

static int
make_pending_calls(void)
{
static int busy = 0;

/* only service pending calls on main thread */
if (_PyRuntime.ceval.pending.main_thread &&
PyThread_get_thread_ident() != _PyRuntime.ceval.pending.main_thread)
{
return 0;
}

/* don't perform recursive pending calls */
if (busy)
if (busy) {
return 0;
}
busy = 1;
/* unsignal before starting to call callbacks, so that any callback
added in-between re-signals */
UNSIGNAL_PENDING_CALLS();
int res = 0;

/* 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;
if (!_PyRuntime.ceval.pending.lock) {
/* initial allocation of the lock */
_PyRuntime.ceval.pending.lock = PyThread_allocate_lock();
if (_PyRuntime.ceval.pending.lock == NULL) {
res = -1;
goto error;
}
}

/* perform a bounded number of calls, in case of recursion */
for (i=0; i<NPENDINGCALLS; i++) {
for (int i=0; i<NPENDINGCALLS; i++) {
int j;
int (*func)(void *);
void *arg = NULL;
Expand All @@ -412,19 +438,41 @@ Py_MakePendingCalls(void)
/* having released the lock, perform the callback */
if (func == NULL)
break;
r = func(arg);
if (r) {
res = func(arg);
if (res) {
goto error;
}
}

busy = 0;
return r;
return res;

error:
busy = 0;
SIGNAL_PENDING_CALLS(); /* We're not done yet */
return -1;
SIGNAL_PENDING_CALLS();
return res;
}

/* Py_MakePendingCalls() is a simple wrapper for the sake
of backward-compatibility. */
int
Py_MakePendingCalls(void)
{
assert(PyGILState_Check());

/* Python signal handler doesn't really queue a callback: it only signals
that a signal was received, see _PyEval_SignalReceived(). */
int res = handle_signals();
if (res != 0) {
return res;
}

res = make_pending_calls();
if (res != 0) {
return res;
}

return 0;
}

/* The interpreter's recursion limit */
Expand Down Expand Up @@ -957,12 +1005,22 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
*/
goto fast_next_opcode;
}

if (_Py_atomic_load_relaxed(
&_PyRuntime.ceval.signals_pending))
{
if (handle_signals() != 0) {
goto error;
}
}
if (_Py_atomic_load_relaxed(
&_PyRuntime.ceval.pending.calls_to_do))
{
if (Py_MakePendingCalls() < 0)
if (make_pending_calls() != 0) {
goto error;
}
}

if (_Py_atomic_load_relaxed(
&_PyRuntime.ceval.gil_drop_request))
{
Expand Down