Skip to content

Commit 7334286

Browse files
committed
gh-155363: Fix QSBR slot leak on thread state creation failure
In the free-threaded build, new_threadstate() reserves a QSBR thread-state slot before it can still fail for other reasons, but the failure paths only called free_threadstate(), which does not know about the reservation.
1 parent 1ec5607 commit 7334286

4 files changed

Lines changed: 40 additions & 7 deletions

File tree

Include/internal/pycore_qsbr.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ _Py_qsbr_detach(struct _qsbr_thread_state *qsbr);
152152
extern Py_ssize_t
153153
_Py_qsbr_reserve(PyInterpreterState *interp);
154154

155+
// Frees a QSBR state reserved by _Py_qsbr_reserve() that was never associated
156+
// with a PyThreadState by _Py_qsbr_register().
157+
extern void
158+
_Py_qsbr_unreserve(PyInterpreterState *interp, Py_ssize_t index);
159+
155160
// Associates a PyThreadState with the QSBR state at the given index
156161
extern void
157162
_Py_qsbr_register(struct _PyThreadStateImpl *tstate,
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a leak in the :term:`free-threaded build` when creating a thread state
2+
fails after an internal QSBR slot has been reserved for it. The slot could
3+
never be reclaimed, so the QSBR array grew without bound across repeated
4+
failures.

Python/pystate.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,6 +1667,15 @@ new_threadstate(PyInterpreterState *interp, int whence)
16671667
return NULL;
16681668
}
16691669

1670+
#ifdef Py_STATS
1671+
// The PyStats structure is quite large and is allocated separated from
1672+
// tstate. This is done before reserving the QSBR and TLBC indices below
1673+
// so that a failure here does not have to give them back.
1674+
if (!_PyStats_ThreadInit(interp, tstate)) {
1675+
free_threadstate(tstate);
1676+
return NULL;
1677+
}
1678+
#endif
16701679
#ifdef Py_GIL_DISABLED
16711680
Py_ssize_t qsbr_idx = _Py_qsbr_reserve(interp);
16721681
if (qsbr_idx < 0) {
@@ -1675,13 +1684,9 @@ new_threadstate(PyInterpreterState *interp, int whence)
16751684
}
16761685
int32_t tlbc_idx = _Py_ReserveTLBCIndex(interp);
16771686
if (tlbc_idx < 0) {
1678-
free_threadstate(tstate);
1679-
return NULL;
1680-
}
1681-
#endif
1682-
#ifdef Py_STATS
1683-
// The PyStats structure is quite large and is allocated separated from tstate.
1684-
if (!_PyStats_ThreadInit(interp, tstate)) {
1687+
// free_threadstate() does not know about the QSBR entry, which is only
1688+
// reclaimed by _Py_qsbr_unregister() once _Py_qsbr_register() has run.
1689+
_Py_qsbr_unreserve(interp, qsbr_idx);
16851690
free_threadstate(tstate);
16861691
return NULL;
16871692
}

Python/qsbr.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,25 @@ _Py_qsbr_reserve(PyInterpreterState *interp)
217217
return index;
218218
}
219219

220+
void
221+
_Py_qsbr_unreserve(PyInterpreterState *interp, Py_ssize_t index)
222+
{
223+
struct _qsbr_shared *shared = &interp->qsbr;
224+
225+
PyMutex_Lock(&shared->mutex);
226+
// NOTE: we must load shared->array inside the mutex because the array may
227+
// have been resized since the entry was reserved. The index remains
228+
// valid: grow_thread_array() preserves the position of existing entries.
229+
struct _qsbr_thread_state *qsbr = &shared->array[index].qsbr;
230+
231+
assert(qsbr->allocated && qsbr->tstate == NULL);
232+
233+
qsbr->allocated = false;
234+
qsbr->freelist_next = shared->freelist;
235+
shared->freelist = qsbr;
236+
PyMutex_Unlock(&shared->mutex);
237+
}
238+
220239
void
221240
_Py_qsbr_register(_PyThreadStateImpl *tstate, PyInterpreterState *interp,
222241
Py_ssize_t index)

0 commit comments

Comments
 (0)