gh-154916: Fix data races in GenericAlias using critical sections (#155634)

Author: kumaraditya303

Lib/test/test_free_threading/test_types.py

@@ -28,6 +28,34 @@ def refresh():
             *[refresh for _ in range(2)],
         ])
 
+    def test_getitem_parameters_race(self):
+        # gh-153298: ga_getitem() lazily initializes __parameters__;
+        # racing subscriptions must not race on the write or leak.
+        T = TypeVar('T')
+        for _ in range(100):
+            alias = list[T]
+
+            def subscribe():
+                self.assertEqual(alias[int], list[int])
+
+            threading_helper.run_concurrently(subscribe, nthreads=8)
+
+    def test_iter_next_reduce_race(self):
+        # gh-154916: next() clears the iterator's reference to the alias
+        # while __reduce__() reads it; the alias must not be freed in
+        # between (the iterator can hold the last reference).
+        def use(it):
+            it.__reduce__()
+            try:
+                next(it)
+            except StopIteration:
+                pass
+            it.__reduce__()
+
+        for _ in range(100):
+            it = iter(list[int])
+            threading_helper.run_concurrently(use, nthreads=8, args=(it,))
+
 
 if __name__ == "__main__":
     unittest.main()

Objects/genericaliasobject.c

@@ -2,6 +2,7 @@
 
 #include "Python.h"
 #include "pycore_ceval.h"         // _PyEval_GetBuiltin()
+#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
 #include "pycore_modsupport.h"    // _PyArg_NoKeywords()
 #include "pycore_object.h"
 #include "pycore_typevarobject.h" // _Py_typing_type_repr
@@ -578,19 +579,24 @@ PyDoc_STRVAR(genericalias__doc__,
 "For example, for t = list[int], t.__origin__ is list and t.__args__\n"
 "is (int,).");
 
+static PyObject *
+ga_parameters_lock_held(PyObject *self);
+
 static PyObject *
 ga_getitem(PyObject *self, PyObject *item)
 {
     gaobject *alias = (gaobject *)self;
     // Populate __parameters__ if needed.
-    if (alias->parameters == NULL) {
-        alias->parameters = _Py_make_parameters(alias->args);
-        if (alias->parameters == NULL) {
-            return NULL;
-        }
+    PyObject *parameters;
+    Py_BEGIN_CRITICAL_SECTION(self);
+    parameters = ga_parameters_lock_held(self);
+    Py_END_CRITICAL_SECTION();
+    if (parameters == NULL) {
+        return NULL;
     }
 
-    PyObject *newargs = _Py_subs_parameters(self, alias->args, alias->parameters, item);
+    PyObject *newargs = _Py_subs_parameters(self, alias->args, parameters, item);
+    Py_DECREF(parameters);
     if (newargs == NULL) {
         return NULL;
     }
@@ -846,6 +852,7 @@ static PyMemberDef ga_members[] = {
 static PyObject *
 ga_parameters_lock_held(PyObject *self)
 {
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
     gaobject *alias = (gaobject *)self;
     if (alias->parameters == NULL) {
         alias->parameters = _Py_make_parameters(alias->args);
@@ -942,12 +949,11 @@ static PyObject *
 ga_iternext(PyObject *op)
 {
     gaiterobject *gi = (gaiterobject*)op;
-#ifdef Py_GIL_DISABLED
-    PyObject *obj = _Py_atomic_exchange_ptr(&gi->obj, NULL);
-#else
-    PyObject* obj = gi->obj;
+    PyObject *obj;
+    Py_BEGIN_CRITICAL_SECTION(gi);
+    obj = gi->obj;
     gi->obj = NULL;
-#endif
+    Py_END_CRITICAL_SECTION();
     if (obj == NULL) {
         PyErr_SetNone(PyExc_StopIteration);
         return NULL;
@@ -997,10 +1003,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);
-    else
+    PyObject *obj;
+    Py_BEGIN_CRITICAL_SECTION(gi);
+    obj = Py_XNewRef(gi->obj);
+    Py_END_CRITICAL_SECTION();
+
+    if (obj) {
+        PyObject *result = Py_BuildValue("N(O)", iter, obj);
+        Py_DECREF(obj);
+        return result;
+    }
+    else {
         return Py_BuildValue("N(())", iter);
+    }
 }
 
 static PyMethodDef ga_iter_methods[] = {