From a9cd1ff958a9f80d9730c7d98533169fdf636f72 Mon Sep 17 00:00:00 2001 From: Tekin Ertekin Date: Thu, 30 Jul 2026 18:13:28 +0300 Subject: [PATCH] gh-154916: Fix data race in ga_iter_reduce under free-threading ga_iternext takes gi->obj out with an atomic exchange and then drops the reference, while ga_iter_reduce read the same field twice without synchronisation. The two reads can straddle the exchange, so the guard can observe a non-NULL pointer that is then passed to Py_BuildValue after the owning thread has already released it. Take a single strong reference instead, and mark the stored object as maybe-weakref in ga_iter, which _Py_XGetRef requires of the writer. --- ...-07-30-15-10-00.gh-issue-154916.Kp3nQz.rst | 3 +++ Objects/genericaliasobject.c | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-07-30-15-10-00.gh-issue-154916.Kp3nQz.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-30-15-10-00.gh-issue-154916.Kp3nQz.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-30-15-10-00.gh-issue-154916.Kp3nQz.rst new file mode 100644 index 00000000000000..921458188a28fe --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-30-15-10-00.gh-issue-154916.Kp3nQz.rst @@ -0,0 +1,3 @@ +Fix a data race in :meth:`!__reduce__` of a shared :class:`types.GenericAlias` +iterator under the :term:`free-threaded build`. Follow-up to +:gh:`154043`, which only covered iteration itself. diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 348c7dd6967a39..0c7ceec37a3154 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -997,8 +997,19 @@ ga_iter_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) * call must be before access of iterator pointers. * see issue #101765 */ - if (gi->obj) - return Py_BuildValue("N(O)", iter, gi->obj); + /* ga_iternext takes gi->obj out with an atomic exchange and then drops the + * reference, so a plain read here is not just a data race: the two reads of + * gi->obj below could straddle that exchange and hand Py_BuildValue a + * pointer whose last reference is already gone. Take a strong reference + * once instead. Racing with next() may legitimately observe either the + * object or the exhausted iterator; both reductions are correct. */ +#ifdef Py_GIL_DISABLED + PyObject *obj = _Py_XGetRef(&gi->obj); +#else + PyObject *obj = Py_XNewRef(gi->obj); +#endif + if (obj != NULL) + return Py_BuildValue("N(N)", iter, obj); else return Py_BuildValue("N(())", iter); } @@ -1030,6 +1041,11 @@ ga_iter(PyObject *self) { return NULL; } gi->obj = Py_NewRef(self); +#ifdef Py_GIL_DISABLED + /* _Py_XGetRef in ga_iter_reduce needs the stored object to be flagged, or + * its try-incref cannot succeed from another thread and it would spin. */ + _PyObject_SetMaybeWeakref(self); +#endif PyObject_GC_Track(gi); return (PyObject *)gi; }