Skip to content

Commit 70da972

Browse files
authored
gh-144809: Make deque copy atomic in free-threaded build (gh-144966)
1 parent 75c5753 commit 70da972

File tree

3 files changed

+59
-16
lines changed

3 files changed

+59
-16
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import unittest
2+
from collections import deque
3+
from copy import copy
4+
from test.support import threading_helper
5+
6+
threading_helper.requires_working_threading(module=True)
7+
8+
9+
class TestDeque(unittest.TestCase):
10+
def test_copy_race(self):
11+
# gh-144809: Test that deque copy is thread safe. It previously
12+
# could raise a "deque mutated during iteration" error.
13+
d = deque(range(100))
14+
15+
def mutate():
16+
for i in range(1000):
17+
d.append(i)
18+
if len(d) > 200:
19+
d.popleft()
20+
21+
def copy_loop():
22+
for _ in range(1000):
23+
copy(d)
24+
25+
threading_helper.run_concurrently([mutate, copy_loop])
26+
27+
28+
if __name__ == "__main__":
29+
unittest.main()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make :class:`collections.deque` copy atomic in the :term:`free-threaded build`.

Modules/_collectionsmodule.c

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -605,29 +605,42 @@ deque_copy_impl(dequeobject *deque)
605605
collections_state *state = find_module_state_by_def(Py_TYPE(deque));
606606
if (Py_IS_TYPE(deque, state->deque_type)) {
607607
dequeobject *new_deque;
608-
PyObject *rv;
608+
Py_ssize_t n = Py_SIZE(deque);
609609

610610
new_deque = (dequeobject *)deque_new(state->deque_type, NULL, NULL);
611611
if (new_deque == NULL)
612612
return NULL;
613613
new_deque->maxlen = old_deque->maxlen;
614-
/* Fast path for the deque_repeat() common case where len(deque) == 1
615-
*
616-
* It's safe to not acquire the per-object lock for new_deque; it's
617-
* invisible to other threads.
614+
615+
/* Copy elements directly by walking the block structure.
616+
* This is safe because the caller holds the deque lock and
617+
* the new deque is not yet visible to other threads.
618618
*/
619-
if (Py_SIZE(deque) == 1) {
620-
PyObject *item = old_deque->leftblock->data[old_deque->leftindex];
621-
rv = deque_append_impl(new_deque, item);
622-
} else {
623-
rv = deque_extend_impl(new_deque, (PyObject *)deque);
624-
}
625-
if (rv != NULL) {
626-
Py_DECREF(rv);
627-
return (PyObject *)new_deque;
619+
if (n > 0) {
620+
block *b = old_deque->leftblock;
621+
Py_ssize_t index = old_deque->leftindex;
622+
623+
/* Space saving heuristic. Start filling from the left */
624+
assert(new_deque->leftblock == new_deque->rightblock);
625+
assert(new_deque->leftindex == new_deque->rightindex + 1);
626+
new_deque->leftindex = 1;
627+
new_deque->rightindex = 0;
628+
629+
for (Py_ssize_t i = 0; i < n; i++) {
630+
PyObject *item = b->data[index];
631+
if (deque_append_lock_held(new_deque, Py_NewRef(item),
632+
new_deque->maxlen) < 0) {
633+
Py_DECREF(new_deque);
634+
return NULL;
635+
}
636+
index++;
637+
if (index == BLOCKLEN) {
638+
b = b->rightlink;
639+
index = 0;
640+
}
641+
}
628642
}
629-
Py_DECREF(new_deque);
630-
return NULL;
643+
return (PyObject *)new_deque;
631644
}
632645
if (old_deque->maxlen < 0)
633646
result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)),

0 commit comments

Comments
 (0)