Skip to content

Commit 5c7f3aa

Browse files
committed
gh-149110: Fix race in _PyFrame_IsIncomplete for FRAME_OWNED_BY_FRAME_OBJECT frames
1 parent a7bb524 commit 5c7f3aa

6 files changed

Lines changed: 58 additions & 6 deletions

File tree

Include/internal/pycore_code.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,11 @@ _PyCode_GetTLBCFast(PyThreadState *tstate, PyCodeObject *co)
566566
{
567567
_PyCodeArray *code = _PyCode_GetTLBCArray(co);
568568
int32_t idx = ((_PyThreadStateImpl*) tstate)->tlbc_index;
569-
if (idx < code->size && code->entries[idx] != NULL) {
570-
return (_Py_CODEUNIT *) code->entries[idx];
569+
if (idx < code->size) {
570+
void *entry = _Py_atomic_load_ptr_acquire(&code->entries[idx]);
571+
if (entry != NULL) {
572+
return (_Py_CODEUNIT *) entry;
573+
}
571574
}
572575
return NULL;
573576
}

Include/internal/pycore_interpframe.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ _PyFrame_GetBytecode(_PyInterpreterFrame *f)
6161
PyCodeObject *co = _PyFrame_GetCode(f);
6262
_PyCodeArray *tlbc = _PyCode_GetTLBCArray(co);
6363
assert(f->tlbc_index >= 0 && f->tlbc_index < tlbc->size);
64-
return (_Py_CODEUNIT *)tlbc->entries[f->tlbc_index];
64+
return (_Py_CODEUNIT *)_Py_atomic_load_ptr_acquire(&tlbc->entries[f->tlbc_index]);
6565
#else
6666
return _PyCode_CODE(_PyFrame_GetCode(f));
6767
#endif
@@ -294,6 +294,12 @@ _PyFrame_IsIncomplete(_PyInterpreterFrame *frame)
294294
if (frame->owner >= FRAME_OWNED_BY_INTERPRETER) {
295295
return true;
296296
}
297+
/* Frames owned by a frame object are guaranteed complete by take_ownership().
298+
* Checking instr_ptr here would race with take_ownership() when called from
299+
* another thread (e.g. pdb walking frame->f_back cross-thread). */
300+
if (frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) {
301+
return false;
302+
}
297303
return frame->owner != FRAME_OWNED_BY_GENERATOR &&
298304
frame->instr_ptr < _PyFrame_GetBytecode(frame) +
299305
_PyFrame_GetCode(frame)->_co_firsttraceable;

Lib/test/test_free_threading/test_frame.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,40 @@ def clearer():
146146

147147
threading_helper.run_concurrently([reader, reader, clearer])
148148

149+
def test_f_back_after_thread_return(self):
150+
# gh-149110: Accessing frame.f_back cross-thread while the owning
151+
# thread is returning caused a spurious assertion failure in
152+
# PyFrame_GetBack on free-threaded builds.
153+
frames = []
154+
lock = threading.Lock()
155+
156+
def target():
157+
frame = sys._getframe()
158+
with lock:
159+
frames.append(frame)
160+
161+
def inspector():
162+
for _ in range(50):
163+
frame = None
164+
with lock:
165+
if frames:
166+
frame = frames[-1]
167+
if frame is not None:
168+
try:
169+
_ = frame.f_back
170+
except Exception:
171+
pass
172+
sys._clear_internal_caches()
173+
174+
threads = [threading.Thread(target=target) for _ in range(20)]
175+
insp = threading.Thread(target=inspector)
176+
insp.start()
177+
for t in threads:
178+
t.start()
179+
for t in threads:
180+
t.join()
181+
insp.join()
182+
149183

150184
if __name__ == "__main__":
151185
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a race condition in free-threaded builds where cross-thread frame
2+
inspection via :mod:`pdb` could cause a spurious assertion failure in
3+
:c:func:`PyFrame_GetBack`.

Objects/codeobject.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3406,7 +3406,7 @@ create_tlbc_lock_held(PyInterpreterState *interp, PyCodeObject *co, Py_ssize_t i
34063406
}
34073407
copy_code(interp, (_Py_CODEUNIT *) bc, co);
34083408
assert(tlbc->entries[idx] == NULL);
3409-
tlbc->entries[idx] = bc;
3409+
_Py_atomic_store_ptr_release(&tlbc->entries[idx], bc);
34103410
return (_Py_CODEUNIT *) bc;
34113411
}
34123412

Python/frame.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,21 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
5555
// _PyFrame_Copy takes the reference to the executable,
5656
// so we need to restore it.
5757
new_frame->f_executable = PyStackRef_DUP(new_frame->f_executable);
58-
f->f_frame = new_frame;
59-
new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
58+
/* Fix instr_ptr BEFORE setting owner = FRAME_OWNED_BY_FRAME_OBJECT,
59+
* since _PyFrame_IsIncomplete() short-circuits to false for that owner.
60+
* The original owner (FRAME_OWNED_BY_THREAD) is used for the check. */
6061
if (_PyFrame_IsIncomplete(new_frame)) {
6162
// This may be a newly-created generator or coroutine frame. Since it's
6263
// dead anyways, just pretend that the first RESUME ran:
6364
PyCodeObject *code = _PyFrame_GetCode(new_frame);
6465
new_frame->instr_ptr =
6566
_PyFrame_GetBytecode(new_frame) + code->_co_firsttraceable + 1;
6667
}
68+
/* Set owner BEFORE updating f->f_frame so any concurrent reader that
69+
* observes the new f_frame pointer also sees owner = FRAME_OWNED_BY_FRAME_OBJECT,
70+
* causing _PyFrame_IsIncomplete() to short-circuit to false. */
71+
new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
72+
f->f_frame = new_frame;
6773
assert(!_PyFrame_IsIncomplete(new_frame));
6874
assert(f->f_back == NULL);
6975
_PyInterpreterFrame *prev = _PyFrame_GetFirstComplete(frame->previous);

0 commit comments

Comments
 (0)