Skip to content

Commit

Permalink
pythongh-117657: Fix QSBR race condition (python#118843)
Browse files Browse the repository at this point in the history
`_Py_qsbr_unregister` is called when the PyThreadState is already
detached, so the access to `tstate->qsbr` isn't safe without locking the
shared mutex. Grab the `struct _qsbr_shared` from the interpreter
instead.
  • Loading branch information
SonicField authored May 10, 2024
1 parent 22d5185 commit 33d2019
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Include/internal/pycore_qsbr.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ _Py_qsbr_register(struct _PyThreadStateImpl *tstate,

// Disassociates a PyThreadState from the QSBR state and frees the QSBR state.
extern void
_Py_qsbr_unregister(struct _PyThreadStateImpl *tstate);
_Py_qsbr_unregister(PyThreadState *tstate);

extern void
_Py_qsbr_fini(PyInterpreterState *interp);
Expand Down
2 changes: 1 addition & 1 deletion Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1794,7 +1794,7 @@ tstate_delete_common(PyThreadState *tstate)
HEAD_UNLOCK(runtime);

#ifdef Py_GIL_DISABLED
_Py_qsbr_unregister((_PyThreadStateImpl *)tstate);
_Py_qsbr_unregister(tstate);
#endif

// XXX Unbind in PyThreadState_Clear(), or earlier
Expand Down
11 changes: 6 additions & 5 deletions Python/qsbr.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,20 +231,21 @@ _Py_qsbr_register(_PyThreadStateImpl *tstate, PyInterpreterState *interp,
}

void
_Py_qsbr_unregister(_PyThreadStateImpl *tstate)
_Py_qsbr_unregister(PyThreadState *tstate)
{
struct _qsbr_shared *shared = tstate->qsbr->shared;
struct _qsbr_shared *shared = &tstate->interp->qsbr;
struct _PyThreadStateImpl *tstate_imp = (_PyThreadStateImpl*) tstate;

PyMutex_Lock(&shared->mutex);
// NOTE: we must load (or reload) the thread state's qbsr inside the mutex
// because the array may have been resized (changing tstate->qsbr) while
// we waited to acquire the mutex.
struct _qsbr_thread_state *qsbr = tstate->qsbr;
struct _qsbr_thread_state *qsbr = tstate_imp->qsbr;

assert(qsbr->seq == 0 && "thread state must be detached");
assert(qsbr->allocated && qsbr->tstate == (PyThreadState *)tstate);
assert(qsbr->allocated && qsbr->tstate == tstate);

tstate->qsbr = NULL;
tstate_imp->qsbr = NULL;
qsbr->tstate = NULL;
qsbr->allocated = false;
qsbr->freelist_next = shared->freelist;
Expand Down
1 change: 0 additions & 1 deletion Tools/tsan/suppressions_free_threading.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ race_top:_PyParkingLot_Park
race_top:_PyType_HasFeature
race_top:assign_version_tag
race_top:gc_restore_tid
race_top:initialize_new_array
race_top:insertdict
race_top:lookup_tp_dict
race_top:mi_heap_visit_pages
Expand Down

0 comments on commit 33d2019

Please sign in to comment.