Skip to content

gh-144356: Avoid races when computing set_iterator.__length_hint__ under no-gil #144357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hyongtao-code wants to merge 5 commits into
base: main
Choose a base branch
from
+77 −1
Open

gh-144356: Avoid races when computing set_iterator.__length_hint__ under no-gil #144357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47d75fe
Fix data race in setiter_len() under no-gil
hyongtao-code Jan 31, 2026
3e3785c
Add test case
hyongtao-code Jan 31, 2026
229ced3
📜🤖 Added by blurb_it.
blurb-it[bot] Jan 31, 2026
cdcf88a
Resovle comments
hyongtao-code Feb 1, 2026
21f1478
Update test case: used_race and exhaust_race
hyongtao-code Feb 1, 2026
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
56 changes: 56 additions & 0 deletions Lib/test/test_free_threading/test_set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,62 @@ def read_set():
for t in threads:
t.join()

def test_length_hint_used_race(self):
s = set(range(2000))
it = iter(s)

NUM_LOOPS = 50_000
barrier = Barrier(2)

def reader():
barrier.wait()
for _ in range(NUM_LOOPS):
it.__length_hint__()

def writer():
barrier.wait()
i = 0
for _ in range(NUM_LOOPS):
s.add(i)
s.discard(i - 1)
i += 1

t1 = Thread(target=reader)
t2 = Thread(target=writer)
t1.start(); t2.start()
t1.join(); t2.join()

def test_length_hint_exhaust_race(self):
NUM_LOOPS = 10_000
INNER_HINTS = 20
barrier = Barrier(2)
box = {"it": None}

def exhauster():
for _ in range(NUM_LOOPS):
s = set(range(256))
box["it"] = iter(s)
barrier.wait() # start together
try:
while True:
next(box["it"])
except StopIteration:
pass
barrier.wait() # end iteration

def reader():
for _ in range(NUM_LOOPS):
barrier.wait()
it = box["it"]
for _ in range(INNER_HINTS):
it.__length_hint__()
barrier.wait()

t1 = Thread(target=reader)
t2 = Thread(target=exhauster)
t1.start(); t2.start()
t1.join(); t2.join()


@threading_helper.requires_working_threading()
class SmallSetTest(RaceTestBase, unittest.TestCase):
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-01-31-03-40-28.gh-issue-144356.otfq_X.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a data race in ``set_iterator.__length_hint__`` under ``Py_GIL_DISABLED``.
21 changes: 20 additions & 1 deletion Objects/setobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1056,8 +1056,23 @@ setiter_len(PyObject *op, PyObject *Py_UNUSED(ignored))
{
setiterobject *si = (setiterobject*)op;
Py_ssize_t len = 0;
if (si->si_set != NULL && si->si_used == si->si_set->used)
#ifdef Py_GIL_DISABLED
Copy link
Contributor

@eendebakpt eendebakpt Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might work for setiter_len, but setiter_iternext itself is not yet thread safe (also because of setting si->si_set to zero).

For several other iterations the approach is to keep the reference si->si_set , but use another attribute to signal exhaustion of the iterator. For example for itertools.cycle or the reversed operator.

Note: I tried creating a minimal example where concurrent iteration fails, but I have succeeded yet (the example does not crash, although I have not run thread sanitizer on it yet)

Test for concurrent iteration on set iterator 
import unittest
from threading import Thread, Barrier


class TestSetIter(unittest.TestCase):
    def test_set_iter(self):
        """Test concurrent iteration over a set"""

        NUM_LOOPS = 10_000
        NUM_THREADS = 4
        

        for ii in range(NUM_LOOPS):
            if ii % 1000 ==0:
                print(f'test_set_iter {ii}')
            barrier = Barrier(NUM_THREADS)
            
            # make sure the underlying set is unique referenced by the iterator
            iterator = iter(set((1,2,))) 
            
            def worker():
                barrier.wait()
                while True:
                    iterator.__length_hint__()
                    try:
                        next(iterator)
                    except StopIteration:
                        break

                
            threads = [Thread(target=worker) for _ in range(NUM_THREADS)]
            for t in threads:
                t.start()
            for t in threads:
                t.join()
                
            assert iterator.__length_hint__()==0

if __name__ == "__main__":
    unittest.main()

Copy link
Contributor Author

@hyongtao-code hyongtao-code Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I think your points make a lot of sense, and I really appreciate the two links you shared—they helped me get a more complete picture of the iterator-related data race.
I’ll try to construct the case you mentioned under a TSan environment.
If it turns out to be appropriate, we can address it fully in this PR, that would be great. Of course, this will take some time.

PyObject *so_obj = FT_ATOMIC_LOAD_PTR_ACQUIRE(si->si_set);
if (so_obj != NULL) {
/* Turn borrowed si->si_set into a strong ref safely. */
if (_Py_TryIncrefCompare((PyObject **)&si->si_set, so_obj)) {
PySetObject *so = (PySetObject *)so_obj;
if (si->si_used == FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used)) {
len = si->len;
}
Py_DECREF(so_obj);
}
}
#else
if (si->si_set != NULL && si->si_used == si->si_set->used) {
len = si->len;
}
#endif
return PyLong_FromSsize_t(len);
}

Expand Down Expand Up @@ -1124,7 +1139,11 @@ static PyObject *setiter_iternext(PyObject *self)
Py_END_CRITICAL_SECTION();
si->si_pos = i+1;
if (key == NULL) {
#ifdef Py_GIL_DISABLED
FT_ATOMIC_STORE_PTR_RELEASE(si->si_set, NULL);
#else
si->si_set = NULL;
#endif
Py_DECREF(so);
return NULL;
}
Expand Down
Loading