summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDino Viehland <dinoviehland@meta.com>2024-05-02 20:03:05 (GMT)
committerGitHub <noreply@github.com>2024-05-02 20:03:05 (GMT)
commit1e67b9207c31a92d76bfac09fc706c4dc703669e (patch)
tree5f6f4bee04d4168daf715f70ef307518ad956594
parent8ed546679524140d8282175411fd141fe7df070d (diff)
downloadcpython-1e67b9207c31a92d76bfac09fc706c4dc703669e.zip
cpython-1e67b9207c31a92d76bfac09fc706c4dc703669e.tar.gz
cpython-1e67b9207c31a92d76bfac09fc706c4dc703669e.tar.bz2
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
-rw-r--r--Include/internal/pycore_list.h4
-rw-r--r--Lib/test/test_free_threading/test_list.py80
-rw-r--r--Objects/listobject.c9
3 files changed, 90 insertions, 3 deletions
diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h
index 2a82912..73695d1 100644
--- 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
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 4eaf200..3c4e2d2 100644
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -142,6 +142,9 @@ list_resize(PyListObject *self, Py_ssize_t newsize)
}
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;
Py_SET_SIZE(self, newsize);
@@ -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 {