Skip to content

Commit b1889c7

Browse files
committed
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.
1 parent efcfb1a commit b1889c7

6 files changed

Lines changed: 107 additions & 19 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: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ 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, such as
25+
// for cyclic garbage collection. They are only used in `--disable-gil` builds.
26+
// They are similar to the "detached" state in that the thread is not allowed
27+
// to call most Python APIs. However, unlike the "detached" state, a thread may
28+
// not transition itself out from a "suspended" state. Only the thread
29+
// performing a stop-the-world pause may transition a thread from a "suspended"
30+
// state back to the "detached" state.
3131
//
3232
// The "shutting down" state is used when the interpreter is being finalized.
3333
// Threads in this state can't do anything other than block the OS thread.
@@ -36,17 +36,18 @@ extern "C" {
3636
// State transition diagram:
3737
//
3838
// (bound thread) (stop-the-world thread)
39-
// [attached] <-> [detached] <-> [suspended]
39+
// [attached] <-> [detached] <-> [suspended-detached]
4040
// | ^
41-
// +---------------------------->---------------------------+
41+
// +----------------------> [suspended] ---------------------+
4242
// (bound thread)
4343
//
4444
// The (bound thread) and (stop-the-world thread) labels indicate which thread
4545
// is allowed to perform the transition.
4646
#define _Py_THREAD_DETACHED 0
4747
#define _Py_THREAD_ATTACHED 1
4848
#define _Py_THREAD_SUSPENDED 2
49-
#define _Py_THREAD_SHUTTING_DOWN 3
49+
#define _Py_THREAD_SUSPENDED_DETACHED 3
50+
#define _Py_THREAD_SHUTTING_DOWN 4
5051

5152

5253
/* Check if the current thread is the main thread.

‎Include/internal/pycore_tstate.h‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,16 @@ typedef struct _PyThreadStateImpl {
105105

106106
#ifdef Py_GIL_DISABLED
107107
// gh-144438: Add padding to ensure that the fields above don't share a
108-
// cache line with other allocations.
109-
char __padding[64];
108+
// cache line with other allocations. Reuse the first bytes of the padding
109+
// for a cold stop-the-world flag without growing the thread state.
110+
union {
111+
struct {
112+
// Set while the thread is waiting to attach after a
113+
// stop-the-world pause suspended it while detached.
114+
int stw_attach_waiting;
115+
};
116+
char __padding[64];
117+
};
110118
#endif
111119
} _PyThreadStateImpl;
112120

‎Lib/test/test_free_threading/test_gc.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import unittest
22

3+
import subprocess
4+
import sys
5+
import textwrap
36
import threading
47
from threading import Thread
58
from unittest import TestCase
69
import gc
710

11+
from test import support
812
from test.support import threading_helper
913

1014

@@ -153,6 +157,50 @@ def reader():
153157
with threading_helper.start_threads(threads):
154158
pass
155159

160+
@support.requires_subprocess()
161+
def test_tight_gc_loop_does_not_starve_attach(self):
162+
script = textwrap.dedent("""
163+
import gc
164+
import importlib
165+
import threading
166+
import time
167+
168+
modules = (
169+
"abc", "argparse", "collections", "contextlib",
170+
"decimal", "enum", "functools", "heapq",
171+
"importlib", "inspect", "itertools", "json",
172+
"math", "operator", "random", "re",
173+
)
174+
for name in modules:
175+
importlib.import_module(name)
176+
177+
stop = threading.Event()
178+
179+
def collect():
180+
while not stop.is_set():
181+
gc.collect()
182+
183+
thread = threading.Thread(target=collect, daemon=True)
184+
thread.start()
185+
time.sleep(1.0)
186+
stop.set()
187+
thread.join(5.0)
188+
if thread.is_alive():
189+
raise SystemExit("GC thread did not stop")
190+
""")
191+
proc = subprocess.run(
192+
[sys.executable, "-I", "-X", "faulthandler", "-c", script],
193+
stdout=subprocess.PIPE,
194+
stderr=subprocess.PIPE,
195+
text=True,
196+
timeout=support.SHORT_TIMEOUT,
197+
)
198+
self.assertEqual(
199+
proc.returncode,
200+
0,
201+
f"stdout:\n{proc.stdout}\nstderr:\n{proc.stderr}",
202+
)
203+
156204

157205
if __name__ == "__main__":
158206
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 race that could starve a thread reattaching
2+
after being suspended while detached.

‎Python/pystate.c‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,7 +1938,9 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19381938
if (tstate->next) {
19391939
tstate->next->prev = tstate->prev;
19401940
}
1941-
if (tstate->state != _Py_THREAD_SUSPENDED) {
1941+
if (tstate->state != _Py_THREAD_SUSPENDED &&
1942+
tstate->state != _Py_THREAD_SUSPENDED_DETACHED)
1943+
{
19421944
// Any ongoing stop-the-world request should not wait for us because
19431945
// our thread is getting deleted.
19441946
if (interp->stoptheworld.requested) {
@@ -2236,9 +2238,22 @@ tstate_set_detached(PyThreadState *tstate, int detached_state)
22362238
static void
22372239
tstate_wait_attach(PyThreadState *tstate)
22382240
{
2241+
#ifdef Py_GIL_DISABLED
2242+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
2243+
int stw_attach_waiting = 0;
2244+
#endif
22392245
do {
22402246
int state = _Py_atomic_load_int_relaxed(&tstate->state);
2241-
if (state == _Py_THREAD_SUSPENDED) {
2247+
if (state == _Py_THREAD_SUSPENDED ||
2248+
state == _Py_THREAD_SUSPENDED_DETACHED)
2249+
{
2250+
#ifdef Py_GIL_DISABLED
2251+
if (state == _Py_THREAD_SUSPENDED_DETACHED) {
2252+
stw_attach_waiting = 1;
2253+
_Py_atomic_store_int_relaxed(
2254+
&tstate_impl->stw_attach_waiting, 1);
2255+
}
2256+
#endif
22422257
// Wait until we're switched out of SUSPENDED to DETACHED.
22432258
_PyParkingLot_Park(&tstate->state, &state, sizeof(tstate->state),
22442259
/*timeout=*/-1, NULL, /*detach=*/0);
@@ -2252,6 +2267,11 @@ tstate_wait_attach(PyThreadState *tstate)
22522267
}
22532268
// Once we're back in DETACHED we can re-attach
22542269
} while (!tstate_try_attach(tstate));
2270+
#ifdef Py_GIL_DISABLED
2271+
if (stw_attach_waiting) {
2272+
_Py_atomic_store_int_relaxed(&tstate_impl->stw_attach_waiting, 0);
2273+
}
2274+
#endif
22552275
}
22562276

