Skip to content

Commit 5e07231

Browse files
Fix race / leak in pickle XID per-interpreter cae
1 parent 85d3bcd commit 5e07231

1 file changed

Lines changed: 29 additions & 21 deletions

File tree

Python/crossinterp.c

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -575,39 +575,47 @@ _PyObject_GetXIData(PyThreadState *tstate,
575575
* cleared during interpreter finalization in _Py_xi_state_fini().
576576
*
577577
* Note: the cached references are captured at first use and not invalidated
578-
* on module reload. This matches the caching pattern used elsewhere in
579-
* CPython (e.g. arraymodule.c, _decimal.c). */
578+
* on module reload.
579+
*
580+
* Population uses an atomic compare-exchange so two threads racing to
581+
* populate the cache (possible in the free-threaded build, and also under
582+
* the GIL when PyImport_ImportModuleAttrString releases it during module
583+
* initialization) cannot leak references or tear the stored pointer. */
580584

581585
static PyObject *
582-
_get_pickle_dumps(PyThreadState *tstate)
586+
_cache_pickle_attr(PyObject **slot, const char *attr)
583587
{
584-
_PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp);
585-
PyObject *dumps = state->pickle.dumps;
586-
if (dumps != NULL) {
587-
return dumps;
588+
PyObject *cached = _Py_atomic_load_ptr(slot);
589+
if (cached != NULL) {
590+
return cached;
588591
}
589-
dumps = PyImport_ImportModuleAttrString("pickle", "dumps");
590-
if (dumps == NULL) {
592+
PyObject *imported = PyImport_ImportModuleAttrString("pickle", attr);
593+
if (imported == NULL) {
591594
return NULL;
592595
}
593-
state->pickle.dumps = dumps; // owns the reference
594-
return dumps;
596+
PyObject *expected = NULL;
597+
if (_Py_atomic_compare_exchange_ptr(slot, &expected, imported)) {
598+
// We won the race; the slot now owns our reference.
599+
return imported;
600+
}
601+
// Another thread populated the slot first. Drop our reference and
602+
// return the winner's (still owned by the slot).
603+
Py_DECREF(imported);
604+
return expected;
605+
}
606+
607+
static PyObject *
608+
_get_pickle_dumps(PyThreadState *tstate)
609+
{
610+
_PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp);
611+
return _cache_pickle_attr(&state->pickle.dumps, "dumps");
595612
}
596613

597614
static PyObject *
598615
_get_pickle_loads(PyThreadState *tstate)
599616
{
600617
_PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp);
601-
PyObject *loads = state->pickle.loads;
602-
if (loads != NULL) {
603-
return loads;
604-
}
605-
loads = PyImport_ImportModuleAttrString("pickle", "loads");
606-
if (loads == NULL) {
607-
return NULL;
608-
}
609-
state->pickle.loads = loads; // owns the reference
610-
return loads;
618+
return _cache_pickle_attr(&state->pickle.loads, "loads");
611619
}
612620

613621
struct _pickle_context {

0 commit comments

Comments
 (0)