diff --git a/Lib/test/libregrtest/tsan.py b/Lib/test/libregrtest/tsan.py index 00d5779d950e72..5f83cd03067274 100644 --- a/Lib/test/libregrtest/tsan.py +++ b/Lib/test/libregrtest/tsan.py @@ -26,6 +26,7 @@ 'test_threadsignals', 'test_weakref', 'test_free_threading.test_slots', + 'test_free_threading.test_iteration', ] diff --git a/Lib/test/test_free_threading/test_iteration.py b/Lib/test/test_free_threading/test_iteration.py new file mode 100644 index 00000000000000..b2c4a898b25475 --- /dev/null +++ b/Lib/test/test_free_threading/test_iteration.py @@ -0,0 +1,130 @@ +import sys +import threading +import unittest +from test import support + +# The race conditions these tests were written for only happen every now and +# then, even with the current numbers. To find rare race conditions, bumping +# these up will help, but it makes the test runtime highly variable under +# free-threading. Overhead is much higher under ThreadSanitizer, but it's +# also much better at detecting certain races, so we don't need as many +# items/threads. +if support.check_sanitizer(thread=True): + NUMITEMS = 1000 + NUMTHREADS = 2 +else: + NUMITEMS = 100000 + NUMTHREADS = 3 +NUMMUTATORS = 2 + +class ContendedTupleIterationTest(unittest.TestCase): + def make_testdata(self, n): + return tuple(range(n)) + + def assert_iterator_results(self, results, expected): + # Most iterators are not atomic (yet?) so they can skip or duplicate + # items, but they should not invent new items (like the range + # iterator currently does). + extra_items = set(results) - set(expected) + self.assertEqual(set(), extra_items) + + def run_threads(self, func, *args, numthreads=NUMTHREADS): + threads = [] + for _ in range(numthreads): + t = threading.Thread(target=func, args=args) + t.start() + threads.append(t) + return threads + + def test_iteration(self): + """Test iteration over a shared container""" + seq = self.make_testdata(NUMITEMS) + results = [] + start = threading.Barrier(NUMTHREADS) + def worker(): + idx = 0 + start.wait() + for item in seq: + idx += 1 + results.append(idx) + threads = self.run_threads(worker) + for t in threads: + t.join() + # Each thread has its own iterator, so results should be entirely predictable. + self.assertEqual(results, [NUMITEMS] * NUMTHREADS) + + def test_shared_iterator(self): + """Test iteration over a shared iterator""" + seq = self.make_testdata(NUMITEMS) + it = iter(seq) + results = [] + start = threading.Barrier(NUMTHREADS) + def worker(): + items = [] + start.wait() + # We want a tight loop, so put items in the shared list at the end. + for item in it: + items.append(item) + results.extend(items) + threads = self.run_threads(worker) + for t in threads: + t.join() + self.assert_iterator_results(results, seq) + +class ContendedListIterationTest(ContendedTupleIterationTest): + def make_testdata(self, n): + return list(range(n)) + + def test_iteration_while_mutating(self): + """Test iteration over a shared mutating container.""" + seq = self.make_testdata(NUMITEMS) + results = [] + start = threading.Barrier(NUMTHREADS + NUMMUTATORS) + endmutate = threading.Event() + def mutator(): + orig = seq[:] + # Make changes big enough to cause resizing of the list, with + # items shifted around for good measure. + replacement = (orig * 3)[NUMITEMS//2:] + start.wait() + while not endmutate.is_set(): + seq[:] = replacement + seq[:] = orig + def worker(): + items = [] + start.wait() + # We want a tight loop, so put items in the shared list at the end. + for item in seq: + items.append(item) + results.extend(items) + mutators = () + try: + threads = self.run_threads(worker) + mutators = self.run_threads(mutator, numthreads=NUMMUTATORS) + for t in threads: + t.join() + finally: + endmutate.set() + for m in mutators: + m.join() + self.assert_iterator_results(results, list(seq)) + + +class ContendedRangeIterationTest(ContendedTupleIterationTest): + def make_testdata(self, n): + return range(n) + + def assert_iterator_results(self, results, expected): + # Range iterators that are shared between threads will (right now) + # sometimes produce items after the end of the range, sometimes + # _far_ after the end of the range. That should be fixed, but for + # now, let's just check they're integers that could have resulted + # from stepping beyond the range bounds. + extra_items = set(results) - set(expected) + for item in extra_items: + self.assertEqual((item - expected.start) % expected.step, 0) + +# Long iterators are not thread-safe yet. +# class ContendedLongRangeIterationTest(ContendedTupleIterationTest): +# def make_testdata(self, n): +# return range(0, sys.maxsize*n, sys.maxsize) diff --git a/Objects/listobject.c b/Objects/listobject.c index bbd53e7de94a31..99c0a860f283b0 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -357,7 +357,7 @@ list_get_item_ref(PyListObject *op, Py_ssize_t i) return NULL; } Py_ssize_t cap = list_capacity(ob_item); - assert(cap != -1 && cap >= size); + assert(cap != -1); if (!valid_index(i, cap)) { return NULL; } @@ -919,6 +919,8 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO if (d < 0) { /* Delete -d items */ Py_ssize_t tail; tail = (Py_SIZE(a) - ihigh) * sizeof(PyObject *); + // TODO: these memmove/memcpy calls are not safe for shared lists in + // GIL_DISABLED builds. memmove(&item[ihigh+d], &item[ihigh], tail); if (list_resize(a, Py_SIZE(a) + d) < 0) { memmove(&item[ihigh], &item[ihigh+d], tail); @@ -932,12 +934,14 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO if (list_resize(a, k+d) < 0) goto Error; item = a->ob_item; + // TODO: these memmove/memcpy calls are not safe for shared lists in + // GIL_DISABLED builds. memmove(&item[ihigh+d], &item[ihigh], (k - ihigh)*sizeof(PyObject *)); } for (k = 0; k < n; k++, ilow++) { PyObject *w = vitem[k]; - item[ilow] = Py_XNewRef(w); + FT_ATOMIC_STORE_PTR_RELEASE(item[ilow], Py_XNewRef(w)); } for (k = norig - 1; k >= 0; --k) Py_XDECREF(recycle[k]); diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 002002eb455556..a147de27e89a35 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -1020,14 +1020,17 @@ tupleiter_next(PyObject *self) return NULL; assert(PyTuple_Check(seq)); - if (it->it_index < PyTuple_GET_SIZE(seq)) { - item = PyTuple_GET_ITEM(seq, it->it_index); - ++it->it_index; + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index < PyTuple_GET_SIZE(seq)) { + item = PyTuple_GET_ITEM(seq, index); + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index + 1); return Py_NewRef(item); } +#ifndef Py_GIL_DISABLED it->it_seq = NULL; Py_DECREF(seq); +#endif return NULL; } @@ -1036,8 +1039,14 @@ tupleiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { _PyTupleIterObject *it = _PyTupleIterObject_CAST(self); Py_ssize_t len = 0; +#ifdef Py_GIL_DISABLED + Py_ssize_t idx = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (it->it_seq && idx >= 0 && idx < PyTuple_GET_SIZE(it->it_seq)) + len = PyTuple_GET_SIZE(it->it_seq) - idx; +#else if (it->it_seq) len = PyTuple_GET_SIZE(it->it_seq) - it->it_index; +#endif return PyLong_FromSsize_t(len); } @@ -1053,10 +1062,15 @@ tupleiter_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) * see issue #101765 */ _PyTupleIterObject *it = _PyTupleIterObject_CAST(self); +#ifdef Py_GIL_DISABLED + Py_ssize_t idx = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (it->it_seq && idx >= 0 && idx < PyTuple_GET_SIZE(it->it_seq)) + return Py_BuildValue("N(O)n", iter, it->it_seq, idx); +#else if (it->it_seq) return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); - else - return Py_BuildValue("N(())", iter); +#endif + return Py_BuildValue("N(())", iter); } static PyObject * @@ -1071,7 +1085,7 @@ tupleiter_setstate(PyObject *self, PyObject *state) index = 0; else if (index > PyTuple_GET_SIZE(it->it_seq)) index = PyTuple_GET_SIZE(it->it_seq); /* exhausted iterator */ - it->it_index = index; + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index); } Py_RETURN_NONE; }