bpo-38644: Pass tstate explicitly in signalmodule.c (GH-19184)

PyOS_InterruptOccurred() now checks _Py_ThreadCanHandleSignals()
before checking if SIGINT is tripped.

Author: vstinner

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

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)
+    }
+    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;
+        _PyErr_SetString(tstate, PyExc_TypeError,
+                         "signal handler must be signal.SIG_IGN, "
+                         "signal.SIG_DFL, or a callable object");
+        return NULL;
     }
-    else
+    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;
-            }
+    for (int i = 1; i < NSIG; i++) {
+        if (!_Py_atomic_load_relaxed(&Handlers[i].tripped)) {
+            continue;
+        }
+        _Py_atomic_store_relaxed(&Handlers[i].tripped, 0);
 
-            Py_DECREF(result);
+        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

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;
     }