From f459daa0aa8b2e3c773e30ec0f1686d7d16736ee Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Fri, 25 Sep 2026 11:24:14 -0700 Subject: [PATCH] [3.15] gh-151518: Avoid STW starvation of attaching threads (GH-152826) Preserve active attach waiters across successive stop-the-world pauses so a tight collection loop cannot keep them from making progress. Keep the waiter-aware resume operation local to pystate.c because this branch does not have the newer BRC suspend and resume helpers. (cherry picked from commit f1565109603123c39bd2a0c6002f5ea15a62b6e7) --- Include/cpython/pystate.h | 3 +- Include/internal/pycore_pystate.h | 41 +++++----- .../test_free_threading/test_threading.py | 39 +++++++++- ...-06-16-19-20-00.gh-issue-151518.e6v0Js.rst | 2 + Modules/_testinternalcapi.c | 19 +++++ Python/pystate.c | 75 ++++++++++++++++--- 6 files changed, 145 insertions(+), 34 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-06-16-19-20-00.gh-issue-151518.e6v0Js.rst diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index f367146e262bfe8..e551f624b6289b4 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -118,8 +118,7 @@ struct _ts { int _whence; - /* Thread state (_Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, _Py_THREAD_SUSPENDED). - See Include/internal/pycore_pystate.h for more details. */ + /* Thread state. See Include/internal/pycore_pystate.h for details. */ int state; int py_recursion_remaining; diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index b299a4f10b183f4..a7b8b2896ff0317 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -21,32 +21,31 @@ extern "C" { // interpreter at the same time. Only the "bound" thread may perform the // transitions between "attached" and "detached" on its own PyThreadState. // -// The "suspended" state is used to implement stop-the-world pauses, such as -// for cyclic garbage collection. It is only used in `--disable-gil` builds. -// The "suspended" state is similar to the "detached" state in that in both -// states the thread is not allowed to call most Python APIs. However, unlike -// the "detached" state, a thread may not transition itself out from the -// "suspended" state. Only the thread performing a stop-the-world pause may -// transition a thread from the "suspended" state back to the "detached" state. +// The "suspended" states are used to implement stop-the-world pauses in +// `--disable-gil` builds. +// They are similar to the "detached" state in that the thread is not allowed +// to call most Python APIs. A suspended thread trying to attach marks itself +// as "suspended-waiting". Only the thread responsible for suspending it may +// resume it, moving it to "detached" or "detached-waiting". +// A "detached-waiting" thread must attach before it can be suspended again. // // The "shutting down" state is used when the interpreter is being finalized. // Threads in this state can't do anything other than block the OS thread. // (See _PyThreadState_HangThread). // -// State transition diagram: -// -// (bound thread) (stop-the-world thread) -// [attached] <-> [detached] <-> [suspended] -// | ^ -// +---------------------------->---------------------------+ -// (bound thread) -// -// The (bound thread) and (stop-the-world thread) labels indicate which thread -// is allowed to perform the transition. -#define _Py_THREAD_DETACHED 0 -#define _Py_THREAD_ATTACHED 1 -#define _Py_THREAD_SUSPENDED 2 -#define _Py_THREAD_SHUTTING_DOWN 3 +// State transitions: +// Bound thread: attached <-> detached +// attached -> suspended +// suspended -> suspended-waiting +// detached-waiting -> attached +// Suspending thread: detached <-> suspended +// suspended-waiting -> detached-waiting +#define _Py_THREAD_DETACHED 0 +#define _Py_THREAD_ATTACHED 1 +#define _Py_THREAD_SUSPENDED 2 +#define _Py_THREAD_SHUTTING_DOWN 3 +#define _Py_THREAD_SUSPENDED_WAITING 4 +#define _Py_THREAD_DETACHED_WAITING 5 /* Check if the current thread is the main thread. diff --git a/Lib/test/test_free_threading/test_threading.py b/Lib/test/test_free_threading/test_threading.py index b5a5ca272b9405a..ecc42db7b44f3af 100644 --- a/Lib/test/test_free_threading/test_threading.py +++ b/Lib/test/test_free_threading/test_threading.py @@ -1,5 +1,8 @@ import unittest -from test.support import threading_helper +import textwrap + +from test import support +from test.support import script_helper, threading_helper threading_helper.requires_working_threading(module=True) @@ -22,5 +25,39 @@ def mutate_thread(): threading_helper.run_concurrently([repr_thread, mutate_thread]) +class TestThreadState(unittest.TestCase): + @support.requires_subprocess() + def test_tight_stw_loop_does_not_starve_attach(self): + script = textwrap.dedent(f""" + import faulthandler + + faulthandler.dump_traceback_later({support.SHORT_TIMEOUT}, exit=True) + + import _testinternalcapi + import threading + import time + + started = threading.Event() + stop = threading.Event() + + def stop_the_world(): + _testinternalcapi.test_stop_the_world() + started.set() + while not stop.is_set(): + _testinternalcapi.test_stop_the_world() + + thread = threading.Thread(target=stop_the_world) + thread.start() + started.wait() + # Each reattachment must make progress between consecutive pauses. + for _ in range(50): + time.sleep(0.02) + stop.set() + thread.join() + faulthandler.cancel_dump_traceback_later() + """) + script_helper.assert_python_ok("-X", "gil=0", "-c", script) + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-06-16-19-20-00.gh-issue-151518.e6v0Js.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-16-19-20-00.gh-issue-151518.e6v0Js.rst new file mode 100644 index 000000000000000..5da739e8b732235 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-16-19-20-00.gh-issue-151518.e6v0Js.rst @@ -0,0 +1,2 @@ +Fix a free-threaded stop-the-world fairness issue that could starve a thread +reattaching after being suspended while detached. diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 124e06e5302f45e..9ae65a7d033b21f 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -30,6 +30,7 @@ #include "pycore_instruction_sequence.h" // _PyInstructionSequence_New() #include "pycore_interpframe.h" // _PyFrame_GetFunction() #include "pycore_jit.h" // _PyJIT_AddressInJitCode() +#include "pycore_lock.h" // PyEvent_WaitTimed() #include "pycore_object.h" // _PyObject_IsFreed() #include "pycore_optimizer.h" // _Py_Executor_DependsOn #include "pycore_pathconfig.h" // _PyPathConfig_ClearGlobal() @@ -202,6 +203,23 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args)) return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES); } +static PyObject * +test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args)) +{ +#ifdef Py_GIL_DISABLED + PyInterpreterState *interp = _PyInterpreterState_GET(); + // Request consecutive pauses without running Python code between them. + for (int i = 0; i < 100; i++) { + _PyEval_StopTheWorld(interp); + // Give detached threads time to try to reattach during the pause. + PyEvent event = {0}; + PyEvent_WaitTimed(&event, 10 * 1000 * 1000, /*detach=*/0); + _PyEval_StartTheWorld(interp); + } +#endif + Py_RETURN_NONE; +} + #ifdef MS_WINDOWS static const char * classify_address(uintptr_t addr, int jit_enabled, PyInterpreterState *interp) @@ -3261,6 +3279,7 @@ static PyMethodDef module_functions[] = { {"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS}, {"get_stack_pointer", get_stack_pointer, METH_NOARGS}, {"get_stack_margin", get_stack_margin, METH_NOARGS}, + {"test_stop_the_world", test_stop_the_world, METH_NOARGS}, {"classify_stack_addresses", classify_stack_addresses, METH_VARARGS}, {"get_jit_code_ranges", get_jit_code_ranges, METH_NOARGS}, {"get_jit_backend", get_jit_backend, METH_NOARGS}, diff --git a/Python/pystate.c b/Python/pystate.c index 1a853f740c96b5b..1797312be6f042e 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1908,7 +1908,10 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) if (tstate->next) { tstate->next->prev = tstate->prev; } - if (tstate->state != _Py_THREAD_SUSPENDED) { + int state = _Py_atomic_load_int_relaxed(&tstate->state); + if (state != _Py_THREAD_SUSPENDED && + state != _Py_THREAD_SUSPENDED_WAITING) + { // Any ongoing stop-the-world request should not wait for us because // our thread is getting deleted. if (interp->stoptheworld.requested) { @@ -2192,6 +2195,22 @@ tstate_try_attach(PyThreadState *tstate) #endif } +static int +tstate_try_attach_detached(PyThreadState *tstate, int *state) +{ +#ifdef Py_GIL_DISABLED + assert(*state == _Py_THREAD_DETACHED || + *state == _Py_THREAD_DETACHED_WAITING); + return _Py_atomic_compare_exchange_int(&tstate->state, + state, + _Py_THREAD_ATTACHED); +#else + assert(tstate->state == _Py_THREAD_DETACHED); + tstate->state = _Py_THREAD_ATTACHED; + return 1; +#endif +} + static void tstate_set_detached(PyThreadState *tstate, int detached_state) { @@ -2206,10 +2225,20 @@ tstate_set_detached(PyThreadState *tstate, int detached_state) static void tstate_wait_attach(PyThreadState *tstate) { - do { + for (;;) { int state = _Py_atomic_load_int_relaxed(&tstate->state); if (state == _Py_THREAD_SUSPENDED) { - // Wait until we're switched out of SUSPENDED to DETACHED. + // Register an active attach waiter. The next stop-the-world + // request must let this thread attach before suspending it again. + if (!_Py_atomic_compare_exchange_int( + &tstate->state, &state, _Py_THREAD_SUSPENDED_WAITING)) + { + continue; + } + state = _Py_THREAD_SUSPENDED_WAITING; + } + if (state == _Py_THREAD_SUSPENDED_WAITING) { + // Park rechecks the state before sleeping, in case we were resumed. _PyParkingLot_Park(&tstate->state, &state, sizeof(tstate->state), /*timeout=*/-1, NULL, /*detach=*/0); } @@ -2218,10 +2247,13 @@ tstate_wait_attach(PyThreadState *tstate) _PyThreadState_HangThread(tstate); } else { - assert(state == _Py_THREAD_DETACHED); + assert(state == _Py_THREAD_DETACHED || + state == _Py_THREAD_DETACHED_WAITING); + if (tstate_try_attach_detached(tstate, &state)) { + return; + } } - // Once we're back in DETACHED we can re-attach - } while (!tstate_try_attach(tstate)); + } } void @@ -2390,6 +2422,8 @@ park_detached_threads(struct _stoptheworld_state *stw) _Py_FOR_EACH_STW_INTERP(stw, i) { _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { int state = _Py_atomic_load_int_relaxed(&t->state); + // DETACHED_WAITING threads remain counted until they attach and + // stop, so repeated pauses cannot prevent them from attaching. if (state == _Py_THREAD_DETACHED) { // Atomically transition to "suspended" if in "detached" state. if (_Py_atomic_compare_exchange_int( @@ -2465,6 +2499,30 @@ stop_the_world(struct _stoptheworld_state *stw) stw->world_stopped = 1; } +static void +tstate_resume(PyThreadState *tstate) +{ + assert(tstate != _PyThreadState_GET()); + int state = _Py_atomic_load_int_relaxed(&tstate->state); + int next_state; + do { + assert(state == _Py_THREAD_SUSPENDED || + state == _Py_THREAD_SUSPENDED_WAITING); + if (state == _Py_THREAD_SUSPENDED_WAITING) { + next_state = _Py_THREAD_DETACHED_WAITING; + } + else { + next_state = _Py_THREAD_DETACHED; + } + // Retry if an attach waiter registered concurrently. + } while (!_Py_atomic_compare_exchange_int( + &tstate->state, &state, next_state)); + // Wake the thread if it is parked in tstate_wait_attach(). + if (state == _Py_THREAD_SUSPENDED_WAITING) { + _PyParkingLot_UnparkAll(&tstate->state); + } +} + static void start_the_world(struct _stoptheworld_state *stw) { @@ -2478,10 +2536,7 @@ start_the_world(struct _stoptheworld_state *stw) _Py_FOR_EACH_STW_INTERP(stw, i) { _Py_FOR_EACH_TSTATE_UNLOCKED(i, t) { if (t != stw->requester) { - assert(_Py_atomic_load_int_relaxed(&t->state) == - _Py_THREAD_SUSPENDED); - _Py_atomic_store_int(&t->state, _Py_THREAD_DETACHED); - _PyParkingLot_UnparkAll(&t->state); + tstate_resume(t); } } }