Skip to content

Commit eaab974

Browse files
gh-159244: Fix crash in dict() on a concurrently mutated key/value pair (#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.
1 parent 94942ba commit eaab974

3 files changed

Lines changed: 33 additions & 5 deletions

File tree

‎Lib/test/test_free_threading/test_dict.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,5 +532,28 @@ def race(iter_times: int,
532532
CHURN_SIZE = 512
533533
race(ITER_TIMES, CHURN_SIZE, APPENDER_THREADS, REPLACER_THREADS)
534534

535+
def test_racing_update_from_mutating_pair(self):
536+
# gh-159244: dict() must not read the items of a key/value pair
537+
# that another thread empties concurrently.
538+
pair = [object(), object()]
539+
done = Event()
540+
541+
def writer():
542+
try:
543+
for _ in range(10000):
544+
pair.clear()
545+
pair.extend((object(), object()))
546+
finally:
547+
done.set()
548+
549+
def reader():
550+
while not done.is_set():
551+
try:
552+
dict([pair])
553+
except ValueError:
554+
pass
555+
556+
threading_helper.run_concurrently([writer] + [reader] * 4)
557+
535558
if __name__ == "__main__":
536559
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
@@ -4113,7 +4113,7 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41134113
return -1;
41144114

41154115
for (i = 0; ; ++i) {
4116-
PyObject *key, *value;
4116+
PyObject *key = NULL, *value = NULL;
41174117
Py_ssize_t n;
41184118

41194119
fast = NULL;
@@ -4135,7 +4135,13 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41354135
}
41364136
goto Fail;
41374137
}
4138+
Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(item);
41384139
n = PySequence_Fast_GET_SIZE(fast);
4140+
if (n == 2) {
4141+
key = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 0));
4142+
value = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 1));
4143+
}
4144+
Py_END_CRITICAL_SECTION_SEQUENCE_FAST();
41394145
if (n != 2) {
41404146
PyErr_Format(PyExc_ValueError,
41414147
"dictionary update sequence element #%zd "
@@ -4145,10 +4151,6 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41454151
}
41464152

41474153
/* Update/merge with this (key, value) pair. */
4148-
key = PySequence_Fast_GET_ITEM(fast, 0);
4149-
value = PySequence_Fast_GET_ITEM(fast, 1);
4150-
Py_INCREF(key);
4151-
Py_INCREF(value);
41524154
if (override) {
41534155
if (setitem_lock_held((PyDictObject *)d, key, value) < 0) {
41544156
Py_DECREF(key);

0 commit comments

Comments
 (0)