Skip to content

Commit 2f15383

Browse files
[3.13] gh-112075: Fix dict thread safety issues (GH-119288) (#121545)
(cherry picked from commit f0ed186) Co-authored-by: Germán Méndez Bravo <[email protected]>
1 parent f0d16f7 commit 2f15383

File tree

1 file changed

+41
-25
lines changed

1 file changed

+41
-25
lines changed

Objects/dictobject.c

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ ASSERT_DICT_LOCKED(PyObject *op)
154154
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
155155
}
156156
#define ASSERT_DICT_LOCKED(op) ASSERT_DICT_LOCKED(_Py_CAST(PyObject*, op))
157+
#define ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op) \
158+
if (!_PyInterpreterState_GET()->stoptheworld.world_stopped) { \
159+
ASSERT_DICT_LOCKED(op); \
160+
}
161+
157162
#define IS_DICT_SHARED(mp) _PyObject_GC_IS_SHARED(mp)
158163
#define SET_DICT_SHARED(mp) _PyObject_GC_SET_SHARED(mp)
159164
#define LOAD_INDEX(keys, size, idx) _Py_atomic_load_int##size##_relaxed(&((const int##size##_t*)keys->dk_indices)[idx]);
@@ -221,6 +226,7 @@ static inline void split_keys_entry_added(PyDictKeysObject *keys)
221226
#else /* Py_GIL_DISABLED */
222227

223228
#define ASSERT_DICT_LOCKED(op)
229+
#define ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op)
224230
#define LOCK_KEYS(keys)
225231
#define UNLOCK_KEYS(keys)
226232
#define ASSERT_KEYS_LOCKED(keys)
@@ -473,7 +479,7 @@ dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk, bool use_qsbr)
473479
if (FT_ATOMIC_LOAD_SSIZE_RELAXED(dk->dk_refcnt) == _Py_IMMORTAL_REFCNT) {
474480
return;
475481
}
476-
assert(dk->dk_refcnt > 0);
482+
assert(FT_ATOMIC_LOAD_SSIZE(dk->dk_refcnt) > 0);
477483
#ifdef Py_REF_DEBUG
478484
_Py_DecRefTotal(_PyThreadState_GET());
479485
#endif
@@ -670,6 +676,8 @@ dump_entries(PyDictKeysObject *dk)
670676
int
671677
_PyDict_CheckConsistency(PyObject *op, int check_content)
672678
{
679+
ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op);
680+
673681
#define CHECK(expr) \
674682
do { if (!(expr)) { _PyObject_ASSERT_FAILED_MSG(op, Py_STRINGIFY(expr)); } } while (0)
675683

@@ -1580,6 +1588,8 @@ _PyDict_MaybeUntrack(PyObject *op)
15801588
PyObject *value;
15811589
Py_ssize_t i, numentries;
15821590

1591+
ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op);
1592+
15831593
if (!PyDict_CheckExact(op) || !_PyObject_GC_IS_TRACKED(op))
15841594
return;
15851595

@@ -1722,13 +1732,14 @@ static void
17221732
insert_split_value(PyInterpreterState *interp, PyDictObject *mp, PyObject *key, PyObject *value, Py_ssize_t ix)
17231733
{
17241734
assert(PyUnicode_CheckExact(key));
1735+
ASSERT_DICT_LOCKED(mp);
17251736
MAINTAIN_TRACKING(mp, key, value);
17261737
PyObject *old_value = mp->ma_values->values[ix];
17271738
if (old_value == NULL) {
17281739
uint64_t new_version = _PyDict_NotifyEvent(interp, PyDict_EVENT_ADDED, mp, key, value);
17291740
STORE_SPLIT_VALUE(mp, ix, Py_NewRef(value));
17301741
_PyDictValues_AddToInsertionOrder(mp->ma_values, ix);
1731-
mp->ma_used++;
1742+
STORE_USED(mp, mp->ma_used + 1);
17321743
mp->ma_version_tag = new_version;
17331744
}
17341745
else {
@@ -1792,7 +1803,7 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17921803
goto Fail;
17931804
}
17941805
mp->ma_version_tag = new_version;
1795-
mp->ma_used++;
1806+
STORE_USED(mp, mp->ma_used + 1);
17961807
ASSERT_CONSISTENT(mp);
17971808
return 0;
17981809
}
@@ -1861,7 +1872,7 @@ insert_to_emptydict(PyInterpreterState *interp, PyDictObject *mp,
18611872
ep->me_hash = hash;
18621873
STORE_VALUE(ep, value);
18631874
}
1864-
FT_ATOMIC_STORE_SSIZE_RELAXED(mp->ma_used, FT_ATOMIC_LOAD_SSIZE_RELAXED(mp->ma_used) + 1);
1875+
STORE_USED(mp, mp->ma_used + 1);
18651876
mp->ma_version_tag = new_version;
18661877
newkeys->dk_usable--;
18671878
newkeys->dk_nentries++;
@@ -1870,11 +1881,7 @@ insert_to_emptydict(PyInterpreterState *interp, PyDictObject *mp,
18701881
// the case where we're inserting from the non-owner thread. We don't use
18711882
// set_keys here because the transition from empty to non-empty is safe
18721883
// as the empty keys will never be freed.
1873-
#ifdef Py_GIL_DISABLED
1874-
_Py_atomic_store_ptr_release(&mp->ma_keys, newkeys);
1875-
#else
1876-
mp->ma_keys = newkeys;
1877-
#endif
1884+
FT_ATOMIC_STORE_PTR_RELEASE(mp->ma_keys, newkeys);
18781885
return 0;
18791886
}
18801887

