From 5e07231ebbcd5f31d25e0c648b4a7c2f6323469b Mon Sep 17 00:00:00 2001 From: charliekerfoot Date: Sat, 25 Apr 2026 19:30:28 -0500 Subject: [PATCH 1/3] Fix race / leak in pickle XID per-interpreter cae --- Python/crossinterp.c | 50 +++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/Python/crossinterp.c b/Python/crossinterp.c index 4cd4b32ef906bb..ddfd1090268bb6 100644 --- a/Python/crossinterp.c +++ b/Python/crossinterp.c @@ -575,39 +575,47 @@ _PyObject_GetXIData(PyThreadState *tstate, * cleared during interpreter finalization in _Py_xi_state_fini(). * * Note: the cached references are captured at first use and not invalidated - * on module reload. This matches the caching pattern used elsewhere in - * CPython (e.g. arraymodule.c, _decimal.c). */ + * on module reload. + * + * Population uses an atomic compare-exchange so two threads racing to + * populate the cache (possible in the free-threaded build, and also under + * the GIL when PyImport_ImportModuleAttrString releases it during module + * initialization) cannot leak references or tear the stored pointer. */ static PyObject * -_get_pickle_dumps(PyThreadState *tstate) +_cache_pickle_attr(PyObject **slot, const char *attr) { - _PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp); - PyObject *dumps = state->pickle.dumps; - if (dumps != NULL) { - return dumps; + PyObject *cached = _Py_atomic_load_ptr(slot); + if (cached != NULL) { + return cached; } - dumps = PyImport_ImportModuleAttrString("pickle", "dumps"); - if (dumps == NULL) { + PyObject *imported = PyImport_ImportModuleAttrString("pickle", attr); + if (imported == NULL) { return NULL; } - state->pickle.dumps = dumps; // owns the reference - return dumps; + PyObject *expected = NULL; + if (_Py_atomic_compare_exchange_ptr(slot, &expected, imported)) { + // We won the race; the slot now owns our reference. + return imported; + } + // Another thread populated the slot first. Drop our reference and + // return the winner's (still owned by the slot). + Py_DECREF(imported); + return expected; +} + +static PyObject * +_get_pickle_dumps(PyThreadState *tstate) +{ + _PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp); + return _cache_pickle_attr(&state->pickle.dumps, "dumps"); } static PyObject * _get_pickle_loads(PyThreadState *tstate) { _PyXI_state_t *state = _PyXI_GET_STATE(tstate->interp); - PyObject *loads = state->pickle.loads; - if (loads != NULL) { - return loads; - } - loads = PyImport_ImportModuleAttrString("pickle", "loads"); - if (loads == NULL) { - return NULL; - } - state->pickle.loads = loads; // owns the reference - return loads; + return _cache_pickle_attr(&state->pickle.loads, "loads"); } struct _pickle_context { From 7bea710b723d5c54be0d1ff2042df13b3e4214fd Mon Sep 17 00:00:00 2001 From: charliekerfoot Date: Sat, 25 Apr 2026 19:46:43 -0500 Subject: [PATCH 2/3] Added NEWS entry --- .../2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst new file mode 100644 index 00000000000000..28fb5e06439972 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst @@ -0,0 +1,8 @@ +"- Issue #: " or "- gh-issue-: " or that sort of stuff. +########################################################################### + +Fix a race in the per-interpreter ``pickle.dumps``/``pickle.loads`` cache +used by cross-interpreter data transfer. Concurrent use (first time) from +multiple threads in the same interpreter could leak a reference to the +imported attribute and also potentially expose a torn pointer read. The +cache is now populated with an atomic compare-exchange. From 72a74657c771213308ec77e54271bc7359a9212f Mon Sep 17 00:00:00 2001 From: charliekerfoot Date: Sat, 25 Apr 2026 19:48:53 -0500 Subject: [PATCH 3/3] Fixed news entry --- .../2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst index 28fb5e06439972..f06625470f45fe 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-25-19-46-23.gh-issue-149000.jvDIwy.rst @@ -1,6 +1,3 @@ -"- Issue #: " or "- gh-issue-: " or that sort of stuff. -########################################################################### - Fix a race in the per-interpreter ``pickle.dumps``/``pickle.loads`` cache used by cross-interpreter data transfer. Concurrent use (first time) from multiple threads in the same interpreter could leak a reference to the