gh-156310: Make the iter() sequence fallback iterator safe in free-threaded build (#156311)

Co-authored-by: Neil Schemenauer <nas@arctrix.com>

Author: akx

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)

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

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.

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;
 }