gh-117657: Fix TSAN list set failure (#118260)

* Fix TSAN list set failure

* Relaxed atomic is sufficient, add targetted test

* More list

* Remove atomic assign in list

* Fixup white space

diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h
index 2a82912e41d55715cabb5149402e2f9f075899fb..73695d10e0c37216b44e5e209e537a4a4dcaa8b3 100644 (file)
--- a/Include/internal/pycore_list.h
+++ b/Include/internal/pycore_list.h
@@ -28,7 +28,11 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem)
     Py_ssize_t allocated = self->allocated;
     assert((size_t)len + 1 < PY_SSIZE_T_MAX);
     if (allocated > len) {
+#ifdef Py_GIL_DISABLED
+        _Py_atomic_store_ptr_release(&self->ob_item[len], newitem);
+#else
         PyList_SET_ITEM(self, len, newitem);
+#endif
         Py_SET_SIZE(self, len + 1);
         return 0;
     }

diff --git a/Lib/test/test_free_threading/test_list.py b/Lib/test/test_free_threading/test_list.py
new file mode 100644 (file)
index 0000000..79cb0a9
--- /dev/null
+++ b/Lib/test/test_free_threading/test_list.py
@@ -0,0 +1,80 @@
+import unittest
+
+from threading import Thread
+from unittest import TestCase
+
+from test.support import is_wasi
+
+
+class C:
+    def __init__(self, v):
+        self.v = v
+
+
+@unittest.skipIf(is_wasi, "WASI has no threads.")
+class TestList(TestCase):
+    def test_racing_iter_append(self):
+
+        l = []
+        OBJECT_COUNT = 10000
+
+        def writer_func():
+            for i in range(OBJECT_COUNT):
+                l.append(C(i + OBJECT_COUNT))
+
+        def reader_func():
+            while True:
+                count = len(l)
+                for i, x in enumerate(l):
+                    self.assertEqual(x.v, i + OBJECT_COUNT)
+                if count == OBJECT_COUNT:
+                    break
+
+        writer = Thread(target=writer_func)
+        readers = []
+        for x in range(30):
+            reader = Thread(target=reader_func)
+            readers.append(reader)
+            reader.start()
+
+        writer.start()
+        writer.join()
+        for reader in readers:
+            reader.join()
+
+    def test_racing_iter_extend(self):
+        iters = [
+            lambda x: [x],
+        ]
+        for iter_case in iters:
+            with self.subTest(iter=iter_case):
+                l = []
+                OBJECT_COUNT = 10000
+
+                def writer_func():
+                    for i in range(OBJECT_COUNT):
+                        l.extend(iter_case(C(i + OBJECT_COUNT)))
+
+                def reader_func():
+                    while True:
+                        count = len(l)
+                        for i, x in enumerate(l):
+                            self.assertEqual(x.v, i + OBJECT_COUNT)
+                        if count == OBJECT_COUNT:
+                            break
+
+                writer = Thread(target=writer_func)
+                readers = []
+                for x in range(30):
+                    reader = Thread(target=reader_func)
+                    readers.append(reader)
+                    reader.start()
+
+                writer.start()
+                writer.join()
+                for reader in readers:
+                    reader.join()
+
+
+if __name__ == "__main__":
+    unittest.main()

diff --git a/Objects/listobject.c b/Objects/listobject.c
index 4eaf20033fa2627c7d54198115a1a0ad1cc4dff2..3c4e2d2e6ed7ded1ba3427528dbfaef8f4755ff5 100644 (file)
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -141,6 +141,9 @@ list_resize(PyListObject *self, Py_ssize_t newsize)
             target_bytes = allocated * sizeof(PyObject*);
         }
         memcpy(array->ob_item, self->ob_item, target_bytes);
+    }
+    if (new_allocated > (size_t)allocated) {
+        memset(array->ob_item + allocated, 0, sizeof(PyObject *) * (new_allocated - allocated));
     }
      _Py_atomic_store_ptr_release(&self->ob_item, &array->ob_item);
     self->allocated = new_allocated;
@@ -502,7 +505,7 @@ _PyList_AppendTakeRefListResize(PyListObject *self, PyObject *newitem)
         Py_DECREF(newitem);
         return -1;
     }
-    PyList_SET_ITEM(self, len, newitem);
+    FT_ATOMIC_STORE_PTR_RELEASE(self->ob_item[len], newitem);
     return 0;
 }
 
@@ -1181,7 +1184,7 @@ list_extend_fast(PyListObject *self, PyObject *iterable)
     PyObject **dest = self->ob_item + m;
     for (Py_ssize_t i = 0; i < n; i++) {
         PyObject *o = src[i];
-        dest[i] = Py_NewRef(o);
+        FT_ATOMIC_STORE_PTR_RELEASE(dest[i], Py_NewRef(o));
     }
     return 0;
 }
@@ -1238,7 +1241,7 @@ list_extend_iter_lock_held(PyListObject *self, PyObject *iterable)
 
         if (Py_SIZE(self) < self->allocated) {
             Py_ssize_t len = Py_SIZE(self);
-            PyList_SET_ITEM(self, len, item);  // steals item ref
+            FT_ATOMIC_STORE_PTR_RELEASE(self->ob_item[len], item);  // steals item ref
             Py_SET_SIZE(self, len + 1);
         }
         else {