Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 7 additions & 1 deletion Include/internal/pycore_interpframe.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
34 changes: 34 additions & 0 deletions Lib/test/test_free_threading/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion Objects/codeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 8 additions & 2 deletions Python/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,21 @@ 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:
PyCodeObject *code = _PyFrame_GetCode(new_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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the C memory model guarantee this?
Why can't another thread see f->f_frame = new_frame before new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT?

* 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);
Expand Down
Loading