gh-157217: Fix dir() race when merging from a mappingproxy - #157279
KingLizard1020 wants to merge 3 commits into
Conversation
| * The struct layout matches mappingproxyobject in Objects/descrobject.c. | ||
| */ | ||
| static inline PyObject * | ||
| _PyDictProxy_GetMapping(PyObject *op) |
There was a problem hiding this comment.
Why do you need it in the headers?
There was a problem hiding this comment.
We don't — the helper was only used from dict_merge(). I dropped _PyDictProxy_GetMapping from the header and unwrapped the mapping locally there (Py_IS_TYPE, since mappingproxy is not a base type).
a3a6cf9 to
c0381c5
Compare
Unwrap mappingproxy when it wraps a dict so dict_merge uses the locked dict-to-dict path. Add mappingproxy merge tests and a free-threading dir() race regression test.
504aade to
7569c3b
Compare
|
@KingLizard1020 Please don't force push PRs. We squash merge at the end, and it's easier to review without force pushes. Also force pushes can end up spamming all the CODEOWNERS. https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push Also please don't use things like It's confusing to see GitHub Actions force push, and also it requested reviews from 6 people in CODEOWNERs for GHA files. Same applies to #157691. Thanks! |
dir(),dict(vars(cls)), and{**vars(cls)}go throughdict_merge()on a mappingproxy of the class dict. The generic merge path calledPyMapping_Keys()without holding the wrapped dict's critical section. On the free-threaded build, a concurrent insert (for example the first access to__annotations__, which stores__annotations_cache__) raisedRuntimeError: dictionary changed size during iteration.Unwrap
types.MappingProxyTypewhen it wraps a dict so the locked dict-to-dict path is used.Py_BEGIN_CRITICAL_SECTION2already treats a self-merge (a == source) as a single mutex.Tests:
Lib/test/test_dict_mappingproxy.pyfordict(),{**view}, andupdatefrom a mappingproxy (including aUserDictwrap, which still uses the generic path)test_dir_racing_class_dict_insertinLib/test/test_free_threading/test_type.py(free-threaded only). This failed with manyRuntimeErrors without the C change and passed with it.Fixes #157217
dir()can raiseRuntimeError: dictionary changed size during iterationon the free-threaded build #157217