@@ -2560,7 +2567,7 @@ delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
25602567
Py_ssize_t hashpos = lookdict_index(mp->ma_keys, hash, ix);
25612568
assert(hashpos >= 0);
25622569

2563-
FT_ATOMIC_STORE_SSIZE_RELAXED(mp->ma_used, FT_ATOMIC_LOAD_SSIZE(mp->ma_used) - 1);
2570+
STORE_USED(mp, mp->ma_used - 1);
25642571
mp->ma_version_tag = new_version;
25652572
if (_PyDict_HasSplitTable(mp)) {
25662573
assert(old_value == mp->ma_values->values[ix]);
@@ -2730,7 +2737,7 @@ clear_lock_held(PyObject *op)
27302737
// We don't inc ref empty keys because they're immortal
27312738
ensure_shared_on_resize(mp);
27322739
mp->ma_version_tag = new_version;
2733-
mp->ma_used = 0;
2740+
STORE_USED(mp, 0);
27342741
if (oldvalues == NULL) {
27352742
set_keys(mp, Py_EMPTY_KEYS);
27362743
assert(oldkeys->dk_refcnt == 1);
@@ -3160,6 +3167,8 @@ dict_repr_lock_held(PyObject *self)
31603167
_PyUnicodeWriter writer;
31613168
int first;
31623169

3170+
ASSERT_DICT_LOCKED(mp);
3171+
31633172
i = Py_ReprEnter((PyObject *)mp);
31643173
if (i != 0) {
31653174
return i > 0 ? PyUnicode_FromString("{...}") : NULL;
@@ -3248,8 +3257,7 @@ dict_repr(PyObject *self)
32483257
static Py_ssize_t
32493258
dict_length(PyObject *self)
32503259
{
3251-
PyDictObject *mp = (PyDictObject *)self;
3252-
return _Py_atomic_load_ssize_relaxed(&mp->ma_used);
3260+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)self)->ma_used);
32533261
}
32543262

32553263
static PyObject *
@@ -3640,6 +3648,9 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
36403648
static int
36413649
dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *other, int override)
36423650
{
3651+
ASSERT_DICT_LOCKED(mp);
3652+
ASSERT_DICT_LOCKED(other);
3653+
36433654
if (other == mp || other->ma_used == 0)
36443655
/* a.update(a) or a.update({}); nothing to do */
36453656
return 0;
@@ -3667,7 +3678,7 @@ dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *othe
36673678
ensure_shared_on_resize(mp);
36683679
dictkeys_decref(interp, mp->ma_keys, IS_DICT_SHARED(mp));
36693680
mp->ma_keys = keys;
3670-
mp->ma_used = other->ma_used;
3681+
STORE_USED(mp, other->ma_used);
36713682
mp->ma_version_tag = new_version;
36723683
ASSERT_CONSISTENT(mp);
36733684

@@ -4002,7 +4013,7 @@ PyDict_Size(PyObject *mp)
40024013
PyErr_BadInternalCall();
40034014
return -1;
40044015
}
4005-
return ((PyDictObject *)mp)->ma_used;
4016+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)mp)->ma_used);
40064017
}
40074018

