Skip to content

Commit f156510

Browse files
authored
gh-151518: Avoid STW starvation of attaching threads (#152826)
* gh-151518: Avoid STW starvation of attaching threads Free-threaded stop-the-world pauses can otherwise starve a thread trying to reattach after it was suspended while detached. A tight manual gc.collect() loop can release and immediately request the next stop-the-world pause, repeatedly parking the detached thread before it can attach and make progress. Add a distinct _Py_THREAD_SUSPENDED_DETACHED state for tstates parked from DETACHED. tstate_wait_attach() marks an attach waiter only after observing that detached-origin suspended state, and park_detached_threads() skips only those active waiters on later stop-the-world passes. The ordinary successful tstate_try_attach() path remains the baseline CAS-only path. Teach the related stop-the-world paths about both suspended states, including start_the_world() and tstate_delete_common(). Keep the new wait flag after the existing hot free-threaded _PyThreadStateImpl fields so their offsets do not move. Add a free-threaded GC regression test that runs a subprocess with a tight gc.collect() worker and verifies the main thread can reattach after sleeping and stop the worker. * gh-151518: Track attach waiters in the thread state Represent active attach waiters with suspended-waiting and detached-waiting states. Preserve waiter registration when the world resumes, and keep passive detached threads immediately parkable. Restore the thread-state padding and retain the single-CAS uncontended attach path. * gh-151518: Simplify the GC fairness regression Use explicit warmup imports and a joined non-daemon collector. Run the child through script_helper with a faulthandler watchdog so a stalled attachment still fails with a traceback. Describe the bug as a fairness issue in the NEWS entry. * gh-151518: Exercise STW fairness directly * gh-151518: Preserve attach waiters when resuming BRC suspensions Share the waiter-aware resume transition between stop-the-world pauses and biased reference count merging so a concurrently registering waiter keeps its opportunity to attach. Restore the support import needed by the new upstream GC regression and align the thread-state constants. * gh-151518: Strengthen the STW attach fairness regression Hold each test pause for 10 ms and repeat pauses in C to reduce the opportunities for a waiting thread to run between pause requests. Clarify that parking rechecks the thread state before sleeping. * gh-151518: Restrict STW test pauses to free-threaded builds STW is a no-op in GIL builds. Avoid waiting in the helper there, since WASI cannot perform the blocking futex operation. * gh-151518: Wake only registered attach waiters Avoid a parking-lot lookup when resuming a suspended thread that has not tried to reattach. Use the old state from the successful CAS so concurrent waiter registration still receives a wakeup.
1 parent 68d86eb commit f156510

6 files changed

Lines changed: 142 additions & 39 deletions

File tree

‎Include/cpython/pystate.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,7 @@ struct _ts {
118118

119119
int _whence;
120120

121-
/* Thread state (_Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, _Py_THREAD_SUSPENDED).
122-
See Include/internal/pycore_pystate.h for more details. */
121+
/* Thread state. See Include/internal/pycore_pystate.h for details. */
123122
int state;
124123

125124
int py_recursion_remaining;

‎Include/internal/pycore_pystate.h‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,32 @@ extern "C" {
2121
// interpreter at the same time. Only the "bound" thread may perform the
2222
// transitions between "attached" and "detached" on its own PyThreadState.
2323
//
24-
// The "suspended" state is used to implement stop-the-world pauses, such as
25-
// for cyclic garbage collection. It is only used in `--disable-gil` builds.
26-
// The "suspended" state is similar to the "detached" state in that in both
27-
// states the thread is not allowed to call most Python APIs. However, unlike
28-
// the "detached" state, a thread may not transition itself out from the
29-
// "suspended" state. Only the thread performing a stop-the-world pause may
30-
// transition a thread from the "suspended" state back to the "detached" state.
24+
// The "suspended" states are used to implement stop-the-world pauses and to
25+
// merge biased reference counts on behalf of detached threads. They are only
26+
// used in `--disable-gil` builds.
27+
// They are similar to the "detached" state in that the thread is not allowed
28+
// to call most Python APIs. A suspended thread trying to attach marks itself
29+
// as "suspended-waiting". Only the thread responsible for suspending it may
30+
// resume it, moving it to "detached" or "detached-waiting".
31+
// A "detached-waiting" thread must attach before it can be suspended again.
3132
//
3233
// The "shutting down" state is used when the interpreter is being finalized.
3334
// Threads in this state can't do anything other than block the OS thread.
3435
// (See _PyThreadState_HangThread).
3536
//
36-
// State transition diagram:
37-
//
38-
// (bound thread) (stop-the-world thread)
39-
// [attached] <-> [detached] <-> [suspended]
40-
// | ^
41-
// +---------------------------->---------------------------+
42-
// (bound thread)
43-
//
44-
// The (bound thread) and (stop-the-world thread) labels indicate which thread
45-
// is allowed to perform the transition.
46-
#define _Py_THREAD_DETACHED 0
47-
#define _Py_THREAD_ATTACHED 1
48-
#define _Py_THREAD_SUSPENDED 2
49-
#define _Py_THREAD_SHUTTING_DOWN 3
37+
// State transitions:
38+
// Bound thread: attached <-> detached
39+
// attached -> suspended
40+
// suspended -> suspended-waiting
41+
// detached-waiting -> attached
42+
// Suspending thread: detached <-> suspended
43+
// suspended-waiting -> detached-waiting
44+
#define _Py_THREAD_DETACHED 0
45+
#define _Py_THREAD_ATTACHED 1
46+
#define _Py_THREAD_SUSPENDED 2
47+
#define _Py_THREAD_SHUTTING_DOWN 3
48+
#define _Py_THREAD_SUSPENDED_WAITING 4
49+
#define _Py_THREAD_DETACHED_WAITING 5
5050

5151

5252
/* Check if the current thread is the main thread.
@@ -162,8 +162,9 @@ extern void _PyThreadState_Suspend(PyThreadState *tstate);
162162
// Returns 1 on success, 0 if the thread was not in the "detached" state.
163163
extern int _PyThreadState_TrySuspendDetached(PyThreadState *tstate);
164164

165-
// Undo a successful _PyThreadState_TrySuspendDetached(): switch the thread
166-
// back to "detached" and wake it if it is waiting to attach.
165+
// Resume a thread suspended by _PyThreadState_TrySuspendDetached() or a
166+
// stop-the-world pause: switch it back to "detached" or "detached-waiting"
167+
// and wake it if it is waiting to attach.
167168
extern void _PyThreadState_ResumeDetached(PyThreadState *tstate);
168169
#endif
169170

‎Lib/test/test_free_threading/test_threading.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import unittest
2-
from test.support import threading_helper
2+
import textwrap
3+
4+
from test import support
5+
from test.support import script_helper, threading_helper
36

47
threading_helper.requires_working_threading(module=True)
58

@@ -22,5 +25,39 @@ def mutate_thread():
2225
threading_helper.run_concurrently([repr_thread, mutate_thread])
2326

2427

28+
class TestThreadState(unittest.TestCase):
29+
@support.requires_subprocess()
30+
def test_tight_stw_loop_does_not_starve_attach(self):
31+
script = textwrap.dedent(f"""
32+
import faulthandler
33+
34+
faulthandler.dump_traceback_later({support.SHORT_TIMEOUT}, exit=True)
35+
36+
import _testinternalcapi
37+
import threading
38+
import time
39+
40+
started = threading.Event()
41+
stop = threading.Event()
42+
43+
def stop_the_world():
44+
_testinternalcapi.test_stop_the_world()
45+
started.set()
46+
while not stop.is_set():
47+
_testinternalcapi.test_stop_the_world()
48+
49+
thread = threading.Thread(target=stop_the_world)
50+
thread.start()
51+
started.wait()
52+
# Each reattachment must make progress between consecutive pauses.
53+
for _ in range(50):
54+
time.sleep(0.02)
55+
stop.set()
56+
thread.join()
57+
faulthandler.cancel_dump_traceback_later()
58+
""")
59+
script_helper.assert_python_ok("-X", "gil=0", "-c", script)
60+
61+
2562
if __name__ == "__main__":
2663
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a free-threaded stop-the-world fairness issue that could starve a thread
2+
reattaching after being suspended while detached.

‎Modules/_testinternalcapi.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "pycore_instruction_sequence.h" // _PyInstructionSequence_New()
3131
#include "pycore_interpframe.h" // _PyFrame_GetFunction()
3232
#include "pycore_jit.h" // _PyJIT_AddressInJitCode()
33+
#include "pycore_lock.h" // PyEvent_WaitTimed()
3334
#include "pycore_object.h" // _PyObject_IsFreed()
3435
#include "pycore_optimizer.h" // _Py_Executor_DependsOn
3536
#include "pycore_pathconfig.h" // _PyPathConfig_ClearGlobal()
@@ -208,6 +209,23 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args))
208209
return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES);
209210
}
210211

212+
static PyObject *
213+
test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args))
214+
{
215+
#ifdef Py_GIL_DISABLED
216+
PyInterpreterState *interp = _PyInterpreterState_GET();
217+
// Request consecutive pauses without running Python code between them.
218+
for (int i = 0; i < 100; i++) {
219+
_PyEval_StopTheWorld(interp);
220+
// Give detached threads time to try to reattach during the pause.
221+
PyEvent event = {0};
222+
PyEvent_WaitTimed(&event, 10 * 1000 * 1000, /*detach=*/0);
223+
_PyEval_StartTheWorld(interp);
224+
}
225+
#endif
226+
Py_RETURN_NONE;
227+
}
228+
211229
#ifdef MS_WINDOWS
212230
static const char *
213231
classify_address(uintptr_t addr, int jit_enabled, PyInterpreterState *interp)
@@ -3298,6 +3316,7 @@ static PyMethodDef module_functions[] = {
32983316
{"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS},
32993317
{"get_stack_pointer", get_stack_pointer, METH_NOARGS},
33003318
{"get_stack_margin", get_stack_margin, METH_NOARGS},
3319+
{"test_stop_the_world", test_stop_the_world, METH_NOARGS},
33013320
{"classify_stack_addresses", classify_stack_addresses, METH_VARARGS},
33023321
{"get_jit_code_ranges", get_jit_code_ranges, METH_NOARGS},
33033322
{"get_jit_backend", get_jit_backend, METH_NOARGS},

‎Python/pystate.c‎

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,10 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19391939
if (tstate->next) {
19401940
tstate->next->prev = tstate->prev;
19411941
}
1942-
if (tstate->state != _Py_THREAD_SUSPENDED) {
1942+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
1943+
if (state != _Py_THREAD_SUSPENDED &&
1944+
state != _Py_THREAD_SUSPENDED_WAITING)
1945+
{
19431946
// Any ongoing stop-the-world request should not wait for us because
19441947
// our thread is getting deleted.
19451948
if (interp->stoptheworld.requested) {
@@ -2223,6 +2226,22 @@ tstate_try_attach(PyThreadState *tstate)
22232226
#endif
22242227
}
22252228

2229+
static int
2230+
tstate_try_attach_detached(PyThreadState *tstate, int *state)
2231+
{
2232+
#ifdef Py_GIL_DISABLED
2233+
assert(*state == _Py_THREAD_DETACHED ||
2234+
*state == _Py_THREAD_DETACHED_WAITING);
2235+
return _Py_atomic_compare_exchange_int(&tstate->state,
2236+
state,
2237+
_Py_THREAD_ATTACHED);
2238+
#else
2239+
assert(tstate->state == _Py_THREAD_DETACHED);
2240+
tstate->state = _Py_THREAD_ATTACHED;
2241+
return 1;
2242+
#endif
2243+
}
2244+
22262245
static void
22272246
tstate_set_detached(PyThreadState *tstate, int detached_state)
22282247
{
@@ -2237,10 +2256,20 @@ tstate_set_detached(PyThreadState *tstate, int detached_state)
22372256
static void
22382257
tstate_wait_attach(PyThreadState *tstate)
22392258
{
2240-
do {
2259+
for (;;) {
22412260
int state = _Py_atomic_load_int_relaxed(&tstate->state);
22422261
if (state == _Py_THREAD_SUSPENDED) {
2243-
// Wait until we're switched out of SUSPENDED to DETACHED.
2262+
// Register an active attach waiter. The next stop-the-world
2263+
// request must let this thread attach before suspending it again.
2264+
if (!_Py_atomic_compare_exchange_int(
2265+
&tstate->state, &state, _Py_THREAD_SUSPENDED_WAITING))
2266+
{
2267+
continue;
2268+
}
2269+
state = _Py_THREAD_SUSPENDED_WAITING;
2270+
}
2271+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2272+
// Park rechecks the state before sleeping, in case we were resumed.
22442273
_PyParkingLot_Park(&tstate->state, &state, sizeof(tstate->state),
22452274
/*timeout=*/-1, NULL, /*detach=*/0);
22462275
}
@@ -2249,10 +2278,13 @@ tstate_wait_attach(PyThreadState *tstate)
22492278
_PyThreadState_HangThread(tstate);
22502279
}
22512280
else {
2252-
assert(state == _Py_THREAD_DETACHED);
2281+
assert(state == _Py_THREAD_DETACHED ||
2282+
state == _Py_THREAD_DETACHED_WAITING);
2283+
if (tstate_try_attach_detached(tstate, &state)) {
2284+
return;
2285+
}
22532286
}
2254-
// Once we're back in DETACHED we can re-attach
2255-
} while (!tstate_try_attach(tstate));
2287+
}
22562288
}
22572289

22582290
void
@@ -2394,10 +2426,24 @@ void
23942426
_PyThreadState_ResumeDetached(PyThreadState *tstate)
23952427
{
23962428
assert(tstate != _PyThreadState_GET());
2397-
assert(_Py_atomic_load_int_relaxed(&tstate->state) == _Py_THREAD_SUSPENDED);
2398-
_Py_atomic_store_int(&tstate->state, _Py_THREAD_DETACHED);
2429+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
2430+
int next_state;
2431+
do {
2432+
assert(state == _Py_THREAD_SUSPENDED ||
2433+
state == _Py_THREAD_SUSPENDED_WAITING);
2434+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2435+
next_state = _Py_THREAD_DETACHED_WAITING;
2436+
}
2437+
else {
2438+
next_state = _Py_THREAD_DETACHED;
2439+
}
2440+
// Retry if an attach waiter registered concurrently.
2441+
} while (!_Py_atomic_compare_exchange_int(
2442+
&tstate->state, &state, next_state));
23992443
// Wake the thread if it is parked in tstate_wait_attach().
2400-
_PyParkingLot_UnparkAll(&tstate->state);
2444+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2445+
_PyParkingLot_UnparkAll(&tstate->state);
2446+
}
24012447
}
24022448
#endif
24032449

@@ -2442,6 +2488,8 @@ park_detached_threads(struct _stoptheworld_state *stw)
24422488
_Py_FOR_EACH_STW_INTERP(stw, i) {
24432489
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
24442490
int state = _Py_atomic_load_int_relaxed(&t->state);
2491+
// DETACHED_WAITING threads remain counted until they attach and
2492+
// stop, so repeated pauses cannot prevent them from attaching.
24452493
if (state == _Py_THREAD_DETACHED) {
24462494
// Atomically transition to "suspended" if in "detached" state.
24472495
if (_Py_atomic_compare_exchange_int(
@@ -2530,10 +2578,7 @@ start_the_world(struct _stoptheworld_state *stw)
25302578
_Py_FOR_EACH_STW_INTERP(stw, i) {
25312579
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
25322580
if (t != stw->requester) {
2533-
assert(_Py_atomic_load_int_relaxed(&t->state) ==
2534-
_Py_THREAD_SUSPENDED);
2535-
_Py_atomic_store_int(&t->state, _Py_THREAD_DETACHED);
2536-
_PyParkingLot_UnparkAll(&t->state);
2581+
_PyThreadState_ResumeDetached(t);
25372582
}
25382583
}
25392584
}

0 commit comments

Comments
 (0)