From 728189884e0e128c4ffc57b785b04584d57a90c0 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 26 Mar 2020 22:28:11 +0100 Subject: [PATCH] bpo-38644: Pass tstate explicitly in signalmodule.c (GH-19184) PyOS_InterruptOccurred() now checks _Py_ThreadCanHandleSignals() before checking if SIGINT is tripped. --- Include/internal/pycore_pyerrors.h | 2 + Modules/signalmodule.c | 158 +++++++++++++++++------------ Python/ceval.c | 4 +- 3 files changed, 96 insertions(+), 68 deletions(-) diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index f3aa3e8b98a..bae10561c9f 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -65,6 +65,8 @@ PyAPI_FUNC(PyObject *) _PyErr_FormatFromCauseTstate( const char *format, ...); +PyAPI_FUNC(int) _PyErr_CheckSignalsTstate(PyThreadState *tstate); + #ifdef __cplusplus } #endif diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index fa3b01de25a..0a2c665bc46 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -5,7 +5,9 @@ #include "Python.h" #include "pycore_atomic.h" +#include "pycore_call.h" #include "pycore_ceval.h" +#include "pycore_pyerrors.h" #include "pycore_pystate.h" #ifndef MS_WINDOWS @@ -189,13 +191,6 @@ itimer_retval(struct itimerval *iv) } #endif -static int -thread_can_handle_signals(void) -{ - PyThreadState *tstate = _PyThreadState_GET(); - return _Py_ThreadCanHandleSignals(tstate); -} - static PyObject * signal_default_int_handler(PyObject *self, PyObject *args) { @@ -480,43 +475,53 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler) } #endif - if (!thread_can_handle_signals()) { - PyErr_SetString(PyExc_ValueError, - "signal only works in main thread " - "of the main interpreter"); + PyThreadState *tstate = _PyThreadState_GET(); + if (!_Py_ThreadCanHandleSignals(tstate)) { + _PyErr_SetString(tstate, PyExc_ValueError, + "signal only works in main thread " + "of the main interpreter"); return NULL; } if (signalnum < 1 || signalnum >= NSIG) { - PyErr_SetString(PyExc_ValueError, - "signal number out of range"); + _PyErr_SetString(tstate, PyExc_ValueError, + "signal number out of range"); return NULL; } - if (handler == IgnoreHandler) + if (handler == IgnoreHandler) { func = SIG_IGN; - else if (handler == DefaultHandler) - func = SIG_DFL; - else if (!PyCallable_Check(handler)) { - PyErr_SetString(PyExc_TypeError, -"signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object"); - return NULL; } - else + else if (handler == DefaultHandler) { + func = SIG_DFL; + } + else if (!PyCallable_Check(handler)) { + _PyErr_SetString(tstate, PyExc_TypeError, + "signal handler must be signal.SIG_IGN, " + "signal.SIG_DFL, or a callable object"); + return NULL; + } + else { func = signal_handler; + } + /* Check for pending signals before changing signal handler */ - if (_PyErr_CheckSignals()) { + if (_PyErr_CheckSignalsTstate(tstate)) { return NULL; } if (PyOS_setsig(signalnum, func) == SIG_ERR) { PyErr_SetFromErrno(PyExc_OSError); return NULL; } + old_handler = Handlers[signalnum].func; Py_INCREF(handler); Handlers[signalnum].func = handler; - if (old_handler != NULL) + + if (old_handler != NULL) { return old_handler; - else + } + else { Py_RETURN_NONE; + } } @@ -698,10 +703,11 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) return NULL; #endif - if (!thread_can_handle_signals()) { - PyErr_SetString(PyExc_ValueError, - "set_wakeup_fd only works in main thread " - "of the main interpreter"); + PyThreadState *tstate = _PyThreadState_GET(); + if (!_Py_ThreadCanHandleSignals(tstate)) { + _PyErr_SetString(tstate, PyExc_ValueError, + "set_wakeup_fd only works in main thread " + "of the main interpreter"); return NULL; } @@ -727,12 +733,13 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) fd = (int)sockfd; if ((SOCKET_T)fd != sockfd) { - PyErr_SetString(PyExc_ValueError, "invalid fd"); + _PyErr_SetString(tstate, PyExc_ValueError, "invalid fd"); return NULL; } - if (_Py_fstat(fd, &status) != 0) + if (_Py_fstat(fd, &status) != 0) { return NULL; + } /* on Windows, a file cannot be set to non-blocking mode */ } @@ -764,9 +771,9 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) if (blocking < 0) return NULL; if (blocking) { - PyErr_Format(PyExc_ValueError, - "the fd %i must be in non-blocking mode", - fd); + _PyErr_Format(tstate, PyExc_ValueError, + "the fd %i must be in non-blocking mode", + fd); return NULL; } } @@ -1673,23 +1680,22 @@ finisignal(void) int PyErr_CheckSignals(void) { - if (!thread_can_handle_signals()) { + PyThreadState *tstate = _PyThreadState_GET(); + if (!_Py_ThreadCanHandleSignals(tstate)) { return 0; } - return _PyErr_CheckSignals(); + return _PyErr_CheckSignalsTstate(tstate); } /* Declared in cpython/pyerrors.h */ int -_PyErr_CheckSignals(void) +_PyErr_CheckSignalsTstate(PyThreadState *tstate) { - int i; - PyObject *f; - - if (!_Py_atomic_load(&is_tripped)) + if (!_Py_atomic_load(&is_tripped)) { return 0; + } /* * The is_tripped variable is meant to speed up the calls to @@ -1707,32 +1713,48 @@ _PyErr_CheckSignals(void) */ _Py_atomic_store(&is_tripped, 0); - if (!(f = (PyObject *)PyEval_GetFrame())) - f = Py_None; + PyObject *frame = (PyObject *)tstate->frame; + if (!frame) { + frame = Py_None; + } - for (i = 1; i < NSIG; i++) { - if (_Py_atomic_load_relaxed(&Handlers[i].tripped)) { - PyObject *result = NULL; - PyObject *arglist = Py_BuildValue("(iO)", i, f); - _Py_atomic_store_relaxed(&Handlers[i].tripped, 0); - - if (arglist) { - result = PyObject_Call(Handlers[i].func, arglist, NULL); - Py_DECREF(arglist); - } - if (!result) { - _Py_atomic_store(&is_tripped, 1); - return -1; - } - - Py_DECREF(result); + for (int i = 1; i < NSIG; i++) { + if (!_Py_atomic_load_relaxed(&Handlers[i].tripped)) { + continue; } + _Py_atomic_store_relaxed(&Handlers[i].tripped, 0); + + PyObject *arglist = Py_BuildValue("(iO)", i, frame); + PyObject *result; + if (arglist) { + result = _PyObject_Call(tstate, Handlers[i].func, arglist, NULL); + Py_DECREF(arglist); + } + else { + result = NULL; + } + if (!result) { + /* On error, re-schedule a call to _PyErr_CheckSignalsTstate() */ + _Py_atomic_store(&is_tripped, 1); + return -1; + } + + Py_DECREF(result); } return 0; } + +int +_PyErr_CheckSignals(void) +{ + PyThreadState *tstate = _PyThreadState_GET(); + return _PyErr_CheckSignalsTstate(tstate); +} + + /* Simulate the effect of a signal.SIGINT signal arriving. The next time PyErr_CheckSignals is called, the Python SIGINT signal handler will be raised. @@ -1765,14 +1787,17 @@ PyOS_FiniInterrupts(void) int PyOS_InterruptOccurred(void) { - if (_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) { - if (!thread_can_handle_signals()) { - return 0; - } - _Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0); - return 1; + PyThreadState *tstate = _PyThreadState_GET(); + if (!_Py_ThreadCanHandleSignals(tstate)) { + return 0; } - return 0; + + if (!_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) { + return 0; + } + + _Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0); + return 1; } static void @@ -1799,7 +1824,8 @@ _PySignal_AfterFork(void) int _PyOS_IsMainThread(void) { - return thread_can_handle_signals(); + PyThreadState *tstate = _PyThreadState_GET(); + return _Py_ThreadCanHandleSignals(tstate); } #ifdef MS_WINDOWS diff --git a/Python/ceval.c b/Python/ceval.c index afaa6ff1b3c..2be02a1ab6f 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -597,8 +597,8 @@ handle_signals(PyThreadState *tstate) } UNSIGNAL_PENDING_SIGNALS(tstate); - if (_PyErr_CheckSignals() < 0) { - /* We're not done yet */ + if (_PyErr_CheckSignalsTstate(tstate) < 0) { + /* On failure, re-schedule a call to handle_signals(). */ SIGNAL_PENDING_SIGNALS(tstate); return -1; }