22572277
void
@@ -2421,9 +2441,16 @@ park_detached_threads(struct _stoptheworld_state *stw)
24212441
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
24222442
int state = _Py_atomic_load_int_relaxed(&t->state);
24232443
if (state == _Py_THREAD_DETACHED) {
2444+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)t;
2445+
if (_Py_atomic_load_int_relaxed(
2446+
&tstate_impl->stw_attach_waiting))
2447+
{
2448+
continue;
2449+
}
24242450
// Atomically transition to "suspended" if in "detached" state.
24252451
if (_Py_atomic_compare_exchange_int(
2426-
&t->state, &state, _Py_THREAD_SUSPENDED)) {
2452+
&t->state, &state,
2453+
_Py_THREAD_SUSPENDED_DETACHED)) {
24272454
num_parked++;
24282455
}
24292456
}
@@ -2508,8 +2535,11 @@ start_the_world(struct _stoptheworld_state *stw)
25082535
_Py_FOR_EACH_STW_INTERP(stw, i) {
25092536
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
25102537
if (t != stw->requester) {
2511-
assert(_Py_atomic_load_int_relaxed(&t->state) ==
2512-
_Py_THREAD_SUSPENDED);
2538+
#ifndef NDEBUG
2539+
int state = _Py_atomic_load_int_relaxed(&t->state);
2540+
assert(state == _Py_THREAD_SUSPENDED ||
2541+
state == _Py_THREAD_SUSPENDED_DETACHED);
2542+
#endif
25132543
_Py_atomic_store_int(&t->state, _Py_THREAD_DETACHED);
25142544
_PyParkingLot_UnparkAll(&t->state);
25152545
}

0 commit comments

Comments
 (0)