bpo-40010: COMPUTE_EVAL_BREAKER() checks for subinterpreter (GH-19087)

COMPUTE_EVAL_BREAKER() now also checks if the Python thread state
belongs to the main interpreter. Don't break the evaluation loop if
there are pending signals but the Python thread state it belongs to a
subinterpeter.

* Add _Py_IsMainThread() function.
* Add _Py_ThreadCanHandleSignals() function.

diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst
index 8b3ab412bd368f2f5a544fa0fd9135a25beed0b3..4353cc7cdf454da79682310fd73ba97ab1b1df0e 100644 (file)
--- a/Doc/library/signal.rst
+++ b/Doc/library/signal.rst
@@ -53,12 +53,12 @@ This has consequences:
 Signals and threads
 ^^^^^^^^^^^^^^^^^^^
 
-Python signal handlers are always executed in the main Python thread,
+Python signal handlers are always executed in the main Python thread of the main interpreter,
 even if the signal was received in another thread.  This means that signals
 can't be used as a means of inter-thread communication.  You can use
 the synchronization primitives from the :mod:`threading` module instead.
 
-Besides, only the main thread is allowed to set a new signal handler.
+Besides, only the main thread of the main interpreter is allowed to set a new signal handler.
 
 
 Module contents
@@ -266,7 +266,7 @@ The :mod:`signal` module defines the following functions:
    same process as the caller.  The target thread can be executing any code
    (Python or not).  However, if the target thread is executing the Python
    interpreter, the Python signal handlers will be :ref:`executed by the main
-   thread <signals-and-threads>`.  Therefore, the only point of sending a
+   thread of the main interpreter <signals-and-threads>`.  Therefore, the only point of sending a
    signal to a particular Python thread would be to force a running system call
    to fail with :exc:`InterruptedError`.
 
@@ -360,7 +360,8 @@ The :mod:`signal` module defines the following functions:
    If not -1, *fd* must be non-blocking.  It is up to the library to remove
    any bytes from *fd* before calling poll or select again.
 
-   When threads are enabled, this function can only be called from the main thread;
+   When threads are enabled, this function can only be called
+   from :ref:`the main thread of the main interpreter <signals-and-threads>`;
    attempting to call it from other threads will cause a :exc:`ValueError`
    exception to be raised.
 
@@ -413,7 +414,8 @@ The :mod:`signal` module defines the following functions:
    signal handler will be returned (see the description of :func:`getsignal`
    above).  (See the Unix man page :manpage:`signal(2)` for further information.)
 
-   When threads are enabled, this function can only be called from the main thread;
+   When threads are enabled, this function can only be called
+   from :ref:`the main thread of the main interpreter <signals-and-threads>`;
    attempting to call it from other threads will cause a :exc:`ValueError`
    exception to be raised.
 

diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index 92eeac762fd192ca454e69d430b62bac6b08508b..da034e185a0175d8cd30aac853f9f42425465420 100644 (file)
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -294,7 +294,33 @@ _PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
     _Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);
 }
 
-PyAPI_FUNC(int) _Py_IsMainInterpreter(PyThreadState* tstate);
+/* Check if the current thread is the main thread.
+   Use _Py_IsMainInterpreter() to check if it's the main interpreter. */
+static inline int
+_Py_IsMainThread(void)
+{
+    unsigned long thread = PyThread_get_thread_ident();
+    return (thread == _PyRuntime.main_thread);
+}
+
+
+static inline int
+_Py_IsMainInterpreter(PyThreadState* tstate)
+{
+    /* Use directly _PyRuntime rather than tstate->interp->runtime, since
+       this function is used in performance critical code path (ceval) */
+    return (tstate->interp == _PyRuntime.interpreters.main);
+}
+
+
+/* Only handle signals on the main thread of the main interpreter. */
+static inline int
+_Py_ThreadCanHandleSignals(PyThreadState *tstate)
+{
+    /* Use directly _PyRuntime rather than tstate->interp->runtime, since
+       this function is used in performance critical code path (ceval) */
+    return (_Py_IsMainThread() && _Py_IsMainInterpreter(tstate));
+}
 
 
 /* Variable and macro for in-line access to current thread

diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index a26ae7d69ae9fb707226fbe4a9100139500d22dd..fa3b01de25a834ef9d97581efd589e1c4834b6ba 100644 (file)
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -190,12 +190,10 @@ itimer_retval(struct itimerval *iv)
 #endif
 
 static int
-is_main(_PyRuntimeState *runtime)
+thread_can_handle_signals(void)
 {
-    unsigned long thread = PyThread_get_thread_ident();
-    PyInterpreterState *interp = _PyRuntimeState_GetThreadState(runtime)->interp;
-    return (thread == runtime->main_thread
-            && interp == runtime->interpreters.main);
+    PyThreadState *tstate = _PyThreadState_GET();
+    return _Py_ThreadCanHandleSignals(tstate);
 }
 
 static PyObject *
@@ -482,10 +480,10 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler)
     }
 #endif
 
-    _PyRuntimeState *runtime = &_PyRuntime;
-    if (!is_main(runtime)) {
+    if (!thread_can_handle_signals()) {
         PyErr_SetString(PyExc_ValueError,
-                        "signal only works in main thread");
+                        "signal only works in main thread "
+                        "of the main interpreter");
         return NULL;
     }
     if (signalnum < 1 || signalnum >= NSIG) {
@@ -700,10 +698,10 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds)
         return NULL;
 #endif
 
-    _PyRuntimeState *runtime = &_PyRuntime;
-    if (!is_main(runtime)) {
+    if (!thread_can_handle_signals()) {
         PyErr_SetString(PyExc_ValueError,
-                        "set_wakeup_fd only works in main thread");
+                        "set_wakeup_fd only works in main thread "
+                        "of the main interpreter");
         return NULL;
     }
 
@@ -1675,8 +1673,7 @@ finisignal(void)
 int
 PyErr_CheckSignals(void)
 {
-    _PyRuntimeState *runtime = &_PyRuntime;
-    if (!is_main(runtime)) {
+    if (!thread_can_handle_signals()) {
         return 0;
     }
 
@@ -1769,8 +1766,7 @@ int
 PyOS_InterruptOccurred(void)
 {
     if (_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) {
-        _PyRuntimeState *runtime = &_PyRuntime;
-        if (!is_main(runtime)) {
+        if (!thread_can_handle_signals()) {
             return 0;
         }
         _Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0);
@@ -1803,8 +1799,7 @@ _PySignal_AfterFork(void)
 int
 _PyOS_IsMainThread(void)
 {
-    _PyRuntimeState *runtime = &_PyRuntime;
-    return is_main(runtime);
+    return thread_can_handle_signals();
 }
 
 #ifdef MS_WINDOWS

diff --git a/Python/ceval.c b/Python/ceval.c
index 30599a5992cc539871dec595e675e570ea7ee8c1..86aa2253899aa716d42861b77fbc8d72b9751c98 100644 (file)
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -136,14 +136,6 @@ is_tstate_valid(PyThreadState *tstate)
 #endif
 
 
-/* Only handle signals on the main thread of the main interpreter. */
-static int
-thread_can_handle_signals(void)
-{
-    return (PyThread_get_thread_ident() == _PyRuntime.main_thread);
-}
-
-
 /* This can set eval_breaker to 0 even though gil_drop_request became
    1.  We believe this is all right because the eval loop will release
    the GIL eventually anyway. */
@@ -156,7 +148,7 @@ COMPUTE_EVAL_BREAKER(PyThreadState *tstate,
     _Py_atomic_store_relaxed(&ceval2->eval_breaker,
         _Py_atomic_load_relaxed(&ceval->gil_drop_request)
         | (_Py_atomic_load_relaxed(&ceval->signals_pending)
-           && thread_can_handle_signals())
+           && _Py_ThreadCanHandleSignals(tstate))
         | _Py_atomic_load_relaxed(&ceval2->pending.calls_to_do)
         | ceval2->pending.async_exc);
 }
@@ -598,17 +590,7 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
 static int
 handle_signals(PyThreadState *tstate)
 {
-    _PyRuntimeState *runtime = tstate->interp->runtime;
-
-    if (!thread_can_handle_signals()) {
-        return 0;
-    }
-    /*
-     * Ensure that the thread isn't currently running some other
-     * interpreter.
-     */
-    PyInterpreterState *interp = tstate->interp;
-    if (interp != runtime->interpreters.main) {
+    if (!_Py_ThreadCanHandleSignals(tstate)) {
         return 0;
     }
 
@@ -624,11 +606,8 @@ handle_signals(PyThreadState *tstate)
 static int
 make_pending_calls(PyThreadState *tstate)
 {
-
-    _PyRuntimeState *runtime = tstate->interp->runtime;
-
     /* only service pending calls on main thread */
-    if (PyThread_get_thread_ident() != runtime->main_thread) {
+    if (!_Py_IsMainThread()) {
         return 0;
     }
 

diff --git a/Python/pystate.c b/Python/pystate.c
index a792cc5622d0414cf650a3ea03683314e9298684..621318f4b5a582fe32b478930002f91d460b02ca 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -169,12 +169,6 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime)
 #define HEAD_UNLOCK(runtime) \
     PyThread_release_lock((runtime)->interpreters.mutex)
 
-int
-_Py_IsMainInterpreter(PyThreadState* tstate)
-{
-    return (tstate->interp == tstate->interp->runtime->interpreters.main);
-}
-
 /* Forward declaration */
 static void _PyGILState_NoteThreadState(
     struct _gilstate_runtime_state *gilstate, PyThreadState* tstate);