Skip to content

Commit 224f1f1

Browse files
gh-159244: Fix crash in dict() on a concurrently mutated key/value pair (GH-159245)
In the free-threaded build, dict() and dict.update() checked that a list key/value pair had two items and then read them with borrowed references without holding its lock, so another thread could empty the list in between and the items were read from a freed or NULL array. (cherry picked from commit eaab974) Co-authored-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
1 parent aa3903e commit 224f1f1

3 files changed

Lines changed: 34 additions & 6 deletions

File tree

‎Lib/test/test_free_threading/test_dict.py‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from ast import Or
77
from functools import partial
8-
from threading import Barrier, Thread
8+
from threading import Barrier, Event, Thread
99
from unittest import TestCase
1010

1111
try:
@@ -290,5 +290,28 @@ def reader():
290290
with threading_helper.start_threads([t1, t2]):
291291
pass
292292

293+
def test_racing_update_from_mutating_pair(self):
294+
# gh-159244: dict() must not read the items of a key/value pair
295+
# that another thread empties concurrently.
296+
pair = [object(), object()]
297+
done = Event()
298+
299+
def writer():
300+
try:
301+
for _ in range(10000):
302+
pair.clear()
303+
pair.extend((object(), object()))
304+
finally:
305+
done.set()
306+
307+
def reader():
308+
while not done.is_set():
309+
try:
310+
dict([pair])
311+
except ValueError:
312+
pass
313+
314+
threading_helper.run_concurrently([writer] + [reader] * 4)
315+
293316
if __name__ == "__main__":
294317
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :class:`dict` and :meth:`dict.update` in the
2+
:term:`free-threaded build` when another thread concurrently mutates a list
3+
used as a key/value pair. Patch by Christian Aurich Zanettini Martins.

‎Objects/dictobject.c‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3808,7 +3808,7 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
38083808
return -1;
38093809

38103810
for (i = 0; ; ++i) {
3811-
PyObject *key, *value;
3811+
PyObject *key = NULL, *value = NULL;
38123812
Py_ssize_t n;
38133813

38143814
fast = NULL;
@@ -3830,7 +3830,13 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
38303830
}
38313831
goto Fail;
38323832
}
3833+
Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(item);
38333834
n = PySequence_Fast_GET_SIZE(fast);
3835+
if (n == 2) {
3836+
key = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 0));
3837+
value = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 1));
3838+
}
3839+
Py_END_CRITICAL_SECTION_SEQUENCE_FAST();
38343840
if (n != 2) {
38353841
PyErr_Format(PyExc_ValueError,
38363842
"dictionary update sequence element #%zd "
@@ -3840,10 +3846,6 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
38403846
}
38413847

38423848
/* Update/merge with this (key, value) pair. */
3843-
key = PySequence_Fast_GET_ITEM(fast, 0);
3844-
value = PySequence_Fast_GET_ITEM(fast, 1);
3845-
Py_INCREF(key);
3846-
Py_INCREF(value);
38473849
if (override) {
38483850
if (setitem_lock_held((PyDictObject *)d, key, value) < 0) {
38493851
Py_DECREF(key);

0 commit comments

Comments
 (0)