Skip to content

Commit 6cf40ec

Browse files
committed
[3.14] gh-151518: Avoid STW starvation of attaching threads
Preserve active attach waiters across successive stop-the-world pauses. Adapt the resume transition to the 3.14 stop-the-world implementation and include the regression test and free-threaded test helper. (cherry picked from commit f156510)
1 parent 32965ea commit 6cf40ec

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
@@ -114,8 +114,7 @@ struct _ts {
114114

115115
int _whence;
116116

117-
/* Thread state (_Py_THREAD_ATTACHED, _Py_THREAD_DETACHED, _Py_THREAD_SUSPENDED).
118-
See Include/internal/pycore_pystate.h for more details. */
117+
/* Thread state. See Include/internal/pycore_pystate.h for details. */
119118
int state;
120119

121120
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. They
25+
// 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. 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
@@ -28,6 +28,7 @@
2828
#include "pycore_initconfig.h" // _Py_GetConfigsAsDict()
2929
#include "pycore_instruction_sequence.h" // _PyInstructionSequence_New()
3030
#include "pycore_interpframe.h" // _PyFrame_GetFunction()
31+
#include "pycore_lock.h" // PyEvent_WaitTimed()
3132
#include "pycore_object.h" // _PyObject_IsFreed()
3233
#include "pycore_optimizer.h" // _Py_Executor_DependsOn
3334
#include "pycore_pathconfig.h" // _PyPathConfig_ClearGlobal()
@@ -138,6 +139,23 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args))
138139
return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES);
139140
}
140141

