Skip to content

Commit f459daa

Browse files
committed
[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 f156510)
1 parent 13b1659 commit f459daa

6 files changed

Lines changed: 145 additions & 34 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: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,31 @@ 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 in
25+
// `--disable-gil` builds.
26+
// They are similar to the "detached" state in that the thread is not allowed
27+
// to call most Python APIs. A suspended thread trying to attach marks itself
28+
// as "suspended-waiting". Only the thread responsible for suspending it may
29+
// resume it, moving it to "detached" or "detached-waiting".
30+
// A "detached-waiting" thread must attach before it can be suspended again.
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.
3434
// (See _PyThreadState_HangThread).
3535
//
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
36+
// State transitions:
37+
// Bound thread: attached <-> detached
38+
// attached -> suspended
39+
// suspended -> suspended-waiting
40+
// detached-waiting -> attached
41+
// Suspending thread: detached <-> suspended
42+
// suspended-waiting -> detached-waiting
43+
#define _Py_THREAD_DETACHED 0
44+
#define _Py_THREAD_ATTACHED 1
45+
#define _Py_THREAD_SUSPENDED 2
46+
#define _Py_THREAD_SHUTTING_DOWN 3
47+
#define _Py_THREAD_SUSPENDED_WAITING 4
48+
#define _Py_THREAD_DETACHED_WAITING 5
5049

5150

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

‎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()
@@ -202,6 +203,23 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args))
202203
return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES);
203204
}
204205

