Skip to content

Commit 927f882

Browse files
[3.13] gh-117657: Fix TSan race in _PyDict_CheckConsistency (GH-121551) (#121590)
The only remaining race in dictobject.c was in _PyDict_CheckConsistency when the dictionary has shared keys. (cherry picked from commit 3ec719f) Co-authored-by: Sam Gross <[email protected]>
1 parent 867cf40 commit 927f882

File tree

2 files changed

+15
-17
lines changed

2 files changed

+15
-17
lines changed

Objects/dictobject.c

+15-9
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,15 @@ ASSERT_DICT_LOCKED(PyObject *op)
165165
#define STORE_INDEX(keys, size, idx, value) _Py_atomic_store_int##size##_relaxed(&((int##size##_t*)keys->dk_indices)[idx], (int##size##_t)value);
166166
#define ASSERT_OWNED_OR_SHARED(mp) \
167167
assert(_Py_IsOwnedByCurrentThread((PyObject *)mp) || IS_DICT_SHARED(mp));
168-
#define LOAD_KEYS_NENTRIES(d)
169168

170169
#define LOCK_KEYS_IF_SPLIT(keys, kind) \
171170
if (kind == DICT_KEYS_SPLIT) { \
172-
LOCK_KEYS(dk); \
171+
LOCK_KEYS(keys); \
173172
}
174173

175174
#define UNLOCK_KEYS_IF_SPLIT(keys, kind) \
176175
if (kind == DICT_KEYS_SPLIT) { \
177-
UNLOCK_KEYS(dk); \
176+
UNLOCK_KEYS(keys); \
178177
}
179178

180179
static inline Py_ssize_t
@@ -208,7 +207,7 @@ set_values(PyDictObject *mp, PyDictValues *values)
208207
#define INCREF_KEYS(dk) _Py_atomic_add_ssize(&dk->dk_refcnt, 1)
209208
// Dec refs the keys object, giving the previous value
210209
#define DECREF_KEYS(dk) _Py_atomic_add_ssize(&dk->dk_refcnt, -1)
211-
#define LOAD_KEYS_NENTIRES(keys) _Py_atomic_load_ssize_relaxed(&keys->dk_nentries)
210+
#define LOAD_KEYS_NENTRIES(keys) _Py_atomic_load_ssize_relaxed(&keys->dk_nentries)
212211

213212
#define INCREF_KEYS_FT(dk) dictkeys_incref(dk)
214213
#define DECREF_KEYS_FT(dk, shared) dictkeys_decref(_PyInterpreterState_GET(), dk, shared)
@@ -234,7 +233,7 @@ static inline void split_keys_entry_added(PyDictKeysObject *keys)
234233
#define STORE_SHARED_KEY(key, value) key = value
235234
#define INCREF_KEYS(dk) dk->dk_refcnt++
236235
#define DECREF_KEYS(dk) dk->dk_refcnt--
237-
#define LOAD_KEYS_NENTIRES(keys) keys->dk_nentries
236+
#define LOAD_KEYS_NENTRIES(keys) keys->dk_nentries
238237
#define INCREF_KEYS_FT(dk)
239238
#define DECREF_KEYS_FT(dk, shared)
240239
#define LOCK_KEYS_IF_SPLIT(keys, kind)
@@ -689,10 +688,15 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
689688
int splitted = _PyDict_HasSplitTable(mp);
690689
Py_ssize_t usable = USABLE_FRACTION(DK_SIZE(keys));
691690

691+
// In the free-threaded build, shared keys may be concurrently modified,
692+
// so use atomic loads.
693+
Py_ssize_t dk_usable = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(keys->dk_usable);
694+
Py_ssize_t dk_nentries = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(keys->dk_nentries);
695+
692696
CHECK(0 <= mp->ma_used && mp->ma_used <= usable);
693-
CHECK(0 <= keys->dk_usable && keys->dk_usable <= usable);
694-
CHECK(0 <= keys->dk_nentries && keys->dk_nentries <= usable);
695-
CHECK(keys->dk_usable + keys->dk_nentries <= usable);
697+
CHECK(0 <= dk_usable && dk_usable <= usable);
698+
CHECK(0 <= dk_nentries && dk_nentries <= usable);
699+
CHECK(dk_usable + dk_nentries <= usable);
696700

697701
if (!splitted) {
698702
/* combined table */
@@ -709,6 +713,7 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
709713
}
710714

711715
if (check_content) {
716+
LOCK_KEYS_IF_SPLIT(keys, keys->dk_kind);
712717
for (Py_ssize_t i=0; i < DK_SIZE(keys); i++) {
713718
Py_ssize_t ix = dictkeys_get_index(keys, i);
714719
CHECK(DKIX_DUMMY <= ix && ix <= usable);
@@ -764,6 +769,7 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
764769
CHECK(mp->ma_values->values[index] != NULL);
765770
}
766771
}
772+
UNLOCK_KEYS_IF_SPLIT(keys, keys->dk_kind);
767773
}
768774
return 1;
769775

@@ -4032,7 +4038,7 @@ dict_equal_lock_held(PyDictObject *a, PyDictObject *b)
40324038
/* can't be equal if # of entries differ */
40334039
return 0;
40344040
/* Same # of entries -- check all of 'em. Exit early on any diff. */
4035-
for (i = 0; i < LOAD_KEYS_NENTIRES(a->ma_keys); i++) {
4041+
for (i = 0; i < LOAD_KEYS_NENTRIES(a->ma_keys); i++) {
40364042
PyObject *key, *aval;
40374043
Py_hash_t hash;
40384044
if (DK_IS_UNICODE(a->ma_keys)) {

Tools/tsan/suppressions_free_threading.txt

-8
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,8 @@ race_top:_add_to_weak_set
2727
race_top:_in_weak_set
2828
race_top:_PyEval_EvalFrameDefault
2929
race_top:assign_version_tag
30-
race_top:insertdict
31-
race_top:lookup_tp_dict
3230
race_top:new_reference
33-
race_top:_PyDict_CheckConsistency
34-
race_top:_Py_dict_lookup_threadsafe
3531
race_top:_multiprocessing_SemLock_acquire_impl
36-
race_top:dictiter_new
37-
race_top:dictresize
38-
race_top:insert_to_emptydict
39-
race_top:insertdict
4032
race_top:list_get_item_ref
4133
race_top:make_pending_calls
4234
race_top:_Py_slot_tp_getattr_hook

0 commit comments

Comments
 (0)