142+
static PyObject *
143+
test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args))
144+
{
145+
#ifdef Py_GIL_DISABLED
146+
PyInterpreterState *interp = _PyInterpreterState_GET();
147+
// Request consecutive pauses without running Python code between them.
148+
for (int i = 0; i < 100; i++) {
149+
_PyEval_StopTheWorld(interp);
150+
// Give detached threads time to try to reattach during the pause.
151+
PyEvent event = {0};
152+
PyEvent_WaitTimed(&event, 10 * 1000 * 1000, /*detach=*/0);
153+
_PyEval_StartTheWorld(interp);
154+
}
155+
#endif
156+
Py_RETURN_NONE;
157+
}
158+
141159
static PyObject*
142160
test_bswap(PyObject *self, PyObject *Py_UNUSED(args))
143161
{
@@ -2494,6 +2512,7 @@ static PyMethodDef module_functions[] = {
24942512
{"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS},
24952513
{"get_stack_pointer", get_stack_pointer, METH_NOARGS},
24962514
{"get_stack_margin", get_stack_margin, METH_NOARGS},
2515+
{"test_stop_the_world", test_stop_the_world, METH_NOARGS},
24972516
{"test_bswap", test_bswap, METH_NOARGS},
24982517
{"test_popcount", test_popcount, METH_NOARGS},
24992518
{"test_bit_length", test_bit_length, METH_NOARGS},

‎Python/pystate.c‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,7 +1887,10 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
18871887
if (tstate->next) {
18881888
tstate->next->prev = tstate->prev;
18891889
}
1890-
if (tstate->state != _Py_THREAD_SUSPENDED) {
1890+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
1891+
if (state != _Py_THREAD_SUSPENDED &&
1892+
state != _Py_THREAD_SUSPENDED_WAITING)
1893+
{
18911894
// Any ongoing stop-the-world request should not wait for us because
18921895
// our thread is getting deleted.
18931896
if (interp->stoptheworld.requested) {
@@ -2160,6 +2163,22 @@ tstate_try_attach(PyThreadState *tstate)
21602163
#endif
21612164
}
21622165

2166+
static int
2167+
tstate_try_attach_detached(PyThreadState *tstate, int *state)
2168+
{
2169+
#ifdef Py_GIL_DISABLED
2170+
assert(*state == _Py_THREAD_DETACHED ||
2171+
*state == _Py_THREAD_DETACHED_WAITING);
2172+
return _Py_atomic_compare_exchange_int(&tstate->state,
2173+
state,
2174+
_Py_THREAD_ATTACHED);
2175+
#else
2176+
assert(tstate->state == _Py_THREAD_DETACHED);
2177+
tstate->state = _Py_THREAD_ATTACHED;
2178+
return 1;
2179+
#endif
2180+
}
2181+
21632182
static void
21642183
tstate_set_detached(PyThreadState *tstate, int detached_state)
21652184
{
@@ -2174,10 +2193,20 @@ tstate_set_detached(PyThreadState *tstate, int detached_state)
21742193
static void
21752194
tstate_wait_attach(PyThreadState *tstate)
21762195
{
2177-
do {
2196+
for (;;) {
21782197
int state = _Py_atomic_load_int_relaxed(&tstate->state);
21792198
if (state == _Py_THREAD_SUSPENDED) {
2180-
// Wait until we're switched out of SUSPENDED to DETACHED.
2199+
// Register an active attach waiter. The next stop-the-world
2200+
// request must let this thread attach before suspending it again.
2201+
if (!_Py_atomic_compare_exchange_int(
2202+
&tstate->state, &state, _Py_THREAD_SUSPENDED_WAITING))
2203+
{
2204+
continue;
2205+
}
2206+
state = _Py_THREAD_SUSPENDED_WAITING;
2207+
}
2208+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2209+
// Park rechecks the state before sleeping, in case we were resumed.
21812210
_PyParkingLot_Park(&tstate->state, &state, sizeof(tstate->state),
21822211
/*timeout=*/-1, NULL, /*detach=*/0);
21832212
}
@@ -2186,10 +2215,13 @@ tstate_wait_attach(PyThreadState *tstate)
21862215
_PyThreadState_HangThread(tstate);
21872216
}
21882217
else {
2189-
assert(state == _Py_THREAD_DETACHED);
2218+
assert(state == _Py_THREAD_DETACHED ||
2219+
state == _Py_THREAD_DETACHED_WAITING);
2220+
if (tstate_try_attach_detached(tstate, &state)) {
2221+
return;
2222+
}
21902223
}
2191-
// Once we're back in DETACHED we can re-attach
2192-
} while (!tstate_try_attach(tstate));
2224+
}
21932225
}
21942226

21952227
void
@@ -2354,6 +2386,8 @@ park_detached_threads(struct _stoptheworld_state *stw)
23542386
_Py_FOR_EACH_STW_INTERP(stw, i) {
23552387
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
23562388
int state = _Py_atomic_load_int_relaxed(&t->state);
2389+
// DETACHED_WAITING threads remain counted until they attach and
2390+
// stop, so repeated pauses cannot prevent them from attaching.
23572391
if (state == _Py_THREAD_DETACHED) {
23582392
// Atomically transition to "suspended" if in "detached" state.
23592393
if (_Py_atomic_compare_exchange_int(
@@ -2428,6 +2462,30 @@ stop_the_world(struct _stoptheworld_state *stw)
24282462
stw->world_stopped = 1;
24292463
}
24302464

2465+
static void
2466+
tstate_resume(PyThreadState *tstate)
2467+
{
2468+
assert(tstate != _PyThreadState_GET());
2469+
int state = _Py_atomic_load_int_relaxed(&tstate->state);
2470+
int next_state;
2471+
do {
2472+
assert(state == _Py_THREAD_SUSPENDED ||
2473+
state == _Py_THREAD_SUSPENDED_WAITING);
2474+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2475+
next_state = _Py_THREAD_DETACHED_WAITING;
2476+
}
2477+
else {
2478+
next_state = _Py_THREAD_DETACHED;
2479+
}
2480+
// Retry if an attach waiter registered concurrently.
2481+
} while (!_Py_atomic_compare_exchange_int(
2482+
&tstate->state, &state, next_state));
2483+
// Wake the thread if it is parked in tstate_wait_attach().
2484+
if (state == _Py_THREAD_SUSPENDED_WAITING) {
2485+
_PyParkingLot_UnparkAll(&tstate->state);
2486+
}
2487+
}
2488+
24312489
static void
24322490
start_the_world(struct _stoptheworld_state *stw)
24332491
{
@@ -2441,10 +2499,7 @@ start_the_world(struct _stoptheworld_state *stw)
24412499
_Py_FOR_EACH_STW_INTERP(stw, i) {
24422500
_Py_FOR_EACH_TSTATE_UNLOCKED(i, t) {
24432501
if (t != stw->requester) {
2444-
assert(_Py_atomic_load_int_relaxed(&t->state) ==
2445-
_Py_THREAD_SUSPENDED);
2446-
_Py_atomic_store_int(&t->state, _Py_THREAD_DETACHED);
2447-
_PyParkingLot_UnparkAll(&t->state);
2502+
tstate_resume(t);
24482503
}
24492504
}
24502505
}

0 commit comments

Comments
 (0)