René's URL Explorer Experiment

Title: gh-151722: Fix data race on frozendict reads during construction in free-threading build by tonghuaroot · Pull Request #151724 · python/cpython · GitHub

Open Graph Title: gh-151722: Fix data race on frozendict reads during construction in free-threading build by tonghuaroot · Pull Request #151724 · python/cpython

X Title: gh-151722: Fix data race on frozendict reads during construction in free-threading build by tonghuaroot · Pull Request #151724 · python/cpython

Description: In the free-threading build, len(fd), repr(fd) and hash(fd) on a frozendict (PEP 814) read the object's internal state without synchronization. That is safe for a finished frozendict, but not while one is being constructed from a user mapping: the half-built object is already GC-tracked and reachable from other threads (the generic frozendict_new → dict_merge path runs user keys()/__getitem__, which can leak it via gc.get_objects()), so a reader races the construction-time inserts and resizes. ThreadSanitizer reports a data race, and a reader can observe the frozendict's length and contents change, which contradicts its immutability. What frozendict_length, the frozendict_repr/copy_lock_held empty-guards, and the frozendict_hash active-entry factor now read ma_used through GET_USED() (FT_ATOMIC_LOAD_SSIZE_RELAXED), matching dict_length and set_len. frozendict_repr now runs the _PyDict_Next entry-table walk (anydict_repr_impl) inside Py_BEGIN_CRITICAL_SECTION, exactly like dict_repr and like set_repr (the tp_repr of set/frozenset). frozendict_hash is split into a frozendict_hash_lock_held helper (the entry-table walk) wrapped in Py_BEGIN_CRITICAL_SECTION, with the cached ma_hash short-circuit left lock-free (atomic load), mirroring frozenset_hash/frozenset_hash_impl. Why this matches the existing conventions These are not new design choices; they mirror what dictobject.c and setobject.c already do: dict_repr wraps anydict_repr_impl in Py_BEGIN_CRITICAL_SECTION(self). set_repr wraps set_repr_lock_held the same way, and it is the tp_repr for both PySet_Type and PyFrozenSet_Type — so a frozen type already uses a critical-sectioned repr. The critical section is effective here because the construction side already holds Py_BEGIN_CRITICAL_SECTION(a) on the target in the generic dict_merge branch, and inserts run under it (setitem_lock_held). A reader that takes the object's critical section therefore serializes against construction. frozendict_hash was copied from frozenset_hash; keeping the cached-hash short-circuit lock-free with an atomic load keeps that parity. Scalar ma_used reads going through GET_USED() matches dict_length and set_len. The construction (write) side already uses STORE_USED (FT_ATOMIC_STORE_SSIZE_RELAXED) and a critical section, and is unchanged. Tests / verification On a free-threading + ThreadSanitizer build (./configure --disable-gil --with-thread-sanitizer), HEAD 9688d252d330b0b586760a121ee8c8f7215176e8: A reader thread hammering len/repr/hash on a frozendict that is still being constructed (leaked via gc.get_objects() from __getitem__) reports zero ThreadSanitizer warnings with this change, where unpatched main reports data races on all three. The lock-free read fast paths (__getitem__, get, in, iteration, __eq__) were already free-threading-safe and remain clean. FrozenDictTests / FrozenDictMappingTests (21 tests), test_set and test_free_threading all pass on the same build; test_dict passes. Note on frozendict copy copy_lock_held was deliberately made lock-free for frozendict sources in #145920. It reads source state the same way and so has the same latent construction-time race, but re-adding a critical section there would revert that performance decision, so this PR does not touch the copy table walk (only the scalar ma_used guard, which is a pure atomicity fix). The construction-window question for copy is discussed in the linked issue.

Open Graph Description: In the free-threading build, len(fd), repr(fd) and hash(fd) on a frozendict (PEP 814) read the object's internal state without synchronization. That is safe for a finished frozendict, but not w...

X Description: In the free-threading build, len(fd), repr(fd) and hash(fd) on a frozendict (PEP 814) read the object's internal state without synchronization. That is safe for a finished frozendict, but n...

Opengraph URL: https://github.com/python/cpython/pull/151724

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width