From a3ea63c15f5c5fc88f3556ce11e7ab9cb0524f56 Mon Sep 17 00:00:00 2001 From: Jason G <41053218+gianghungtien@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:20:06 +0700 Subject: [PATCH] gh-154535: Fix crash when a contextvars.Context iterator is shared between threads The HAMT iterator that backs iteration over a contextvars.Context keeps its entire depth-first cursor -- i_level, i_pos and the borrowed node pointers in i_nodes -- inside the iterator object, and advanced it with plain reads and writes. Two threads calling next() on the same iterator interleaved those updates, leaving i_level pointing at an i_nodes slot that was never filled in, so the next step dereferenced a stale or NULL node and crashed. Advance the cursor with the iterator locked, so the whole descent is atomic. The HAMT itself is immutable, so no other locking is required. As with the other free-threaded iterators, concurrent iteration may still skip or repeat items, but it no longer crashes. --- Include/internal/pycore_hamt.h | 5 +++ .../test_free_threading/test_iteration.py | 39 +++++++++++++++++++ ...-07-24-09-05-11.gh-issue-154535.Kq7vXm.rst | 6 +++ Python/hamt.c | 31 +++++++++++++-- 4 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-07-24-09-05-11.gh-issue-154535.Kq7vXm.rst 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: {