Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions Lib/test/test_free_threading/test_set.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sys
import unittest
from threading import Thread, Barrier, Event
from test.support import threading_helper
Expand Down Expand Up @@ -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()
Original file line number Diff line number Diff line change
@@ -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.
75 changes: 49 additions & 26 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading