From d2a8e5b42c5e9c4e745a0589043a8aebb49f8ca2 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 20 Mar 2020 13:38:58 +0100 Subject: 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. --- Doc/library/signal.rst | 12 +++++++----- Include/internal/pycore_pystate.h | 28 +++++++++++++++++++++++++++- Modules/signalmodule.c | 29 ++++++++++++----------------- Python/ceval.c | 27 +++------------------------ Python/pystate.c | 6 ------ 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 `. Therefore, the only point of sending a + thread of the main interpreter `. 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 `; 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 `; 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); -- cgit v0.12