From 5c7f3aa420f26a3d673c5ab989fe1f744105e095 Mon Sep 17 00:00:00 2001 From: Bhuvansh Kataria Date: Tue, 18 Aug 2026 11:24:08 +0000 Subject: [PATCH] gh-149110: Fix race in _PyFrame_IsIncomplete for FRAME_OWNED_BY_FRAME_OBJECT frames --- Include/internal/pycore_code.h | 7 ++-- Include/internal/pycore_interpframe.h | 8 ++++- Lib/test/test_free_threading/test_frame.py | 34 +++++++++++++++++++ ...-08-18-10-38-00.gh-issue-149110.rqYKjG.rst | 3 ++ Objects/codeobject.c | 2 +- Python/frame.c | 10 ++++-- 6 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-18-10-38-00.gh-issue-149110.rqYKjG.rst diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 293c1ea4414e23..1d194666393e02 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -566,8 +566,11 @@ _PyCode_GetTLBCFast(PyThreadState *tstate, PyCodeObject *co) { _PyCodeArray *code = _PyCode_GetTLBCArray(co); int32_t idx = ((_PyThreadStateImpl*) tstate)->tlbc_index; - if (idx < code->size && code->entries[idx] != NULL) { - return (_Py_CODEUNIT *) code->entries[idx]; + if (idx < code->size) { + void *entry = _Py_atomic_load_ptr_acquire(&code->entries[idx]); + if (entry != NULL) { + return (_Py_CODEUNIT *) entry; + } } return NULL; } diff --git a/Include/internal/pycore_interpframe.h b/Include/internal/pycore_interpframe.h index 9809cd292995f0..4aa6b477285ff4 100644 --- a/Include/internal/pycore_interpframe.h +++ b/Include/internal/pycore_interpframe.h @@ -61,7 +61,7 @@ _PyFrame_GetBytecode(_PyInterpreterFrame *f) PyCodeObject *co = _PyFrame_GetCode(f); _PyCodeArray *tlbc = _PyCode_GetTLBCArray(co); assert(f->tlbc_index >= 0 && f->tlbc_index < tlbc->size); - return (_Py_CODEUNIT *)tlbc->entries[f->tlbc_index]; + return (_Py_CODEUNIT *)_Py_atomic_load_ptr_acquire(&tlbc->entries[f->tlbc_index]); #else return _PyCode_CODE(_PyFrame_GetCode(f)); #endif @@ -294,6 +294,12 @@ _PyFrame_IsIncomplete(_PyInterpreterFrame *frame) if (frame->owner >= FRAME_OWNED_BY_INTERPRETER) { return true; } + /* Frames owned by a frame object are guaranteed complete by take_ownership(). + * Checking instr_ptr here would race with take_ownership() when called from + * another thread (e.g. pdb walking frame->f_back cross-thread). */ + if (frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) { + return false; + } return frame->owner != FRAME_OWNED_BY_GENERATOR && frame->instr_ptr < _PyFrame_GetBytecode(frame) + _PyFrame_GetCode(frame)->_co_firsttraceable; diff --git a/Lib/test/test_free_threading/test_frame.py b/Lib/test/test_free_threading/test_frame.py index bea49df557aa2c..61e3109fe38e46 100644 --- a/Lib/test/test_free_threading/test_frame.py +++ b/Lib/test/test_free_threading/test_frame.py @@ -146,6 +146,40 @@ def clearer(): threading_helper.run_concurrently([reader, reader, clearer]) + def test_f_back_after_thread_return(self): + # gh-149110: Accessing frame.f_back cross-thread while the owning + # thread is returning caused a spurious assertion failure in + # PyFrame_GetBack on free-threaded builds. + frames = [] + lock = threading.Lock() + + def target(): + frame = sys._getframe() + with lock: + frames.append(frame) + + def inspector(): + for _ in range(50): + frame = None + with lock: + if frames: + frame = frames[-1] + if frame is not None: + try: + _ = frame.f_back + except Exception: + pass + sys._clear_internal_caches() + + threads = [threading.Thread(target=target) for _ in range(20)] + insp = threading.Thread(target=inspector) + insp.start() + for t in threads: + t.start() + for t in threads: + t.join() + insp.join() + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-18-10-38-00.gh-issue-149110.rqYKjG.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-18-10-38-00.gh-issue-149110.rqYKjG.rst new file mode 100644 index 00000000000000..6ec80f32766090 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-18-10-38-00.gh-issue-149110.rqYKjG.rst @@ -0,0 +1,3 @@ +Fix a race condition in free-threaded builds where cross-thread frame +inspection via :mod:`pdb` could cause a spurious assertion failure in +:c:func:`PyFrame_GetBack`. diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 58811d63c7e318..408486c5ccf23b 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -3406,7 +3406,7 @@ create_tlbc_lock_held(PyInterpreterState *interp, PyCodeObject *co, Py_ssize_t i } copy_code(interp, (_Py_CODEUNIT *) bc, co); assert(tlbc->entries[idx] == NULL); - tlbc->entries[idx] = bc; + _Py_atomic_store_ptr_release(&tlbc->entries[idx], bc); return (_Py_CODEUNIT *) bc; } diff --git a/Python/frame.c b/Python/frame.c index ba8222417d208c..17f999f37668d7 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -55,8 +55,9 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame) // _PyFrame_Copy takes the reference to the executable, // so we need to restore it. new_frame->f_executable = PyStackRef_DUP(new_frame->f_executable); - f->f_frame = new_frame; - new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT; + /* Fix instr_ptr BEFORE setting owner = FRAME_OWNED_BY_FRAME_OBJECT, + * since _PyFrame_IsIncomplete() short-circuits to false for that owner. + * The original owner (FRAME_OWNED_BY_THREAD) is used for the check. */ if (_PyFrame_IsIncomplete(new_frame)) { // This may be a newly-created generator or coroutine frame. Since it's // dead anyways, just pretend that the first RESUME ran: @@ -64,6 +65,11 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame) new_frame->instr_ptr = _PyFrame_GetBytecode(new_frame) + code->_co_firsttraceable + 1; } + /* Set owner BEFORE updating f->f_frame so any concurrent reader that + * observes the new f_frame pointer also sees owner = FRAME_OWNED_BY_FRAME_OBJECT, + * causing _PyFrame_IsIncomplete() to short-circuit to false. */ + new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT; + f->f_frame = new_frame; assert(!_PyFrame_IsIncomplete(new_frame)); assert(f->f_back == NULL); _PyInterpreterFrame *prev = _PyFrame_GetFirstComplete(frame->previous);