From 6e97a9647ae028facb392d12fc24973503693bd6 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 5 Oct 2023 15:46:33 +0000 Subject: [PATCH] gh-109549: Add new states to PyThreadState to support PEP 703 (gh-109915) This adds a new field 'state' to PyThreadState that can take on one of three values: _Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, or _Py_THREAD_GC. The "attached" and "detached" states correspond closely to acquiring and releasing the GIL. The "gc" state is current unused, but will be used to implement stop-the-world GC for --disable-gil builds in the near future. --- Include/cpython/pystate.h | 4 + Include/internal/pycore_ceval.h | 1 - Include/internal/pycore_pystate.h | 42 ++++++++++ Python/ceval_gil.c | 47 +++-------- Python/pylifecycle.c | 14 ++-- Python/pystate.c | 125 +++++++++++++++++++----------- 6 files changed, 141 insertions(+), 92 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 8fd06242bc8..40102f88550 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -102,6 +102,10 @@ struct _ts { #endif int _whence; + /* Thread state (_Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, _Py_THREAD_GC). + See Include/internal/pycore_pystate.h for more details. */ + int state; + int py_recursion_remaining; int py_recursion_limit; diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 48fee697324..d3ea3a898bb 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -121,7 +121,6 @@ extern void _PyEval_FiniGIL(PyInterpreterState *interp); extern void _PyEval_AcquireLock(PyThreadState *tstate); extern void _PyEval_ReleaseLock(PyInterpreterState *, PyThreadState *); -extern PyThreadState * _PyThreadState_SwapNoGIL(PyThreadState *); extern void _PyEval_DeactivateOpCache(void); diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index a60d949bff1..7135b1e966f 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -11,6 +11,33 @@ extern "C" { #include "pycore_runtime.h" // _PyRuntime +// Values for PyThreadState.state. A thread must be in the "attached" state +// before calling most Python APIs. If the GIL is enabled, then "attached" +// implies that the thread holds the GIL and "detached" implies that the +// thread does not hold the GIL (or is in the process of releasing it). In +// `--disable-gil` builds, multiple threads may be "attached" to the same +// interpreter at the same time. Only the "bound" thread may perform the +// transitions between "attached" and "detached" on its own PyThreadState. +// +// The "gc" state is used to implement stop-the-world pauses, such as for +// cyclic garbage collection. It is only used in `--disable-gil` builds. It is +// similar to the "detached" state, but only the thread performing a +// stop-the-world pause may transition threads between the "detached" and "gc" +// states. A thread trying to "attach" from the "gc" state will block until +// it is transitioned back to "detached" when the stop-the-world pause is +// complete. +// +// State transition diagram: +// +// (bound thread) (stop-the-world thread) +// [attached] <-> [detached] <-> [gc] +// +// See `_PyThreadState_Attach()` and `_PyThreadState_Detach()`. +#define _Py_THREAD_DETACHED 0 +#define _Py_THREAD_ATTACHED 1 +#define _Py_THREAD_GC 2 + + /* Check if the current thread is the main thread. Use _Py_IsMainInterpreter() to check if it's the main interpreter. */ static inline int @@ -104,6 +131,21 @@ _PyThreadState_GET(void) #endif } +// Attaches the current thread to the interpreter. +// +// This may block while acquiring the GIL (if the GIL is enabled) or while +// waiting for a stop-the-world pause (if the GIL is disabled). +// +// High-level code should generally call PyEval_RestoreThread() instead, which +// calls this function. +void _PyThreadState_Attach(PyThreadState *tstate); + +// Detaches the current thread from the interpreter. +// +// High-level code should generally call PyEval_SaveThread() instead, which +// calls this function. +void _PyThreadState_Detach(PyThreadState *tstate); + static inline void _Py_EnsureFuncTstateNotNULL(const char *func, PyThreadState *tstate) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 6b4ec8eed03..f237e384333 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -462,24 +462,22 @@ PyStatus _PyEval_InitGIL(PyThreadState *tstate, int own_gil) { assert(tstate->interp->ceval.gil == NULL); - int locked; if (!own_gil) { /* The interpreter will share the main interpreter's instead. */ PyInterpreterState *main_interp = _PyInterpreterState_Main(); assert(tstate->interp != main_interp); struct _gil_runtime_state *gil = main_interp->ceval.gil; init_shared_gil(tstate->interp, gil); - locked = current_thread_holds_gil(gil, tstate); + assert(!current_thread_holds_gil(gil, tstate)); } else { PyThread_init_thread(); init_own_gil(tstate->interp, &tstate->interp->_gil); - locked = 0; - } - if (!locked) { - take_gil(tstate); } + // Lock the GIL and mark the current thread as attached. + _PyThreadState_Attach(tstate); + return _PyStatus_OK(); } @@ -569,24 +567,14 @@ void PyEval_AcquireThread(PyThreadState *tstate) { _Py_EnsureTstateNotNULL(tstate); - - take_gil(tstate); - - if (_PyThreadState_SwapNoGIL(tstate) != NULL) { - Py_FatalError("non-NULL old thread state"); - } + _PyThreadState_Attach(tstate); } void PyEval_ReleaseThread(PyThreadState *tstate) { assert(_PyThreadState_CheckConsistency(tstate)); - - PyThreadState *new_tstate = _PyThreadState_SwapNoGIL(NULL); - if (new_tstate != tstate) { - Py_FatalError("wrong thread state"); - } - drop_gil(tstate->interp, tstate); + _PyThreadState_Detach(tstate); } #ifdef HAVE_FORK @@ -629,11 +617,8 @@ _PyEval_SignalAsyncExc(PyInterpreterState *interp) PyThreadState * PyEval_SaveThread(void) { - PyThreadState *tstate = _PyThreadState_SwapNoGIL(NULL); - _Py_EnsureTstateNotNULL(tstate); - - assert(gil_created(tstate->interp->ceval.gil)); - drop_gil(tstate->interp, tstate); + PyThreadState *tstate = _PyThreadState_GET(); + _PyThreadState_Detach(tstate); return tstate; } @@ -641,10 +626,7 @@ void PyEval_RestoreThread(PyThreadState *tstate) { _Py_EnsureTstateNotNULL(tstate); - - take_gil(tstate); - - _PyThreadState_SwapNoGIL(tstate); + _PyThreadState_Attach(tstate); } @@ -1015,18 +997,11 @@ _Py_HandlePending(PyThreadState *tstate) /* GIL drop request */ if (_Py_eval_breaker_bit_is_set(interp, _PY_GIL_DROP_REQUEST_BIT)) { /* Give another thread a chance */ - if (_PyThreadState_SwapNoGIL(NULL) != tstate) { - Py_FatalError("tstate mix-up"); - } - drop_gil(interp, tstate); + _PyThreadState_Detach(tstate); /* Other threads may run now */ - take_gil(tstate); - - if (_PyThreadState_SwapNoGIL(tstate) != NULL) { - Py_FatalError("orphan tstate"); - } + _PyThreadState_Attach(tstate); } /* Check for asynchronous exception. */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index eb10aa3562d..14033162377 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -661,8 +661,6 @@ pycore_create_interpreter(_PyRuntimeState *runtime, return _PyStatus_ERR("can't make first thread"); } _PyThreadState_Bind(tstate); - // XXX For now we do this before the GIL is created. - (void) _PyThreadState_SwapNoGIL(tstate); status = init_interp_create_gil(tstate, config.gil); if (_PyStatus_EXCEPTION(status)) { @@ -2060,8 +2058,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config) } _PyThreadState_Bind(tstate); - // XXX For now we do this before the GIL is created. - PyThreadState *save_tstate = _PyThreadState_SwapNoGIL(tstate); + PyThreadState *save_tstate = _PyThreadState_GET(); int has_gil = 0; /* From this point until the init_interp_create_gil() call, @@ -2073,7 +2070,7 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config) const PyConfig *src_config; if (save_tstate != NULL) { // XXX Might new_interpreter() have been called without the GIL held? - _PyEval_ReleaseLock(save_tstate->interp, save_tstate); + _PyThreadState_Detach(save_tstate); src_config = _PyInterpreterState_GetConfig(save_tstate->interp); } else @@ -2120,12 +2117,11 @@ error: *tstate_p = NULL; /* Oops, it didn't work. Undo it all. */ - PyErr_PrintEx(0); if (has_gil) { - PyThreadState_Swap(save_tstate); + _PyThreadState_Detach(tstate); } - else { - _PyThreadState_SwapNoGIL(save_tstate); + if (save_tstate != NULL) { + _PyThreadState_Attach(save_tstate); } PyThreadState_Clear(tstate); PyThreadState_Delete(tstate); diff --git a/Python/pystate.c b/Python/pystate.c index 068740828e9..a024ae7e380 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -998,6 +998,7 @@ _PyInterpreterState_Clear(PyThreadState *tstate) static inline void tstate_deactivate(PyThreadState *tstate); +static void tstate_set_detached(PyThreadState *tstate); static void zapthreads(PyInterpreterState *interp); void @@ -1011,9 +1012,7 @@ PyInterpreterState_Delete(PyInterpreterState *interp) PyThreadState *tcur = current_fast_get(runtime); if (tcur != NULL && interp == tcur->interp) { /* Unset current thread. After this, many C API calls become crashy. */ - current_fast_clear(runtime); - tstate_deactivate(tcur); - _PyEval_ReleaseLock(interp, NULL); + _PyThreadState_Detach(tcur); } zapthreads(interp); @@ -1651,6 +1650,7 @@ static void tstate_delete_common(PyThreadState *tstate) { assert(tstate->_status.cleared && !tstate->_status.finalized); + assert(tstate->state != _Py_THREAD_ATTACHED); PyInterpreterState *interp = tstate->interp; if (interp == NULL) { @@ -1711,6 +1711,7 @@ void _PyThreadState_DeleteCurrent(PyThreadState *tstate) { _Py_EnsureTstateNotNULL(tstate); + tstate_set_detached(tstate); tstate_delete_common(tstate); current_fast_clear(tstate->interp->runtime); _PyEval_ReleaseLock(tstate->interp, NULL); @@ -1867,6 +1868,79 @@ tstate_deactivate(PyThreadState *tstate) // It will still be used in PyGILState_Ensure(). } +static int +tstate_try_attach(PyThreadState *tstate) +{ +#ifdef Py_NOGIL + int expected = _Py_THREAD_DETACHED; + if (_Py_atomic_compare_exchange_int( + &tstate->state, + &expected, + _Py_THREAD_ATTACHED)) { + return 1; + } + return 0; +#else + assert(tstate->state == _Py_THREAD_DETACHED); + tstate->state = _Py_THREAD_ATTACHED; + return 1; +#endif +} + +static void +tstate_set_detached(PyThreadState *tstate) +{ + assert(tstate->state == _Py_THREAD_ATTACHED); +#ifdef Py_NOGIL + _Py_atomic_store_int(&tstate->state, _Py_THREAD_DETACHED); +#else + tstate->state = _Py_THREAD_DETACHED; +#endif +} + +void +_PyThreadState_Attach(PyThreadState *tstate) +{ +#if defined(Py_DEBUG) + // This is called from PyEval_RestoreThread(). Similar + // to it, we need to ensure errno doesn't change. + int err = errno; +#endif + + _Py_EnsureTstateNotNULL(tstate); + if (current_fast_get(&_PyRuntime) != NULL) { + Py_FatalError("non-NULL old thread state"); + } + + _PyEval_AcquireLock(tstate); + + // XXX assert(tstate_is_alive(tstate)); + current_fast_set(&_PyRuntime, tstate); + tstate_activate(tstate); + + if (!tstate_try_attach(tstate)) { + // TODO: Once stop-the-world GC is implemented for --disable-gil builds + // this will need to wait until the GC completes. For now, this case + // should never happen. + Py_FatalError("thread attach failed"); + } + +#if defined(Py_DEBUG) + errno = err; +#endif +} + +void +_PyThreadState_Detach(PyThreadState *tstate) +{ + // XXX assert(tstate_is_alive(tstate) && tstate_is_bound(tstate)); + assert(tstate->state == _Py_THREAD_ATTACHED); + assert(tstate == current_fast_get(&_PyRuntime)); + tstate_set_detached(tstate); + tstate_deactivate(tstate); + current_fast_clear(&_PyRuntime); + _PyEval_ReleaseLock(tstate->interp, tstate); +} //---------- // other API @@ -1939,56 +2013,15 @@ PyThreadState_Get(void) return tstate; } - -static void -_swap_thread_states(_PyRuntimeState *runtime, - PyThreadState *oldts, PyThreadState *newts) -{ - // XXX Do this only if oldts != NULL? - current_fast_clear(runtime); - - if (oldts != NULL) { - // XXX assert(tstate_is_alive(oldts) && tstate_is_bound(oldts)); - tstate_deactivate(oldts); - } - - if (newts != NULL) { - // XXX assert(tstate_is_alive(newts)); - assert(tstate_is_bound(newts)); - current_fast_set(runtime, newts); - tstate_activate(newts); - } -} - -PyThreadState * -_PyThreadState_SwapNoGIL(PyThreadState *newts) -{ -#if defined(Py_DEBUG) - /* This can be called from PyEval_RestoreThread(). Similar - to it, we need to ensure errno doesn't change. - */ - int err = errno; -#endif - - PyThreadState *oldts = current_fast_get(&_PyRuntime); - _swap_thread_states(&_PyRuntime, oldts, newts); - -#if defined(Py_DEBUG) - errno = err; -#endif - return oldts; -} - PyThreadState * _PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts) { PyThreadState *oldts = current_fast_get(runtime); if (oldts != NULL) { - _PyEval_ReleaseLock(oldts->interp, oldts); + _PyThreadState_Detach(oldts); } - _swap_thread_states(runtime, oldts, newts); if (newts != NULL) { - _PyEval_AcquireLock(newts); + _PyThreadState_Attach(newts); } return oldts; }