gh-120321: Make gi_frame_state transitions atomic in FT build (gh-142599)

This makes generator frame state transitions atomic in the free
threading build, which avoids segfaults when trying to execute
a generator from multiple threads concurrently.

There are still a few operations that aren't thread-safe and may crash
if performed concurrently on the same generator/coroutine:

 * Accessing gi_yieldfrom/cr_await/ag_await
 * Accessing gi_frame/cr_frame/ag_frame
 * Async generator operations
This commit is contained in:
Sam Gross 2025-12-19 14:10:37 -05:00 committed by GitHub
parent e2a7db7175
commit 08bc03ff2a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 1124 additions and 883 deletions

View file

@ -1312,14 +1312,13 @@ dummy_func(
assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
if ((tstate->interp->eval_frame == NULL) &&
(Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) &&
((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING)
gen_try_set_executing((PyGenObject *)receiver_o))
{
PyGenObject *gen = (PyGenObject *)receiver_o;
_PyInterpreterFrame *gen_frame = &gen->gi_iframe;
_PyFrame_StackPush(gen_frame, PyStackRef_MakeHeapSafe(v));
DEAD(v);
SYNC_SP();
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX);
@ -1360,12 +1359,11 @@ dummy_func(
op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame)) {
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver);
DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type);
DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
DEOPT_IF(!gen_try_set_executing((PyGenObject *)gen));
STAT_INC(SEND, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_MakeHeapSafe(v));
DEAD(v);
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX);
@ -1389,7 +1387,6 @@ dummy_func(
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
_PyStackRef temp = retval;
DEAD(retval);
SAVE_STACK();
@ -1399,6 +1396,8 @@ dummy_func(
_PyInterpreterFrame *gen_frame = frame;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_YIELD;
FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_SUSPENDED + oparg);
/* We don't know which of these is relevant here, so keep them equal */
assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
#if TIER_ONE
@ -3405,18 +3404,10 @@ dummy_func(
op(_FOR_ITER_GEN_FRAME, (iter, null -- iter, null, gen_frame)) {
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter);
DEOPT_IF(Py_TYPE(gen) != &PyGen_Type);
#ifdef Py_GIL_DISABLED
// Since generators can't be used by multiple threads anyway we
// don't need to deopt here, but this lets us work on making
// generators thread-safe without necessarily having to
// specialize them thread-safely as well.
DEOPT_IF(!_PyObject_IsUniquelyReferenced((PyObject *)gen));
#endif
DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
DEOPT_IF(!gen_try_set_executing((PyGenObject *)gen));
STAT_INC(FOR_ITER, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
pushed_frame->previous = frame;

View file

@ -2304,7 +2304,8 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
{
assert(frame->owner == FRAME_OWNED_BY_GENERATOR);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
gen->gi_frame_state = FRAME_CLEARED;
FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_CLEARED);
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_RETURN;
assert(tstate->exc_info == &gen->gi_exc_state);
tstate->exc_info = gen->gi_exc_state.previous_item;
gen->gi_exc_state.previous_item = NULL;
@ -3979,15 +3980,13 @@ _PyEval_GetAwaitable(PyObject *iterable, int oparg)
Py_TYPE(iterable), oparg);
}
else if (PyCoro_CheckExact(iter)) {
PyObject *yf = _PyGen_yf((PyGenObject*)iter);
if (yf != NULL) {
/* `iter` is a coroutine object that is being
awaited, `yf` is a pointer to the current awaitable
being awaited on. */
Py_DECREF(yf);
PyCoroObject *coro = (PyCoroObject *)iter;
int8_t frame_state = FT_ATOMIC_LOAD_INT8_RELAXED(coro->cr_frame_state);
if (frame_state == FRAME_SUSPENDED_YIELD_FROM) {
/* `iter` is a coroutine object that is being awaited. */
Py_CLEAR(iter);
_PyErr_SetString(PyThreadState_GET(), PyExc_RuntimeError,
"coroutine is being awaited already");
"coroutine is being awaited already");
}
}
return iter;

View file

@ -496,3 +496,28 @@ check_periodics(PyThreadState *tstate) {
return 0;
}
// Mark the generator as executing. Returns true if the state was changed,
// false if it was already executing or finished.
static inline bool
gen_try_set_executing(PyGenObject *gen)
{
#ifdef Py_GIL_DISABLED
if (!_PyObject_IsUniquelyReferenced((PyObject *)gen)) {
int8_t frame_state = _Py_atomic_load_int8_relaxed(&gen->gi_frame_state);
while (frame_state < FRAME_EXECUTING) {
if (_Py_atomic_compare_exchange_int8(&gen->gi_frame_state,
&frame_state,
FRAME_EXECUTING)) {
return true;
}
}
}
#endif
// Use faster non-atomic modifications in the GIL-enabled build and when
// the object is uniquely referenced in the free-threaded build.
if (gen->gi_frame_state < FRAME_EXECUTING) {
gen->gi_frame_state = FRAME_EXECUTING;
return true;
}
return false;
}

View file

@ -5823,7 +5823,7 @@
SET_CURRENT_CACHED_VALUES(2);
JUMP_TO_JUMP_TARGET();
}
if (gen->gi_frame_state >= FRAME_EXECUTING) {
if (!gen_try_set_executing((PyGenObject *)gen)) {
UOP_STAT_INC(uopcode, miss);
_tos_cache1 = v;
_tos_cache0 = receiver;
@ -5833,7 +5833,6 @@
STAT_INC(SEND, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_MakeHeapSafe(v));
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
assert( 2u + oparg <= UINT16_MAX);
@ -5861,7 +5860,6 @@
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
_PyStackRef temp = retval;
_PyFrame_SetStackPointer(frame, stack_pointer);
tstate->exc_info = gen->gi_exc_state.previous_item;
@ -5870,6 +5868,8 @@
_PyInterpreterFrame *gen_frame = frame;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_YIELD;
FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_SUSPENDED + oparg);
assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
#if TIER_ONE
assert(frame->instr_ptr->op.code == INSTRUMENTED_LINE ||
@ -10859,6 +10859,81 @@
break;
}
case _FOR_ITER_GEN_FRAME_r03: {
CHECK_CURRENT_CACHED_VALUES(0);
assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
_PyStackRef iter;
_PyStackRef gen_frame;
oparg = CURRENT_OPARG();
iter = stack_pointer[-2];
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter);
if (Py_TYPE(gen) != &PyGen_Type) {
UOP_STAT_INC(uopcode, miss);
SET_CURRENT_CACHED_VALUES(0);
JUMP_TO_JUMP_TARGET();
}
if (!gen_try_set_executing((PyGenObject *)gen)) {
UOP_STAT_INC(uopcode, miss);
SET_CURRENT_CACHED_VALUES(0);
JUMP_TO_JUMP_TARGET();
}
STAT_INC(FOR_ITER, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
pushed_frame->previous = frame;
frame->return_offset = (uint16_t)( 2u + oparg);
gen_frame = PyStackRef_Wrap(pushed_frame);
_tos_cache2 = gen_frame;
_tos_cache1 = stack_pointer[-1];
_tos_cache0 = iter;
SET_CURRENT_CACHED_VALUES(3);
stack_pointer += -2;
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
break;
}
case _FOR_ITER_GEN_FRAME_r13: {
CHECK_CURRENT_CACHED_VALUES(1);
assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
_PyStackRef iter;
_PyStackRef gen_frame;
_PyStackRef _stack_item_0 = _tos_cache0;
oparg = CURRENT_OPARG();
iter = stack_pointer[-1];
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter);
if (Py_TYPE(gen) != &PyGen_Type) {
UOP_STAT_INC(uopcode, miss);
_tos_cache0 = _stack_item_0;
SET_CURRENT_CACHED_VALUES(1);
JUMP_TO_JUMP_TARGET();
}
if (!gen_try_set_executing((PyGenObject *)gen)) {
UOP_STAT_INC(uopcode, miss);
_tos_cache0 = _stack_item_0;
SET_CURRENT_CACHED_VALUES(1);
JUMP_TO_JUMP_TARGET();
}
STAT_INC(FOR_ITER, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
pushed_frame->previous = frame;
frame->return_offset = (uint16_t)( 2u + oparg);
gen_frame = PyStackRef_Wrap(pushed_frame);
_tos_cache2 = gen_frame;
_tos_cache1 = _stack_item_0;
_tos_cache0 = iter;
SET_CURRENT_CACHED_VALUES(3);
stack_pointer += -1;
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
break;
}
case _FOR_ITER_GEN_FRAME_r23: {
CHECK_CURRENT_CACHED_VALUES(2);
assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
@ -10876,17 +10951,7 @@
SET_CURRENT_CACHED_VALUES(2);
JUMP_TO_JUMP_TARGET();
}
#ifdef Py_GIL_DISABLED
if (!_PyObject_IsUniquelyReferenced((PyObject *)gen)) {
UOP_STAT_INC(uopcode, miss);
_tos_cache1 = _stack_item_1;
_tos_cache0 = iter;
SET_CURRENT_CACHED_VALUES(2);
JUMP_TO_JUMP_TARGET();
}
#endif
if (gen->gi_frame_state >= FRAME_EXECUTING) {
if (!gen_try_set_executing((PyGenObject *)gen)) {
UOP_STAT_INC(uopcode, miss);
_tos_cache1 = _stack_item_1;
_tos_cache0 = iter;
@ -10896,7 +10961,6 @@
STAT_INC(FOR_ITER, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
pushed_frame->previous = frame;

View file

@ -5548,14 +5548,7 @@
assert(_PyOpcode_Deopt[opcode] == (FOR_ITER));
JUMP_TO_PREDICTED(FOR_ITER);
}
#ifdef Py_GIL_DISABLED
if (!_PyObject_IsUniquelyReferenced((PyObject *)gen)) {
UPDATE_MISS_STATS(FOR_ITER);
assert(_PyOpcode_Deopt[opcode] == (FOR_ITER));
JUMP_TO_PREDICTED(FOR_ITER);
}
#endif
if (gen->gi_frame_state >= FRAME_EXECUTING) {
if (!gen_try_set_executing((PyGenObject *)gen)) {
UPDATE_MISS_STATS(FOR_ITER);
assert(_PyOpcode_Deopt[opcode] == (FOR_ITER));
JUMP_TO_PREDICTED(FOR_ITER);
@ -5563,7 +5556,6 @@
STAT_INC(FOR_ITER, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
pushed_frame->previous = frame;
@ -7327,7 +7319,6 @@
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
_PyStackRef temp = retval;
stack_pointer += -1;
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
@ -7338,6 +7329,8 @@
_PyInterpreterFrame *gen_frame = frame;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_YIELD;
FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_SUSPENDED + oparg);
assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
#if TIER_ONE
assert(frame->instr_ptr->op.code == INSTRUMENTED_LINE ||
@ -10301,14 +10294,13 @@
assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
if ((tstate->interp->eval_frame == NULL) &&
(Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) &&
((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING)
gen_try_set_executing((PyGenObject *)receiver_o))
{
PyGenObject *gen = (PyGenObject *)receiver_o;
_PyInterpreterFrame *gen_frame = &gen->gi_iframe;
_PyFrame_StackPush(gen_frame, PyStackRef_MakeHeapSafe(v));
stack_pointer += -1;
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
assert( 2u + oparg <= UINT16_MAX);
@ -10401,7 +10393,7 @@
assert(_PyOpcode_Deopt[opcode] == (SEND));
JUMP_TO_PREDICTED(SEND);
}
if (gen->gi_frame_state >= FRAME_EXECUTING) {
if (!gen_try_set_executing((PyGenObject *)gen)) {
UPDATE_MISS_STATS(SEND);
assert(_PyOpcode_Deopt[opcode] == (SEND));
JUMP_TO_PREDICTED(SEND);
@ -10409,7 +10401,6 @@
STAT_INC(SEND, hit);
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
_PyFrame_StackPush(pushed_frame, PyStackRef_MakeHeapSafe(v));
gen->gi_frame_state = FRAME_EXECUTING;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
assert( 2u + oparg <= UINT16_MAX);
@ -12045,7 +12036,6 @@
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
_PyStackRef temp = retval;
stack_pointer += -1;
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
@ -12056,6 +12046,8 @@
_PyInterpreterFrame *gen_frame = frame;
frame = tstate->current_frame = frame->previous;
gen_frame->previous = NULL;
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_YIELD;
FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_SUSPENDED + oparg);
assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
#if TIER_ONE
assert(frame->instr_ptr->op.code == INSTRUMENTED_LINE ||