From 4b614d39478d4eb600b649de350ea7861557954c Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 19 Aug 2026 12:14:22 +0530 Subject: [PATCH 1/2] gh-144446: Fix thread safety of gi_frame, cr_frame and ag_frame in free-threading build --- Include/internal/pycore_interpframe.h | 3 +- .../test_free_threading/test_generators.py | 61 +++++++++++++++++++ ...-08-19-06-43-54.gh-issue-144446.k3QzXa.rst | 3 + Objects/genobject.c | 15 ++++- Python/ceval.c | 4 +- Python/frame.c | 21 +++++-- 6 files changed, 98 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst diff --git a/Include/internal/pycore_interpframe.h b/Include/internal/pycore_interpframe.h index 9809cd292995f0..812e1a28debef4 100644 --- a/Include/internal/pycore_interpframe.h +++ b/Include/internal/pycore_interpframe.h @@ -7,6 +7,7 @@ #include "pycore_code.h" // _PyCode_CODE() #include "pycore_interpframe_structs.h" // _PyInterpreterFrame +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_ACQUIRE() #include "pycore_stackref.h" // PyStackRef_AsPyObjectBorrow() #include "pycore_stats.h" // CALL_STAT_INC() @@ -344,7 +345,7 @@ _PyFrame_GetFrameObject(_PyInterpreterFrame *frame) { assert(!_PyFrame_IsIncomplete(frame)); - PyFrameObject *res = frame->frame_obj; + PyFrameObject *res = FT_ATOMIC_LOAD_PTR_ACQUIRE(frame->frame_obj); if (res != NULL) { return res; } diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 382503eebd123f..7b4fd4d10a79a0 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -155,3 +155,64 @@ def closer(): done.set() threading_helper.run_concurrently([reader, closer]) + + def test_gi_frame_teardown_race(self): + ROUNDS = 20000 + + def gen(): + yield 1 + + barrier = Barrier(2) + shared = {} + captured = [] + + def reader(): + for _ in range(ROUNDS): + barrier.wait() + frame = shared['gen'].gi_frame + if frame is not None: + captured.append(frame) + barrier.wait() + + def driver(): + for _ in range(ROUNDS): + g = gen() + next(g) + shared['gen'] = g + barrier.wait() + try: + next(g) + except StopIteration: + pass + barrier.wait() + + threading_helper.run_concurrently([reader, driver]) + shared.clear() + for frame in captured: + self.assertIsNotNone(frame.f_lineno) + + def test_concurrent_gi_frame(self): + frames = set() + def gen(): + for i in range(10000): + yield i + + g = gen() + done = threading.Event() + + def runner(): + for _ in g: + pass + done.set() + + def reader(): + while not done.is_set(): + frame = g.gi_frame + if frame: + frame.f_code + frame.f_locals + frames.add(frame) + self.assertIsNone(g.gi_frame) + + threading_helper.run_concurrently([runner, reader]) + self.assertEqual(len(frames), 1) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst new file mode 100644 index 00000000000000..86a0a5e7fd285c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst @@ -0,0 +1,3 @@ +Fix thread safety of the :attr:`generator.gi_frame`, +:attr:`coroutine.cr_frame` and :attr:`agen.ag_frame` attributes in the +free-threading build. diff --git a/Objects/genobject.c b/Objects/genobject.c index 3cdc06733363d3..fbe8462c4bea94 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -170,7 +170,9 @@ gen_clear_frame(PyGenObject *gen) _PyInterpreterFrame *frame = &gen->gi_iframe; _PyThreadState_UpdateLastProfiledFrame(_PyThreadState_GET(), frame, frame->previous); frame->previous = NULL; + Py_BEGIN_CRITICAL_SECTION(gen); _PyFrame_ClearExceptCode(frame); + Py_END_CRITICAL_SECTION(); _PyErr_ClearExcState(&gen->gi_exc_state); } @@ -960,8 +962,17 @@ _gen_getframe(PyGenObject *gen, const char *const name) if (FRAME_STATE_FINISHED(frame_state)) { Py_RETURN_NONE; } - // TODO: still not thread-safe with free threading - return _Py_XNewRef((PyObject *)_PyFrame_GetFrameObject(&gen->gi_iframe)); + PyObject *frame = NULL; + Py_BEGIN_CRITICAL_SECTION(gen); + frame_state = FT_ATOMIC_LOAD_INT8_RELAXED(gen->gi_frame_state); + if (FRAME_STATE_FINISHED(frame_state)) { + frame = Py_None; + } + else { + frame = _Py_XNewRef((PyObject *)_PyFrame_GetFrameObject(&gen->gi_iframe)); + } + Py_END_CRITICAL_SECTION(); + return frame; } static PyObject * diff --git a/Python/ceval.c b/Python/ceval.c index 3f636c3416e120..3a859c05a03724 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1996,9 +1996,11 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) assert(tstate->exc_info == &gen->gi_exc_state); tstate->exc_info = gen->gi_exc_state.previous_item; gen->gi_exc_state.previous_item = NULL; - assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); frame->previous = NULL; + Py_BEGIN_CRITICAL_SECTION(gen); + assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); _PyFrame_ClearExceptCode(frame); + Py_END_CRITICAL_SECTION(); _PyErr_ClearExcState(&gen->gi_exc_state); // gh-143939: There must not be any escaping calls between setting // the generator return kind and returning from _PyEval_EvalFrame. diff --git a/Python/frame.c b/Python/frame.c index ba8222417d208c..bef8c5e9fbc60c 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -20,7 +20,6 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg) PyFrameObject * _PyFrame_MakeAndSetFrameObject(_PyInterpreterFrame *frame) { - assert(frame->frame_obj == NULL); PyObject *exc = PyErr_GetRaisedException(); PyFrameObject *f = _PyFrame_New_NoTrack(_PyFrame_GetCode(frame)); @@ -37,10 +36,18 @@ _PyFrame_MakeAndSetFrameObject(_PyInterpreterFrame *frame) // Notice that _PyFrame_New_NoTrack() can potentially raise a MemoryError, // but it won't allocate a traceback until the frame unwinds, so we are safe // here. - assert(frame->frame_obj == NULL); assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT); f->f_frame = frame; +#ifdef Py_GIL_DISABLED + PyFrameObject *expected = NULL; + if (!_Py_atomic_compare_exchange_ptr(&frame->frame_obj, &expected, f)) { + Py_DECREF(f); + return expected; + } +#else + assert(frame->frame_obj == NULL); frame->frame_obj = f; +#endif return f; } @@ -113,9 +120,13 @@ _PyFrame_ClearExceptCode(_PyInterpreterFrame *frame) // GH-99729: Clearing this frame can expose the stack (via finalizers). It's // crucial that this frame has been unlinked, and is no longer visible: assert(_PyThreadState_GET()->current_frame != frame); - if (frame->frame_obj) { - PyFrameObject *f = frame->frame_obj; - frame->frame_obj = NULL; +#ifdef Py_GIL_DISABLED + PyFrameObject *f = _Py_atomic_exchange_ptr(&frame->frame_obj, NULL); +#else + PyFrameObject *f = frame->frame_obj; + frame->frame_obj = NULL; +#endif + if (f != NULL) { if (!_PyObject_IsUniquelyReferenced((PyObject *)f)) { take_ownership(f, frame); Py_DECREF(f); From 192814457d81d72e418ae8e5d407250c11f742c1 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 19 Aug 2026 19:36:45 +0530 Subject: [PATCH 2/2] Update Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst --- .../2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst index 86a0a5e7fd285c..66d2ef0391a45a 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-19-06-43-54.gh-issue-144446.k3QzXa.rst @@ -1,3 +1,3 @@ -Fix thread safety of the :attr:`generator.gi_frame`, -:attr:`coroutine.cr_frame` and :attr:`agen.ag_frame` attributes in the +Fix thread safety of the :attr:`!generator.gi_frame`, +:attr:`!coroutine.cr_frame` and :attr:`!agen.ag_frame` attributes in the free-threading build.