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.

5 files changed, 49 insertions, 53 deletions

diff --git a/Doc/library/signal.rst b/Doc/library/signal.rst
index 8b3ab41..4353cc7 100644
--- 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 92eeac7..da034e1 100644
--- 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 a26ae7d..fa3b01d 100644
--- 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 30599a5..86aa225 100644
--- 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 a792cc5..621318f 100644
--- 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);