diff options
| author | Alex Turner <alexturner@meta.com> | 2024-05-10 14:26:35 (GMT) |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-05-10 14:26:35 (GMT) |
| commit | 33d20199af65c741bdc908a968edd8dc179b6974 (patch) | |
| tree | d9172e4a7410ea8d80f87ca939683cb83e9d00f9 | |
| parent | 22d5185308f85efa22ec1e8251c409fe1cbd9e6b (diff) | |
| download | cpython-33d20199af65c741bdc908a968edd8dc179b6974.zip cpython-33d20199af65c741bdc908a968edd8dc179b6974.tar.gz cpython-33d20199af65c741bdc908a968edd8dc179b6974.tar.bz2 | |
gh-117657: Fix QSBR race condition (#118843)
`_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.
| -rw-r--r-- | Include/internal/pycore_qsbr.h | 2 | ||||
| -rw-r--r-- | Python/pystate.c | 2 | ||||
| -rw-r--r-- | Python/qsbr.c | 11 | ||||
| -rw-r--r-- | Tools/tsan/suppressions_free_threading.txt | 1 |
4 files changed, 8 insertions, 8 deletions
diff --git a/Include/internal/pycore_qsbr.h b/Include/internal/pycore_qsbr.h index c3680a2..20e643e 100644 --- a/Include/internal/pycore_qsbr.h +++ b/Include/internal/pycore_qsbr.h @@ -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); diff --git a/Python/pystate.c b/Python/pystate.c index de6a768..0832b37 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -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 diff --git a/Python/qsbr.c b/Python/qsbr.c index d7ac8f4..1e02ff9 100644 --- a/Python/qsbr.c +++ b/Python/qsbr.c @@ -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; diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index d5f4cd7..dfa4a1f 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -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 |