40084019
/* Return 1 if dicts equal, 0 if not, -1 if error.
@@ -4256,7 +4267,7 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42564267
}
42574268

42584269
MAINTAIN_TRACKING(mp, key, value);
4259-
mp->ma_used++;
4270+
STORE_USED(mp, mp->ma_used + 1);
42604271
mp->ma_version_tag = new_version;
42614272
assert(mp->ma_keys->dk_usable >= 0);
42624273
ASSERT_CONSISTENT(mp);
@@ -4378,6 +4389,8 @@ dict_popitem_impl(PyDictObject *self)
43784389
uint64_t new_version;
43794390
PyInterpreterState *interp = _PyInterpreterState_GET();
43804391

4392+
ASSERT_DICT_LOCKED(self);
4393+
43814394
/* Allocate the result tuple before checking the size. Believe it
43824395
* or not, this allocation could trigger a garbage collection which
43834396
* could empty the dict, so if we checked the size first and that
@@ -4916,19 +4929,21 @@ typedef struct {
49164929
static PyObject *
49174930
dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
49184931
{
4932+
Py_ssize_t used;
49194933
dictiterobject *di;
49204934
di = PyObject_GC_New(dictiterobject, itertype);
49214935
if (di == NULL) {
49224936
return NULL;
49234937
}
49244938
di->di_dict = (PyDictObject*)Py_NewRef(dict);
4925-
di->di_used = dict->ma_used;
4926-
di->len = dict->ma_used;
4939+
used = FT_ATOMIC_LOAD_SSIZE_RELAXED(dict->ma_used);
4940+
di->di_used = used;
4941+
di->len = used;
49274942
if (itertype == &PyDictRevIterKey_Type ||
49284943
itertype == &PyDictRevIterItem_Type ||
49294944
itertype == &PyDictRevIterValue_Type) {
49304945
if (_PyDict_HasSplitTable(dict)) {
4931-
di->di_pos = dict->ma_used - 1;
4946+
di->di_pos = used - 1;
49324947
}
49334948
else {
49344949
di->di_pos = load_keys_nentries(dict) - 1;
@@ -4977,8 +4992,8 @@ dictiter_len(PyObject *self, PyObject *Py_UNUSED(ignored))
49774992
{
49784993
dictiterobject *di = (dictiterobject *)self;
49794994
Py_ssize_t len = 0;
4980-
if (di->di_dict != NULL && di->di_used == di->di_dict->ma_used)
4981-
len = di->len;
4995+
if (di->di_dict != NULL && di->di_used == FT_ATOMIC_LOAD_SSIZE_RELAXED(di->di_dict->ma_used))
4996+
len = FT_ATOMIC_LOAD_SSIZE_RELAXED(di->len);
49824997
return PyLong_FromSize_t(len);
49834998
}
49844999

@@ -5261,6 +5276,7 @@ dictiter_iternextitem_lock_held(PyDictObject *d, PyObject *self,
52615276
Py_ssize_t i;
52625277

52635278
assert (PyDict_Check(d));
5279+
ASSERT_DICT_LOCKED(d);
52645280

52655281
if (di->di_used != d->ma_used) {
52665282
PyErr_SetString(PyExc_RuntimeError,
@@ -5775,7 +5791,7 @@ dictview_len(PyObject *self)
57755791
_PyDictViewObject *dv = (_PyDictViewObject *)self;
57765792
Py_ssize_t len = 0;
57775793
if (dv->dv_dict != NULL)
5778-
len = dv->dv_dict->ma_used;
5794+
len = FT_ATOMIC_LOAD_SSIZE_RELAXED(dv->dv_dict->ma_used);
57795795
return len;
57805796
}
57815797

@@ -6782,15 +6798,15 @@ store_instance_attr_lock_held(PyObject *obj, PyDictValues *values,
67826798
_PyDictValues_AddToInsertionOrder(values, ix);
67836799
if (dict) {
67846800
assert(dict->ma_values == values);
6785-
dict->ma_used++;
6801+
STORE_USED(dict, dict->ma_used + 1);
67866802
}
67876803
}
67886804
else {
67896805
if (value == NULL) {
67906806
delete_index_from_values(values, ix);
67916807
if (dict) {
67926808
assert(dict->ma_values == values);
6793-
dict->ma_used--;
6809+
STORE_USED(dict, dict->ma_used - 1);
67946810
}
67956811
}
67966812
Py_DECREF(old_value);
@@ -7001,7 +7017,7 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj)
70017017
if (dict == NULL) {
70027018
return 1;
70037019
}
7004-
return ((PyDictObject *)dict)->ma_used == 0;
7020+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)dict)->ma_used) == 0;
70057021
}
70067022

70077023
int

0 commit comments

Comments
 (0)