Skip to content

Commit 0279ee2

Browse files
gh-159244: Fix crash in dict() on a concurrently mutated key/value pair
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. Read the size and both items under Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST, as bytes.join() and str.join() already do.
1 parent 0babcdd commit 0279ee2

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:
@@ -356,5 +356,28 @@ def writer():
356356

357357
threading_helper.run_concurrently([reader, writer])
358358

359+
def test_racing_update_from_mutating_pair(self):
360+
# gh-159244: dict() must not read the items of a key/value pair
361+
# that another thread empties concurrently.
362+
pair = [object(), object()]
363+
done = Event()
364+
365+
def writer():
366+
try:
367+
for _ in range(10000):
368+
pair.clear()
369+
pair.extend((object(), object()))
370+
finally:
371+
done.set()
372+
373+
def reader():
374+
while not done.is_set():
375+
try:
376+
dict([pair])
377+
except ValueError:
378+
pass
379+
380+
threading_helper.run_concurrently([writer] + [reader] * 4)
381+
359382
if __name__ == "__main__":
360383
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
@@ -4115,7 +4115,7 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41154115
return -1;
41164116

41174117
for (i = 0; ; ++i) {
4118-
PyObject *key, *value;
4118+
PyObject *key = NULL, *value = NULL;
41194119
Py_ssize_t n;
41204120

41214121
fast = NULL;
@@ -4137,7 +4137,13 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41374137
}
41384138
goto Fail;
41394139
}
4140+
Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(item);
41404141
n = PySequence_Fast_GET_SIZE(fast);
4142+
if (n == 2) {
4143+
key = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 0));
4144+
value = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 1));
4145+
}
4146+
Py_END_CRITICAL_SECTION_SEQUENCE_FAST();
41414147
if (n != 2) {
41424148
PyErr_Format(PyExc_ValueError,
41434149
"dictionary update sequence element #%zd "
@@ -4147,10 +4153,6 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
41474153
}
41484154

41494155
/* Update/merge with this (key, value) pair. */
4150-
key = PySequence_Fast_GET_ITEM(fast, 0);
4151-
value = PySequence_Fast_GET_ITEM(fast, 1);
4152-
Py_INCREF(key);
4153-
Py_INCREF(value);
41544156
if (override) {
41554157
if (setitem_lock_held((PyDictObject *)d, key, value) < 0) {
41564158
Py_DECREF(key);

0 commit comments

Comments
 (0)