Skip to content

Commit a9cd1ff

Browse files
committed
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.
1 parent 405daf5 commit a9cd1ff

2 files changed

Lines changed: 21 additions & 2 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a data race in :meth:`!__reduce__` of a shared :class:`types.GenericAlias`
2+
iterator under the :term:`free-threaded build`. Follow-up to
3+
:gh:`154043`, which only covered iteration itself.

Objects/genericaliasobject.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -997,8 +997,19 @@ ga_iter_reduce(PyObject *self, PyObject *Py_UNUSED(ignored))
997997
* call must be before access of iterator pointers.
998998
* see issue #101765 */
999999

1000-
if (gi->obj)
1001-
return Py_BuildValue("N(O)", iter, gi->obj);
1000+
/* ga_iternext takes gi->obj out with an atomic exchange and then drops the
1001+
* reference, so a plain read here is not just a data race: the two reads of
1002+
* gi->obj below could straddle that exchange and hand Py_BuildValue a
1003+
* pointer whose last reference is already gone. Take a strong reference
1004+
* once instead. Racing with next() may legitimately observe either the
1005+
* object or the exhausted iterator; both reductions are correct. */
1006+
#ifdef Py_GIL_DISABLED
1007+
PyObject *obj = _Py_XGetRef(&gi->obj);
1008+
#else
1009+
PyObject *obj = Py_XNewRef(gi->obj);
1010+
#endif
1011+
if (obj != NULL)
1012+
return Py_BuildValue("N(N)", iter, obj);
10021013
else
10031014
return Py_BuildValue("N(())", iter);
10041015
}
@@ -1030,6 +1041,11 @@ ga_iter(PyObject *self) {
10301041
return NULL;
10311042
}
10321043
gi->obj = Py_NewRef(self);
1044+
#ifdef Py_GIL_DISABLED
1045+
/* _Py_XGetRef in ga_iter_reduce needs the stored object to be flagged, or
1046+
* its try-incref cannot succeed from another thread and it would spin. */
1047+
_PyObject_SetMaybeWeakref(self);
1048+
#endif
10331049
PyObject_GC_Track(gi);
10341050
return (PyObject *)gi;
10351051
}

0 commit comments

Comments
 (0)