diff --git a/Lib/test/test_free_threading/test_iteration.py b/Lib/test/test_free_threading/test_iteration.py index 44d3e9ccfdd14e..f2df0b4acb9f8f 100644 --- a/Lib/test/test_free_threading/test_iteration.py +++ b/Lib/test/test_free_threading/test_iteration.py @@ -125,3 +125,46 @@ def assert_iterator_results(self, results, expected): extra_items = set(results) - set(expected) for item in extra_items: self.assertEqual((item - expected.start) % expected.step, 0) + +class ContendedStringIterationTest(ContendedTupleIterationTest): + def make_testdata(self, n): + return "A" * n + + def test_shared_iterator(self): + """Test iteration over a shared string iterator""" + seq = self.make_testdata(NUMITEMS) + it = iter(seq) + results = [] + start = threading.Barrier(NUMTHREADS) + + def worker(): + items = [] + start.wait() + 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) + + def test_shared_iterator_exhaustion(self): + """Test concurrent exhaustion of a shared string iterator""" + for _ in range(100): + seq = self.make_testdata(NUMITEMS) + it = iter(seq) + start = threading.Barrier(NUMTHREADS) + + def worker(): + start.wait() + while True: + try: + next(it) + except StopIteration: + return + + threads = self.run_threads(worker) + for t in threads: + t.join() diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py index 18e4b676c53236..645036f0fd4376 100644 --- a/Lib/test/test_iter.py +++ b/Lib/test/test_iter.py @@ -1137,6 +1137,34 @@ def test_iter_neg_setstate(self): self.assertEqual(next(it), 0) self.assertEqual(next(it), 1) + @cpython_only + def test_exhausted_str_iter_setstate_does_not_revive(self): + # gh-153928: a str iterator exhausted by normal iteration must stay + # exhausted, even if __setstate__() is called afterwards (as pickle + # does when restoring one). In free-threaded builds, the iterator's + # reference to the string is kept until the iterator itself is + # deallocated rather than dropped on exhaustion (to avoid racing + # with other threads also exhausting it -- see unicodeiter_next()), + # so __setstate__() can no longer rely on it_seq being NULL to + # detect "already permanently exhausted". See also gh-129139, the + # same bug for list/tuple iterators. + s = "hello" + it = iter(s) + self.assertEqual(list(it), list(s)) + with self.assertRaises(StopIteration): + next(it) + it.__setstate__(2) + with self.assertRaises(StopIteration): + next(it) + + # same invariant via a real pickle round-trip + it2 = iter(s) + for _ in s: + next(it2) + it3 = pickle.loads(pickle.dumps(it2)) + with self.assertRaises(StopIteration): + next(it3) + def test_free_after_iterating(self): check_free_after_iterating(self, iter, SequenceClass, (0,)) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-15-20-47-35.gh-issue-153928.FUi0YU.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-15-20-47-35.gh-issue-153928.FUi0YU.rst new file mode 100644 index 00000000000000..481288fec31d5c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-15-20-47-35.gh-issue-153928.FUi0YU.rst @@ -0,0 +1 @@ +Fix data races in Unicode iterators in free-threaded builds. diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 2249280d0af811..81cad7ff6e16d5 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -55,6 +55,7 @@ OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include "pycore_pyhash.h" // _Py_HashSecret_t #include "pycore_pylifecycle.h" // _Py_SetFileSystemEncoding() #include "pycore_pystate.h" // _PyInterpreterState_GET() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_*() #include "pycore_ucnhash.h" // _PyUnicode_Name_CAPI #include "pycore_unicodectype.h" // _PyUnicode_IsXidStart #include "pycore_unicodeobject.h" // struct _Py_unicode_state @@ -14965,24 +14966,32 @@ static PyObject * unicodeiter_next(PyObject *op) { unicodeiterobject *it = (unicodeiterobject *)op; - PyObject *seq; assert(it != NULL); - seq = it->it_seq; + /* it_seq is set once at construction and, in free-threaded builds, + * released only in unicodeiter_dealloc() -- never here -- so reading + * it needs no atomic/critical-section protection: a thread calling + * next() always holds its own reference to `it`, which is exactly + * what keeps dealloc() from running concurrently. This mirrors + * listiter_next()/tupleiter_next(). */ + PyObject *seq = it->it_seq; if (seq == NULL) return NULL; assert(_PyUnicode_CHECK(seq)); - if (it->it_index < PyUnicode_GET_LENGTH(seq)) { + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index < PyUnicode_GET_LENGTH(seq)) { int kind = PyUnicode_KIND(seq); const void *data = PyUnicode_DATA(seq); - Py_UCS4 chr = PyUnicode_READ(kind, data, it->it_index); - it->it_index++; + Py_UCS4 chr = PyUnicode_READ(kind, data, index); + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index + 1); return unicode_char(chr); } +#ifndef Py_GIL_DISABLED it->it_seq = NULL; Py_DECREF(seq); +#endif return NULL; } @@ -14991,21 +15000,26 @@ unicode_ascii_iter_next(PyObject *op) { unicodeiterobject *it = (unicodeiterobject *)op; assert(it != NULL); - PyObject *seq = it->it_seq; + PyObject *seq = it->it_seq; /* see unicodeiter_next() for why this is safe unsynchronized */ if (seq == NULL) { return NULL; } assert(_PyUnicode_CHECK(seq)); assert(PyUnicode_IS_COMPACT_ASCII(seq)); - if (it->it_index < PyUnicode_GET_LENGTH(seq)) { + + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index < PyUnicode_GET_LENGTH(seq)) { const void *data = ((void*)(_PyASCIIObject_CAST(seq) + 1)); - Py_UCS1 chr = (Py_UCS1)PyUnicode_READ(PyUnicode_1BYTE_KIND, - data, it->it_index); - it->it_index++; + Py_UCS1 chr = (Py_UCS1)PyUnicode_READ( + PyUnicode_1BYTE_KIND, data, index); + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index + 1); return (PyObject*)&_Py_SINGLETON(strings).ascii[chr]; } + +#ifndef Py_GIL_DISABLED it->it_seq = NULL; Py_DECREF(seq); +#endif return NULL; } @@ -15014,8 +15028,14 @@ unicodeiter_len(PyObject *op, PyObject *Py_UNUSED(ignored)) { unicodeiterobject *it = (unicodeiterobject *)op; Py_ssize_t len = 0; - if (it->it_seq) - len = PyUnicode_GET_LENGTH(it->it_seq) - it->it_index; + PyObject *seq = it->it_seq; + if (seq != NULL) { + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + Py_ssize_t seq_len = PyUnicode_GET_LENGTH(seq); + if (index < seq_len) { + len = seq_len - index; + } + } return PyLong_FromSsize_t(len); } @@ -15031,16 +15051,20 @@ unicodeiter_reduce(PyObject *op, PyObject *Py_UNUSED(ignored)) * call must be before access of iterator pointers. * see issue #101765 */ - if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); - } else { - PyObject *u = _PyUnicode_GetEmpty(); - if (u == NULL) { - Py_XDECREF(iter); - return NULL; + PyObject *seq = it->it_seq; + if (seq != NULL) { + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index < PyUnicode_GET_LENGTH(seq)) { + return Py_BuildValue("N(O)n", iter, seq, index); } - return Py_BuildValue("N(N)", iter, u); } + + PyObject *u = _PyUnicode_GetEmpty(); + if (u == NULL) { + Py_XDECREF(iter); + return NULL; + } + return Py_BuildValue("N(N)", iter, u); } PyDoc_STRVAR(reduce_doc, "Return state information for pickling."); @@ -15052,12 +15076,25 @@ unicodeiter_setstate(PyObject *op, PyObject *state) Py_ssize_t index = PyLong_AsSsize_t(state); if (index == -1 && PyErr_Occurred()) return NULL; - if (it->it_seq != NULL) { - if (index < 0) - index = 0; - else if (index > PyUnicode_GET_LENGTH(it->it_seq)) - index = PyUnicode_GET_LENGTH(it->it_seq); /* iterator truncated */ - it->it_index = index; + PyObject *seq = it->it_seq; + if (seq != NULL) { + /* A permanently exhausted iterator must stay exhausted: in + * free-threaded builds it_seq is never cleared on exhaustion + * (see unicodeiter_next()), so unlike the GIL build we can't + * use "it_seq == NULL" to detect that. Use the index instead: + * once it has reached the sequence length, refuse to move it + * backwards. Without this check, __setstate__() could "revive" + * an exhausted iterator -- the same bug reported against list + * and tuple iterators in gh-129139. */ + Py_ssize_t cur_index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + Py_ssize_t len = PyUnicode_GET_LENGTH(seq); + if (cur_index < len) { + if (index < 0) + index = 0; + else if (index > len) + index = len; /* iterator truncated */ + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index); + } } Py_RETURN_NONE; }