mirror of
https://github.com/python/cpython.git
synced 2026-05-03 09:01:07 +00:00
gh-144295: Fix data race in dict method lookup and global load (gh-144312)
In `_PyDict_GetMethodStackRef`, only use the fast-path unicode lookup when the dict is owned by the current thread or already marked as shared. This prevents a race between the lookup and concurrent dict resizes, which may free the PyDictKeysObject (i.e., it ensures that the resize uses QSBR). Address a similar issue in `_Py_dict_lookup_threadsafe_stackref` by calling `ensure_shared_on_read()`.
This commit is contained in:
parent
1dc12b2883
commit
e666a01ef4
2 changed files with 45 additions and 13 deletions
|
|
@ -245,5 +245,27 @@ def reader():
|
|||
with threading_helper.start_threads([t1, t2]):
|
||||
pass
|
||||
|
||||
def test_racing_dict_update_and_method_lookup(self):
|
||||
# gh-144295: test race between dict modifications and method lookups.
|
||||
# Uses BytesIO because the race requires a type without Py_TPFLAGS_INLINE_VALUES
|
||||
# for the _PyDict_GetMethodStackRef code path.
|
||||
import io
|
||||
obj = io.BytesIO()
|
||||
|
||||
def writer():
|
||||
for _ in range(10000):
|
||||
obj.x = 1
|
||||
del obj.x
|
||||
|
||||
def reader():
|
||||
for _ in range(10000):
|
||||
obj.getvalue()
|
||||
|
||||
t1 = Thread(target=writer)
|
||||
t2 = Thread(target=reader)
|
||||
|
||||
with threading_helper.start_threads([t1, t2]):
|
||||
pass
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
|
|
@ -1615,7 +1615,9 @@ lookup_threadsafe_unicode(PyDictKeysObject *dk, PyObject *key, Py_hash_t hash, _
|
|||
Py_ssize_t
|
||||
_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr)
|
||||
{
|
||||
PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys);
|
||||
ensure_shared_on_read(mp);
|
||||
|
||||
PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys);
|
||||
if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) {
|
||||
Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, value_addr);
|
||||
if (ix != DKIX_KEY_CHANGED) {
|
||||
|
|
@ -1669,19 +1671,27 @@ _PyDict_GetMethodStackRef(PyDictObject *mp, PyObject *key, _PyStackRef *method)
|
|||
Py_hash_t hash = hash_unicode_key(key);
|
||||
|
||||
#ifdef Py_GIL_DISABLED
|
||||
PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys);
|
||||
if (dk->dk_kind == DICT_KEYS_UNICODE) {
|
||||
_PyStackRef ref;
|
||||
Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref);
|
||||
if (ix >= 0) {
|
||||
assert(!PyStackRef_IsNull(ref));
|
||||
PyStackRef_XSETREF(*method, ref);
|
||||
return 1;
|
||||
// NOTE: We can only do the fast-path lookup if we are on the owning
|
||||
// thread or if the dict is already marked as shared so that the load
|
||||
// of ma_keys is safe without a lock. We cannot call ensure_shared_on_read()
|
||||
// in this code path without incref'ing the dict because the dict is a
|
||||
// borrowed reference protected by QSBR, and acquiring the lock could lead
|
||||
// to a quiescent state (allowing the dict to be freed).
|
||||
if (_Py_IsOwnedByCurrentThread((PyObject *)mp) || IS_DICT_SHARED(mp)) {
|
||||
PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys);
|
||||
if (dk->dk_kind == DICT_KEYS_UNICODE) {
|
||||
_PyStackRef ref;
|
||||
Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref);
|
||||
if (ix >= 0) {
|
||||
assert(!PyStackRef_IsNull(ref));
|
||||
PyStackRef_XSETREF(*method, ref);
|
||||
return 1;
|
||||
}
|
||||
else if (ix == DKIX_EMPTY) {
|
||||
return 0;
|
||||
}
|
||||
assert(ix == DKIX_KEY_CHANGED);
|
||||
}
|
||||
else if (ix == DKIX_EMPTY) {
|
||||
return 0;
|
||||
}
|
||||
assert(ix == DKIX_KEY_CHANGED);
|
||||
}
|
||||
#endif
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue