You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
pythongh-132869: Fix crash due to memory ordering problem in dictobject under free threading.
Currently the reads and writes to a dictionary entry and to its contents are
unordered. For example in `do_lookup` we have the following code:
```
for (;;) {
ix = dictkeys_get_index(dk, i);
if (ix >= 0) {
int cmp = check_lookup(mp, dk, ep0, ix, key, hash);
```
where `dictkeys_get_index` performs a relaxed atomic read of `dk_indices[i]`,
where the `check_lookup` function might be, say, compare_unicode_unicode which
makes a relaxed load of the me_key value in that index.
```
PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix];
PyObject *ep_key = FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key);
assert(ep_key != NULL);
```
However, the writer also does not order these two writes appropriately; for
example `insert_combined_dict` does the following:
```
Py_ssize_t hashpos = find_empty_slot(mp->ma_keys, hash);
dictkeys_set_index(mp->ma_keys, hashpos, mp->ma_keys->dk_nentries);
if (DK_IS_UNICODE(mp->ma_keys)) {
PyDictUnicodeEntry *ep;
ep = &DK_UNICODE_ENTRIES(mp->ma_keys)[mp->ma_keys->dk_nentries];
STORE_KEY(ep, key);
STORE_VALUE(ep, value);
}
else {
PyDictKeyEntry *ep;
ep = &DK_ENTRIES(mp->ma_keys)[mp->ma_keys->dk_nentries];
STORE_KEY(ep, key);
STORE_VALUE(ep, value);
STORE_HASH(ep, hash);
}
mp->ma_version_tag = new_version;
```
where the `dk_indices` value is set first, followed by setting the `me_key`,
both as relaxed writes. This is problematic because the write to
`dk_indices` may be ordered first, either by the program order on x86, or
allowed by the relaxed memory ordering semantics of ARM. The reader above
will be able to observe a state where the index has been set but the key value
is still null, leading to a crash in the reproducer of python#132869.
The fix is two-fold:
* order the index write after the the write to its contents, and
* use sequentially consistent reads and writes. It would suffice to use
load-acquire and store-release here but those atomic operations do not exist
in the CPython atomic headers at the necessary types.
I was only able to reproduce the crash under CPython 3.13, but I do not
see any reason the bug is fixed on the 3.14 branch either since the code
does not seem to have changed.
Fixespython#132869
0 commit comments