https://github.com/python/cpython/commit/b707ce61968161a444f18e45910d1a25f1181ab7 commit: b707ce61968161a444f18e45910d1a25f1181ab7 branch: main author: Aarni Koskela <[email protected]> committer: kumaraditya303 <[email protected]> date: 2026-09-06T16:00:51+05:30 summary:
gh-156310: Make the iter() sequence fallback iterator safe in free-threaded build (#156311) Co-authored-by: Neil Schemenauer <[email protected]> files: A Misc/NEWS.d/next/Core_and_Builtins/2026-08-24-12-00-00.gh-issue-156310.pSqIter.rst M Lib/test/test_free_threading/test_iteration.py M Lib/test/test_iter.py M Objects/iterobject.c diff --git a/Lib/test/test_free_threading/test_iteration.py b/Lib/test/test_free_threading/test_iteration.py index 44d3e9ccfdd14e0..540e8edcc676285 100644 --- a/Lib/test/test_free_threading/test_iteration.py +++ b/Lib/test/test_free_threading/test_iteration.py @@ -1,6 +1,8 @@ +import sys import threading import unittest from test import support +from test.support import threading_helper # 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 @@ -112,6 +114,62 @@ def worker(): self.assert_iterator_results(results, list(seq)) +class ContendedSeqIterExhaustionTest(unittest.TestCase): + """Test draining a shared iter() fallback iterator (PySeqIter_Type). + + Sequences implementing __getitem__ but not __iter__ iterate through + PySeqIter_Type. Unlike the other tests in this file, this uses a + tiny sequence and many rounds so that many threads reach the racy + exhaustion path simultaneously (see gh-156310, where this + use-after-freed the sequence). + """ + + class Seq: + def __init__(self, n): + self.n = n + + def __getitem__(self, i): + if i >= self.n: + raise IndexError(i) + return i + + @support.refcount_test + def test_shared_iterator_exhaustion(self): + nthreads = 8 + nrounds = 20 if support.check_sanitizer(thread=True) else 100 + seq = self.Seq(4) + expected = set(range(seq.n)) + refcount_before = sys.getrefcount(seq) + + def drain(it, barrier, results): + items = [] + barrier.wait() + for item in it: + items.append(item) + results.extend(items) + + for _ in range(nrounds): + it = iter(seq) + barrier = threading.Barrier(nthreads) + results = [] + threads = [ + threading.Thread(target=drain, args=(it, barrier, results)) + for _ in range(nthreads) + ] + with threading_helper.catch_threading_exception() as cm: + with threading_helper.start_threads(threads): + pass + self.assertIsNone(cm.exc_value) + del it + # Threads may see duplicate or missing items, but never + # invented ones. + self.assertEqual(set(results) - expected, set()) + + # A double-DECREF of the sequence does not always crash; it + # reliably shows up as a sagging reference count. + self.assertEqual(sys.getrefcount(seq), refcount_before) + + class ContendedRangeIterationTest(ContendedTupleIterationTest): def make_testdata(self, n): return range(n) diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py index be9d0a709f2f4a7..fe8617309da29d0 100644 --- a/Lib/test/test_iter.py +++ b/Lib/test/test_iter.py @@ -2,6 +2,7 @@ import sys import unittest +from test import support from test.support import cpython_only from test.support.os_helper import TESTFN, unlink from test.support import check_free_after_iterating, ALWAYS_EQ, NEVER_EQ @@ -249,6 +250,71 @@ def test_mutating_seq_class_exhausted_iter(self): self.assertEqual(list(empit), [5, 6]) self.assertEqual(list(a), [0, 1, 2, 3, 4, 5, 6]) + @support.refcount_test + def test_seq_class_reentrant_exhaustion(self): + # gh-156310: a re-entrant next() from inside __getitem__ (or from + # __del__ of the IndexError instance) that exhausts the iterator + # used to make the outer next() DECREF the sequence a second time. + it = None + + class ReentrantGetItem: + def __init__(self): + self.calls = 0 + + def __getitem__(self, i): + self.calls += 1 + if self.calls == 1: + for _ in it: + pass + raise IndexError(i) + + seq = ReentrantGetItem() + refcount = sys.getrefcount(seq) + it = iter(seq) + self.assertEqual(list(it), []) + del it + support.gc_collect() + self.assertEqual(sys.getrefcount(seq), refcount) + + class ReentrantIndexError(IndexError): + def __del__(self): + try: + next(it) + except StopIteration: + pass + + class RaiseReentrant: + def __getitem__(self, i): + raise ReentrantIndexError(i) + + seq = RaiseReentrant() + refcount = sys.getrefcount(seq) + it = iter(seq) + self.assertEqual(list(it), []) + del it + support.gc_collect() + self.assertEqual(sys.getrefcount(seq), refcount) + + # An outer __getitem__ that succeeds after a re-entrant next() + # exhausted the iterator must not revive it. + class ReviveGetItem: + def __init__(self): + self.calls = 0 + + def __getitem__(self, i): + self.calls += 1 + if self.calls == 1: + for _ in it: + pass + if i >= 3: + raise IndexError(i) + return i + + it = iter(ReviveGetItem()) + self.assertEqual(next(it), 0) + self.assertEqual(list(it), []) + self.assertEqual(it.__length_hint__(), 0) + def test_reduce_mutating_builtins_iter(self): # This is a reproducer of issue #101765 # where iter `__reduce__` calls could lead to a segfault or SystemError diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-24-12-00-00.gh-issue-156310.pSqIter.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-24-12-00-00.gh-issue-156310.pSqIter.rst new file mode 100644 index 000000000000000..440929d62333a6f --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-24-12-00-00.gh-issue-156310.pSqIter.rst @@ -0,0 +1,7 @@ +Fix memory safety issues in the :func:`iter` fallback for objects that +implement :meth:`~object.__getitem__` without :meth:`~object.__iter__` +(``PySeqIter_Type``). Sharing an iterator between threads in the +free-threaded build could use the underlying sequence after it was freed, +and re-entrant exhaustion in the default build could decrement the sequence's +reference count twice. Concurrent iteration may still see duplicate or +missing items, but it no longer corrupts the interpreter state. diff --git a/Objects/iterobject.c b/Objects/iterobject.c index 0394227cd482dbc..b5783c92c8eb689 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -7,14 +7,16 @@ #include "pycore_genobject.h" // _PyCoro_GetAwaitableIter() #include "pycore_iterobject.h" // _PyCallIter_NewEx() #include "pycore_object.h" // _PyObject_GC_TRACK() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SSIZE_RELAXED() #include "pycore_pyerrors.h" // _PyErr_FormatFromCause() #include "pycore_pystate.h" // _PyThreadState_GET() typedef struct { PyObject_HEAD - Py_ssize_t it_index; - PyObject *it_seq; /* Set to NULL when iterator is exhausted */ + Py_ssize_t it_index; /* -1 when iterator is exhausted */ + PyObject *it_seq; /* Set to NULL when iterator is exhausted + (in the default build) */ } seqiterobject; PyObject * @@ -61,26 +63,41 @@ iter_iternext(PyObject *iterator) assert(PySeqIter_Check(iterator)); it = (seqiterobject *)iterator; + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index < 0) + return NULL; seq = it->it_seq; +#ifndef Py_GIL_DISABLED if (seq == NULL) return NULL; - if (it->it_index == PY_SSIZE_T_MAX) { +#endif + if (index == PY_SSIZE_T_MAX) { PyErr_SetString(PyExc_OverflowError, "iter index too large"); return NULL; } - result = PySequence_GetItem(seq, it->it_index); + result = PySequence_GetItem(seq, index); if (result != NULL) { - it->it_index++; + /* PySequence_GetItem() can exhaust the iterator re-entrantly. + * Preserve the exhaustion sentinel if it is observed. Concurrent + * exhaustion can still race with the store, but remains memory-safe + * because the sequence stays alive. */ + if (FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index) >= 0) { + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index + 1); + } return result; } if (PyErr_ExceptionMatches(PyExc_IndexError) || PyErr_ExceptionMatches(PyExc_StopIteration)) { + /* Mark the iterator exhausted before anything that can run + * arbitrary code. */ + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, -1); +#ifndef Py_GIL_DISABLED + Py_CLEAR(it->it_seq); +#endif PyErr_Clear(); - it->it_seq = NULL; - Py_DECREF(seq); } return NULL; } @@ -91,7 +108,8 @@ iter_len(PyObject *op, PyObject *Py_UNUSED(ignored)) seqiterobject *it = (seqiterobject*)op; Py_ssize_t seqsize, len; - if (it->it_seq) { + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index >= 0 && it->it_seq != NULL) { if (_PyObject_HasLen(it->it_seq)) { seqsize = PySequence_Size(it->it_seq); if (seqsize == -1) @@ -100,7 +118,7 @@ iter_len(PyObject *op, PyObject *Py_UNUSED(ignored)) else { Py_RETURN_NOTIMPLEMENTED; } - len = seqsize - it->it_index; + len = seqsize - index; if (len >= 0) return PyLong_FromSsize_t(len); } @@ -119,8 +137,9 @@ iter_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); + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index); + if (index >= 0 && it->it_seq != NULL) + return Py_BuildValue("N(O)n", iter, it->it_seq, index); else return Py_BuildValue("N(())", iter); } @@ -134,10 +153,10 @@ iter_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; - it->it_index = index; + if (index < 0) + index = 0; + if (it->it_seq && FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index) >= 0) { + FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index); } Py_RETURN_NONE; } _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
