diff --git a/Include/internal/pycore_hamt.h b/Include/internal/pycore_hamt.h index f973ce6dcb20e08..fe72ef718ebed91 100644 --- a/Include/internal/pycore_hamt.h +++ b/Include/internal/pycore_hamt.h @@ -46,6 +46,11 @@ extern PyTypeObject _PyHamtItems_Type; - i_nodes: an array of seven pointers to tree nodes - i_level: the current node in i_nodes - i_pos: an array of positions within nodes in i_nodes. + + Because i_nodes holds borrowed references, advancing the state is only + safe while nothing else can observe it half-updated. When the state is + embedded in a PyHamtIterator (as opposed to living on the stack), it must + be advanced with the iterator locked -- see hamt_baseiter_tp_iternext(). */ typedef struct { PyHamtNode *i_nodes[_Py_HAMT_MAX_TREE_DEPTH]; diff --git a/Lib/test/test_free_threading/test_iteration.py b/Lib/test/test_free_threading/test_iteration.py index 44d3e9ccfdd14e0..3589eeb3f91a36d 100644 --- a/Lib/test/test_free_threading/test_iteration.py +++ b/Lib/test/test_free_threading/test_iteration.py @@ -1,3 +1,4 @@ +import contextvars import threading import unittest from test import support @@ -112,6 +113,44 @@ def worker(): self.assert_iterator_results(results, list(seq)) +class ContendedContextIterationTest(ContendedTupleIterationTest): + # A contextvars.Context is iterated by the HAMT iterator, which keeps its + # whole depth-first cursor -- including borrowed node pointers -- inside + # the iterator object, so sharing one between threads used to crash the + # interpreter (gh-154535). + def make_testdata(self, n): + variables = [contextvars.ContextVar(f'v{i}') for i in range(n)] + ctx = contextvars.Context() + def populate(): + for i, var in enumerate(variables): + var.set(i) + ctx.run(populate) + return ctx + + def test_restarted_shared_iterator(self): + """Test a shared iterator that is restarted when it is exhausted""" + # A single pass over a shared iterator is over too quickly to reliably + # desync its cursor. Keep replacing the iterator once it is exhausted + # so that the threads stay on top of each other for a while. + seq = self.make_testdata(16) + cell = [iter(seq)] + results = [] + start = threading.Barrier(NUMTHREADS) + def worker(): + items = [] + start.wait() + for _ in range(NUMITEMS): + try: + items.append(next(cell[0])) + except StopIteration: + cell[0] = iter(seq) + results.extend(items) + threads = self.run_threads(worker) + for t in threads: + t.join() + self.assert_iterator_results(results, seq) + + class ContendedRangeIterationTest(ContendedTupleIterationTest): def make_testdata(self, n): return range(n) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-24-09-05-11.gh-issue-154535.Kq7vXm.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-24-09-05-11.gh-issue-154535.Kq7vXm.rst new file mode 100644 index 000000000000000..53a48d52e791204 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-24-09-05-11.gh-issue-154535.Kq7vXm.rst @@ -0,0 +1,6 @@ +Fix a crash on the :term:`free-threaded ` build when a single +iterator over a :class:`contextvars.Context` (as returned by :func:`iter`, +:meth:`~contextvars.Context.keys`, :meth:`~contextvars.Context.values` or +:meth:`~contextvars.Context.items`) is advanced from several threads at once. +The underlying HAMT iterator now advances its traversal cursor while holding +the iterator lock. diff --git a/Python/hamt.c b/Python/hamt.c index 95998ae5062ac7e..33b1e2fe500928a 100644 --- a/Python/hamt.c +++ b/Python/hamt.c @@ -1,5 +1,6 @@ #include "Python.h" #include "pycore_bitutils.h" // _Py_popcount32() +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_hamt.h" #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_long.h" // _PyLong_Format() @@ -2474,9 +2475,28 @@ static PyObject * hamt_baseiter_tp_iternext(PyObject *op) { PyHamtIterator *it = (PyHamtIterator*)op; - PyObject *key; - PyObject *val; - hamt_iter_t res = hamt_iterator_next(&it->hi_iter, &key, &val); + PyObject *key = NULL; + PyObject *val = NULL; + hamt_iter_t res; + + /* The depth-first cursor (i_level, i_pos and i_nodes) is stored in the + iterator and is updated by every step of the descent, so two threads + advancing the same iterator would interleave their updates and leave + i_level pointing at an i_nodes slot that was never filled in. Since + i_nodes holds borrowed pointers, that desync makes the next step + dereference a stale or NULL node. Serialize the whole descent on the + iterator; the HAMT itself is immutable, so nothing else needs locking. + + Concurrent iteration can still skip or repeat items, which matches the + behaviour of the other free-threaded iterators. */ + Py_BEGIN_CRITICAL_SECTION(op); + res = hamt_iterator_next(&it->hi_iter, &key, &val); + if (res == I_ITEM) { + /* hamt_iterator_next() returns borrowed references. */ + Py_INCREF(key); + Py_INCREF(val); + } + Py_END_CRITICAL_SECTION(); switch (res) { case I_END: @@ -2484,7 +2504,10 @@ hamt_baseiter_tp_iternext(PyObject *op) return NULL; case I_ITEM: { - return (*(it->hi_yield))(key, val); + PyObject *item = (*(it->hi_yield))(key, val); + Py_DECREF(key); + Py_DECREF(val); + return item; } default: {