diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index 9dd3d68d5dad13..7c9ffa53cfbee0 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -1,3 +1,4 @@ +import sys import unittest from threading import Thread, Barrier, Event from test.support import threading_helper @@ -159,5 +160,53 @@ class LargeSetTest(RaceTestBase, unittest.TestCase): SET_SIZE = 20 # larger than PySet_MINSIZE +class TestSetIteratorExhaustion(unittest.TestCase): + """Exhausting one set iterator from several threads at once.""" + + # One live element followed by a long run of dummy entries widens the + # window between reading si_set and clearing it, so threads that arrive + # together all reach the exhausted branch. + SET_SIZE = 300_000 + NUM_THREADS = 8 + ROUNDS = 3 + + @threading_helper.requires_working_threading() + def test_racing_exhaustion_releases_set_once(self): + s = set(range(self.SET_SIZE)) + for i in range(1, self.SET_SIZE): + s.discard(i) + # Extra references keep a wrongly released set alive, so every lost + # reference shows up as a refcount difference instead of a crash. + keep = [s] * 1000 + base = sys.getrefcount(s) + try: + for _ in range(self.ROUNDS): + it = iter(s) + next(it) # leave the iterator on the exhausted branch + barrier = Barrier(self.NUM_THREADS, timeout=10) + errors = [] + + def exhaust(): + barrier.wait() + try: + next(it) + except StopIteration: + pass + except BaseException as exc: + errors.append(exc) + + threads = [Thread(target=exhaust) + for _ in range(self.NUM_THREADS)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + self.assertEqual(errors, []) + self.assertEqual(sys.getrefcount(s), base) + finally: + del keep + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-24-19-40-00.gh-issue-160573.sT9kQe.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-24-19-40-00.gh-issue-160573.sT9kQe.rst new file mode 100644 index 00000000000000..0c4f49ca1ba5bb --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-24-19-40-00.gh-issue-160573.sT9kQe.rst @@ -0,0 +1,4 @@ +Fix a crash when a :class:`set` iterator that is shared between threads reaches +its end in a free-threaded build: every thread that walked off the end of the +table released the iterator's reference to the set, so the set was over-released. +The reference is now dropped once, under the iterator's critical section. diff --git a/Objects/setobject.c b/Objects/setobject.c index a762edf5933e18..0ae20f83f112a0 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1093,42 +1093,65 @@ static PyObject *setiter_iternext(PyObject *self) { setiterobject *si = (setiterobject*)self; PyObject *key = NULL; - Py_ssize_t i, mask; + Py_ssize_t i = 0, mask; setentry *entry; - PySetObject *so = si->si_set; + PySetObject *so; + int released = 0; + int size_changed = 0; - if (so == NULL) + /* The fast path still reads si_set without the lock, so it must be the + atomic load; a plain read races with the clearing thread's store. */ + if (FT_ATOMIC_LOAD_PTR_RELAXED(si->si_set) == NULL) return NULL; - assert (PyAnySet_Check(so)); - Py_ssize_t so_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); - Py_ssize_t si_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(si->si_used); - if (si_used != so_used) { + /* si_set is cleared by whichever thread walks off the end of the table + first, so both the read and the clear need the iterator's own lock: an + unlocked read can pick up a pointer to a set that the release has + already deallocated. */ + Py_BEGIN_CRITICAL_SECTION(self); + so = si->si_set; + if (so != NULL) { + assert (PyAnySet_Check(so)); + + Py_ssize_t so_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); + Py_ssize_t si_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(si->si_used); + if (si_used != so_used) { + si->si_used = -1; /* Make this state sticky */ + size_changed = 1; + } + else { + Py_BEGIN_CRITICAL_SECTION(so); + i = si->si_pos; + assert(i>=0); + entry = so->table; + mask = so->mask; + while (i <= mask && (entry[i].key == NULL || entry[i].key == dummy)) { + i++; + } + if (i <= mask) { + key = Py_NewRef(entry[i].key); + } + Py_END_CRITICAL_SECTION(); + si->si_pos = i+1; + if (key == NULL) { + FT_ATOMIC_STORE_PTR_RELAXED(si->si_set, NULL); + released = 1; + } + else { + si->len--; + } + } + } + Py_END_CRITICAL_SECTION(); + + if (size_changed) { PyErr_SetString(PyExc_RuntimeError, "Set changed size during iteration"); - si->si_used = -1; /* Make this state sticky */ return NULL; } - - Py_BEGIN_CRITICAL_SECTION(so); - i = si->si_pos; - assert(i>=0); - entry = so->table; - mask = so->mask; - while (i <= mask && (entry[i].key == NULL || entry[i].key == dummy)) { - i++; - } - if (i <= mask) { - key = Py_NewRef(entry[i].key); - } - Py_END_CRITICAL_SECTION(); - si->si_pos = i+1; - if (key == NULL) { - si->si_set = NULL; + if (released) { Py_DECREF(so); - return NULL; } - si->len--; return key; }