Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Include/internal/pycore_descrobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ extern PyTypeObject _PyMethodWrapper_Type;

extern void *_PyMember_GetOffset(PyObject *, PyMemberDef *);

/* Return a borrowed reference to the mapping wrapped by a mappingproxy.
* The struct layout matches mappingproxyobject in Objects/descrobject.c.
*/
static inline PyObject *
_PyDictProxy_GetMapping(PyObject *op)
{
typedef struct {
PyObject_HEAD
PyObject *mapping;
} _PyMappingProxyObject;
assert(op != NULL);
assert(PyObject_TypeCheck(op, &PyDictProxy_Type));
return ((_PyMappingProxyObject *)op)->mapping;
}

#ifdef __cplusplus
}
#endif
Expand Down
34 changes: 34 additions & 0 deletions Lib/test/test_dict_mappingproxy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
"""Tests for dict() / update / unpacking of types.MappingProxyType.

gh-157217: merging from a mappingproxy wrapping a dict should take the
locked dict-to-dict path instead of iterating the proxy unlocked.
"""

import collections
import types
import unittest


class MappingProxyDictMergeTests(unittest.TestCase):
def test_update_from_mappingproxy_dict(self):
d = {}
d.update(types.MappingProxyType({1: 1, 2: 2, 3: 3}))
self.assertEqual(d, {1: 1, 2: 2, 3: 3})

def test_dict_constructor_from_mappingproxy(self):
view = types.MappingProxyType({'a': 1, 'b': 2})
self.assertEqual(dict(view), {'a': 1, 'b': 2})
self.assertEqual({**view}, {'a': 1, 'b': 2})
dest = {'z': 0}
dest.update(view)
self.assertEqual(dest, {'z': 0, 'a': 1, 'b': 2})

def test_dict_constructor_from_mappingproxy_userdict(self):
self.assertEqual(
dict(types.MappingProxyType(collections.UserDict(x=1))),
{'x': 1},
)


if __name__ == '__main__':
unittest.main()
42 changes: 41 additions & 1 deletion Lib/test/test_free_threading/test_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from threading import Barrier, Thread
from unittest import TestCase
import sys
from test.support import import_helper, threading_helper
from test.support import import_helper, threading_helper, Py_GIL_DISABLED

_testinternalcapi = import_helper.import_module("_testinternalcapi")

Expand Down Expand Up @@ -401,6 +401,46 @@ class B(A):
with threading_helper.start_threads(threads):
pass

@unittest.skipUnless(Py_GIL_DISABLED,
"race only occurs on the free-threaded build")
def test_dir_racing_class_dict_insert(self):
# gh-157217: dir() iterated a mappingproxy of the class dict without
# holding that dict's critical section. A concurrent insert into the
# class dict (for example a lazy __annotations_cache__) then raised
# RuntimeError: dictionary changed size during iteration.
errors = []

class C:
x: int

for i in range(200):
setattr(C, f'attr_{i}', i)

def reader():
barrier.wait()
for _ in range(400):
try:
dir(C)
dict(vars(C))
{**vars(C)}
except RuntimeError as exc:
errors.append(exc)

def writer():
barrier.wait()
# First access stores __annotations_cache__ on the class.
C.__annotations__
for i in range(200):
setattr(C, f'extra_{i}', i)

n_readers = 4
barrier = threading.Barrier(n_readers + 1)
threads = [Thread(target=reader) for _ in range(n_readers)]
threads.append(Thread(target=writer))
with threading_helper.start_threads(threads):
pass
self.assertEqual(errors, [])


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix :func:`dir` raising :exc:`RuntimeError` on the free-threaded build
when another thread concurrently inserts into a class ``__dict__``.
``dict.update()`` and related merges now take the locked dict-to-dict path
when the source is a :class:`types.MappingProxyType` wrapping a dict.
17 changes: 14 additions & 3 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ As a consequence of this, split keys have a maximum size of 16.
#include "pycore_ceval.h" // _PyEval_GetBuiltin()
#include "pycore_code.h" // stats
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION, Py_END_CRITICAL_SECTION
#include "pycore_descrobject.h" // _PyDictProxy_GetMapping()
#include "pycore_dict.h" // export _PyDict_SizeOf()
#include "pycore_freelist.h" // _PyFreeListState_GET()
#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED()
Expand Down Expand Up @@ -4305,11 +4306,21 @@ dict_merge(PyObject *a, PyObject *b, int override, PyObject **dupkey)

PyDictObject *mp = _PyAnyDict_CAST(a);

/* Mapping proxies (including type.__dict__) wrap a real dict. Unwrap
* so we take the locked dict-to-dict path instead of iterating the
* proxy without holding the underlying dict's critical section.
* See gh-157217.
*/
PyObject *source = b;
if (PyObject_TypeCheck(b, &PyDictProxy_Type)) {
source = _PyDictProxy_GetMapping(b);
}

int res = 0;
if (PyAnyDict_Check(b) && (Py_TYPE(b)->tp_iter == dict_iter)) {
PyDictObject *other = (PyDictObject*)b;
if (PyAnyDict_Check(source) && (Py_TYPE(source)->tp_iter == dict_iter)) {
PyDictObject *other = (PyDictObject*)source;
int res;
Py_BEGIN_CRITICAL_SECTION2(a, b);
Py_BEGIN_CRITICAL_SECTION2(a, source);
assert(can_modify_dict(mp));
res = dict_dict_merge((PyDictObject *)a, other, override, dupkey);
ASSERT_CONSISTENT(a);
Expand Down
Loading