Skip to content

Commit 039f605

Browse files
christianaurichzmmethane
authored andcommitted
[3.15] 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 15799d3 commit 039f605

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

438438
threading_helper.run_concurrently([reader, writer])
439439

440+
def test_racing_update_from_mutating_pair(self):
441+
# gh-159244: dict() must not read the items of a key/value pair
442+
# that another thread empties concurrently.
443+
pair = [object(), object()]
444+
done = Event()
445+
446+
def writer():
447+
try:
448+
for _ in range(10000):
449+
pair.clear()
450+
pair.extend((object(), object()))
451+
finally:
452+
done.set()
453+
454+
def reader():
455+
while not done.is_set():
456+
try:
457+
dict([pair])
458+
except ValueError:
459+
pass
460+
461+
threading_helper.run_concurrently([writer] + [reader] * 4)
462+
440463
if __name__ == "__main__":
441464
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
@@ -4066,7 +4066,7 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
40664066
return -1;
40674067

40684068
for (i = 0; ; ++i) {
4069-
PyObject *key, *value;
4069+
PyObject *key = NULL, *value = NULL;
40704070
Py_ssize_t n;
40714071

40724072
fast = NULL;
@@ -4088,7 +4088,13 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
40884088
}
40894089
goto Fail;
40904090
}
4091+
Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(item);
40914092
n = PySequence_Fast_GET_SIZE(fast);
4093+
if (n == 2) {
4094+
key = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 0));
4095+
value = Py_NewRef(PySequence_Fast_GET_ITEM(fast, 1));
4096+
}
4097+
Py_END_CRITICAL_SECTION_SEQUENCE_FAST();
40924098
if (n != 2) {
40934099
PyErr_Format(PyExc_ValueError,
40944100
"dictionary update sequence element #%zd "
@@ -4098,10 +4104,6 @@ merge_from_seq2_lock_held(PyObject *d, PyObject *seq2, int override)
40984104
}
40994105

41004106
/* Update/merge with this (key, value) pair. */
4101-
key = PySequence_Fast_GET_ITEM(fast, 0);
4102-
value = PySequence_Fast_GET_ITEM(fast, 1);
4103-
Py_INCREF(key);
4104-
Py_INCREF(value);
41054107
if (override) {
41064108
if (setitem_lock_held((PyDictObject *)d, key, value) < 0) {
41074109
Py_DECREF(key);

0 commit comments

Comments
 (0)