206+
static PyObject *
207+
test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args))
208+
{
209+
#ifdef Py_GIL_DISABLED
210+
PyInterpreterState *interp = _PyInterpreterState_GET();
211+
// Request consecutive pauses without running Python code between them.
212+
for (int i = 0; i < 100; i++) {
213+
_PyEval_StopTheWorld(interp);
214+
// Give detached threads time to try to reattach during the pause.
215+
PyEvent event = {0};
216+
PyEvent_WaitTimed(&event, 10 * 1000 * 1000, /*detach=*/0);
217+
_PyEval_StartTheWorld(interp);
218+
}
219+
#endif
220+
Py_RETURN_NONE;
221+
}
222+
205223
#ifdef MS_WINDOWS
206224
static const char *
207225
classify_address(uintptr_t addr, int jit_enabled, PyInterpreterState *interp)
@@ -3261,6 +3279,7 @@ static PyMethodDef module_functions[] = {
32613279
{"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS},
32623280
{"get_stack_pointer", get_stack_pointer, METH_NOARGS},
32633281
{"get_stack_margin", get_stack_margin, METH_NOARGS},
3282+
{"test_stop_the_world", test_stop_the_world, METH_NOARGS},
32643283
{"classify_stack_addresses", classify_stack_addresses, METH_VARARGS},
32653284
{"get_jit_code_ranges", get_jit_code_ranges, METH_NOARGS},
32663285
{"get_jit_backend", get_jit_backend, METH_NOARGS},

‎Python/pystate.c‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,7 +1908,10 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19081908
if (tstate->next) {
19091909
tstate->next->prev = tstate->prev;
19101910
}
1911-
if (tstate->state != _Py_THREAD_SUSPENDED) {
1911+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
1912+
if (state != _Py_THREAD_SUSPENDED &&
1913+
state != _Py_THREAD_SUSPENDED_WAITING)
1914+
{
19121915
// Any ongoing stop-the-world request should not wait for us because
19131916
// our thread is getting deleted.
19141917
if (interp->stoptheworld.requested) {
@@ -2192,6 +2195,22 @@ tstate_try_attach(PyThreadState *tstate)
21922195
#endif
21932196
}
21942197

2198+
static int
2199+
tstate_try_attach_detached(PyThreadState *tstate, int *state)
2200+
{
2201+
#ifdef Py_GIL_DISABLED
2202+
assert(*state == _Py_THREAD_DETACHED ||
2203+
*state == _Py_THREAD_DETACHED_WAITING);
2204+
return _Py_atomic_compare_exchange_int(&tstate->state,
2205+
state,
2206+
_Py_THREAD_ATTACHED);
2207+
#else
2208+
assert(tstate->state == _Py_THREAD_DETACHED);
2209+
tstate->state = _Py_THREAD_ATTACHED;
2210+
return 1;
2211+
#endif
2212+
}
2213+
21952214
static void
21962215
tstate_set_detached(PyThreadState *tstate, int detached_state)
21972216
{
@@ -2206,10 +2225,20 @@ tstate_set_detached(PyThreadState *tstate, int detached_state)
22062225
static void
22072226
tstate_wait_attach(PyThreadState *tstate)
22082227
{
2209-
do {
2228+
for (;;) {
22102229
int state = _Py_atomic_load_int_relaxed(&tstate->state);
22112230
if (state == _Py_THREAD_SUSPENDED) {
2212-
// Wait until we're switched out of SUSPENDED to DETACHED.
2231+
// Register an active attach waiter. The next stop-the-world
2232+
// request must let this thread attach before suspending it again.
2233+
if (!_Py_atomic_compare_exchange_int(
2234+
&tstate->state, &state, _Py_THREAD_SUSPENDED_WAITING))
2235+
{
2236+
continue;
2237+
}
2238+
state = _Py_THREAD_SUSPENDED_WAITING;
2239+
}
2240+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2241+
// Park rechecks the state before sleeping, in case we were resumed.
22132242
_PyParkingLot_Park(&tstate->state, &state, sizeof(tstate->state),
22142243
/*timeout=*/-1, NULL, /*detach=*/0);
22152244
}
@@ -2218,10 +2247,13 @@ tstate_wait_attach(PyThreadState *tstate)
22182247
_PyThreadState_HangThread(tstate);
22192248
}
22202249
else {
2221-
assert(state == _Py_THREAD_DETACHED);
2250+
assert(state == _Py_THREAD_DETACHED ||
2251+
state == _Py_THREAD_DETACHED_WAITING);
2252+
if (tstate_try_attach_detached(tstate, &state)) {
2253+
return;
2254+
}
22222255
}
2223-
// Once we're back in DETACHED we can re-attach
2224-
} while (!tstate_try_attach(tstate));
2256+
}
22252257
}
22262258

22272259
void
@@ -2390,6 +2422,8 @@ park_detached_threads(struct _stoptheworld_state *stw)
23902422
_Py_FOR_EACH_STW_INTERP(stw, i) {
23912423
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
23922424
int state = _Py_atomic_load_int_relaxed(&t->state);
2425+
// DETACHED_WAITING threads remain counted until they attach and
2426+
// stop, so repeated pauses cannot prevent them from attaching.
23932427
if (state == _Py_THREAD_DETACHED) {
23942428
// Atomically transition to "suspended" if in "detached" state.
23952429
if (_Py_atomic_compare_exchange_int(
@@ -2465,6 +2499,30 @@ stop_the_world(struct _stoptheworld_state *stw)
24652499
stw->world_stopped = 1;
24662500
}
24672501

2502+
static void
2503+
tstate_resume(PyThreadState *tstate)
2504+
{
2505+
assert(tstate != _PyThreadState_GET());
2506+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
2507+
int next_state;
2508+
do {
2509+
assert(state == _Py_THREAD_SUSPENDED ||
2510+
state == _Py_THREAD_SUSPENDED_WAITING);
2511+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2512+
next_state = _Py_THREAD_DETACHED_WAITING;
2513+
}
2514+
else {
2515+
next_state = _Py_THREAD_DETACHED;
2516+
}
2517+
// Retry if an attach waiter registered concurrently.
2518+
} while (!_Py_atomic_compare_exchange_int(
2519+
&tstate->state, &state, next_state));
2520+
// Wake the thread if it is parked in tstate_wait_attach().
2521+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2522+
_PyParkingLot_UnparkAll(&tstate->state);
2523+
}
2524+
}
2525+
24682526
static void
24692527
start_the_world(struct _stoptheworld_state *stw)
24702528
{
@@ -2478,10 +2536,7 @@ start_the_world(struct _stoptheworld_state *stw)
24782536
_Py_FOR_EACH_STW_INTERP(stw, i) {
24792537
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
24802538
if (t != stw->requester) {
2481-
assert(_Py_atomic_load_int_relaxed(&t->state) ==
2482-
_Py_THREAD_SUSPENDED);
2483-
_Py_atomic_store_int(&t->state, _Py_THREAD_DETACHED);
2484-
_PyParkingLot_UnparkAll(&t->state);
2539+
tstate_resume(t);
24852540
}
24862541
}
24872542
}

0 commit comments

Comments
 (0)