From dd05e6798f331bb3592a287ad26c14b465e2817b Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 18 Feb 2026 12:12:59 -0500 Subject: [PATCH 1/4] gh-144809: Make deque copy atomic in free-threaded build --- .../test_free_threading/test_collections.py | 32 +++++++++++++++++++ ...-02-18-00-00-00.gh-issue-144809.nYpEUx.rst | 1 + Modules/_collectionsmodule.c | 16 ++++++++++ 3 files changed, 49 insertions(+) create mode 100644 Lib/test/test_free_threading/test_collections.py create mode 100644 Misc/NEWS.d/next/Library/2026-02-18-00-00-00.gh-issue-144809.nYpEUx.rst diff --git a/Lib/test/test_free_threading/test_collections.py b/Lib/test/test_free_threading/test_collections.py new file mode 100644 index 00000000000000..50e62e275e0b23 --- /dev/null +++ b/Lib/test/test_free_threading/test_collections.py @@ -0,0 +1,32 @@ +import unittest +from collections import deque +from copy import copy +from test.support import threading_helper + +threading_helper.requires_working_threading(module=True) + + +class TestDeque(unittest.TestCase): + def test_copy_race(self): + # gh-144809: Test that deque copy is thread safe. It previously + # could raise a "deque mutated during iteration" error. + d = deque(range(100)) + + def mutate(): + i = 0 + for _ in range(1000): + d.append(i) + if len(d) > 200: + d.popleft() + i += 1 + + def copy_loop(): + for _ in range(1000): + copy(d) + + workers = [mutate, mutate, copy_loop, copy_loop] + threading_helper.run_concurrently(workers) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-02-18-00-00-00.gh-issue-144809.nYpEUx.rst b/Misc/NEWS.d/next/Library/2026-02-18-00-00-00.gh-issue-144809.nYpEUx.rst new file mode 100644 index 00000000000000..263bd5c2b8bef1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-02-18-00-00-00.gh-issue-144809.nYpEUx.rst @@ -0,0 +1 @@ +Make :class:`collections.deque` copy atomic in the free-threaded build. diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 45ca63e6d7c77f..dbe55b6bd13bf3 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1983,6 +1983,22 @@ dequeiter_next(PyObject *op) // It's safe to access it->deque without holding the per-object lock for it // here; it->deque is only assigned during construction of it. dequeobject *deque = it->deque; + +#ifdef Py_GIL_DISABLED + // gh-144809: When called from deque_copy(), the deque is already + // locked. The two-object critical section below would unlock and + // re-lock the deque between calls, allowing another thread to modify + // it mid-iteration. The one-object critical section avoids this + // because it keeps the deque locked across calls when it's already + // held, due to a fast-path optimization. + if (_PyObject_IsUniquelyReferenced(it)) { + Py_BEGIN_CRITICAL_SECTION(deque); + result = dequeiter_next_lock_held(it, deque); + Py_END_CRITICAL_SECTION(); + return result; + } +#endif + Py_BEGIN_CRITICAL_SECTION2(it, deque); result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); From a4655735220dcbd797a5f078f33495994b6b1984 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 18 Feb 2026 12:49:21 -0500 Subject: [PATCH 2/4] Fix cast and minor edits to test --- Lib/test/test_free_threading/test_collections.py | 3 +-- Modules/_collectionsmodule.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_free_threading/test_collections.py b/Lib/test/test_free_threading/test_collections.py index 50e62e275e0b23..4454d033f7b9cf 100644 --- a/Lib/test/test_free_threading/test_collections.py +++ b/Lib/test/test_free_threading/test_collections.py @@ -24,8 +24,7 @@ def copy_loop(): for _ in range(1000): copy(d) - workers = [mutate, mutate, copy_loop, copy_loop] - threading_helper.run_concurrently(workers) + threading_helper.run_concurrently([mutate, copy_loop]) if __name__ == "__main__": diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index dbe55b6bd13bf3..5c001715725e49 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1991,7 +1991,7 @@ dequeiter_next(PyObject *op) // it mid-iteration. The one-object critical section avoids this // because it keeps the deque locked across calls when it's already // held, due to a fast-path optimization. - if (_PyObject_IsUniquelyReferenced(it)) { + if (_PyObject_IsUniquelyReferenced((PyObject *)it)) { Py_BEGIN_CRITICAL_SECTION(deque); result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION(); From 8885f31466807f2849a00276d1f3f29e9c708d92 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 18 Feb 2026 12:50:56 -0500 Subject: [PATCH 3/4] More simplifications --- Lib/test/test_free_threading/test_collections.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/test/test_free_threading/test_collections.py b/Lib/test/test_free_threading/test_collections.py index 4454d033f7b9cf..3a413ccf396d4b 100644 --- a/Lib/test/test_free_threading/test_collections.py +++ b/Lib/test/test_free_threading/test_collections.py @@ -13,12 +13,10 @@ def test_copy_race(self): d = deque(range(100)) def mutate(): - i = 0 - for _ in range(1000): + for i in range(1000): d.append(i) if len(d) > 200: d.popleft() - i += 1 def copy_loop(): for _ in range(1000): From 2ce845d444a7b1d0dcd590bdde3be3bcd94844bd Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Fri, 20 Feb 2026 09:19:34 -0500 Subject: [PATCH 4/4] Copy deque elements directly in deque_copy --- Modules/_collectionsmodule.c | 61 +++++++++++++++++------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5c001715725e49..72865f87fc484f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -605,29 +605,42 @@ deque_copy_impl(dequeobject *deque) collections_state *state = find_module_state_by_def(Py_TYPE(deque)); if (Py_IS_TYPE(deque, state->deque_type)) { dequeobject *new_deque; - PyObject *rv; + Py_ssize_t n = Py_SIZE(deque); new_deque = (dequeobject *)deque_new(state->deque_type, NULL, NULL); if (new_deque == NULL) return NULL; new_deque->maxlen = old_deque->maxlen; - /* Fast path for the deque_repeat() common case where len(deque) == 1 - * - * It's safe to not acquire the per-object lock for new_deque; it's - * invisible to other threads. + + /* Copy elements directly by walking the block structure. + * This is safe because the caller holds the deque lock and + * the new deque is not yet visible to other threads. */ - if (Py_SIZE(deque) == 1) { - PyObject *item = old_deque->leftblock->data[old_deque->leftindex]; - rv = deque_append_impl(new_deque, item); - } else { - rv = deque_extend_impl(new_deque, (PyObject *)deque); - } - if (rv != NULL) { - Py_DECREF(rv); - return (PyObject *)new_deque; + if (n > 0) { + block *b = old_deque->leftblock; + Py_ssize_t index = old_deque->leftindex; + + /* Space saving heuristic. Start filling from the left */ + assert(new_deque->leftblock == new_deque->rightblock); + assert(new_deque->leftindex == new_deque->rightindex + 1); + new_deque->leftindex = 1; + new_deque->rightindex = 0; + + for (Py_ssize_t i = 0; i < n; i++) { + PyObject *item = b->data[index]; + if (deque_append_lock_held(new_deque, Py_NewRef(item), + new_deque->maxlen) < 0) { + Py_DECREF(new_deque); + return NULL; + } + index++; + if (index == BLOCKLEN) { + b = b->rightlink; + index = 0; + } + } } - Py_DECREF(new_deque); - return NULL; + return (PyObject *)new_deque; } if (old_deque->maxlen < 0) result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)), @@ -1983,22 +1996,6 @@ dequeiter_next(PyObject *op) // It's safe to access it->deque without holding the per-object lock for it // here; it->deque is only assigned during construction of it. dequeobject *deque = it->deque; - -#ifdef Py_GIL_DISABLED - // gh-144809: When called from deque_copy(), the deque is already - // locked. The two-object critical section below would unlock and - // re-lock the deque between calls, allowing another thread to modify - // it mid-iteration. The one-object critical section avoids this - // because it keeps the deque locked across calls when it's already - // held, due to a fast-path optimization. - if (_PyObject_IsUniquelyReferenced((PyObject *)it)) { - Py_BEGIN_CRITICAL_SECTION(deque); - result = dequeiter_next_lock_held(it, deque); - Py_END_CRITICAL_SECTION(); - return result; - } -#endif - Py_BEGIN_CRITICAL_SECTION2(it, deque); result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2();