From 1e67b9207c31a92d76bfac09fc706c4dc703669e Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Thu, 2 May 2024 13:03:05 -0700 Subject: 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 --- Include/internal/pycore_list.h | 4 ++ Lib/test/test_free_threading/test_list.py | 80 +++++++++++++++++++++++++++++++ Objects/listobject.c | 9 ++-- 3 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 Lib/test/test_free_threading/test_list.py 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 { -- cgit v0.12