Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f0ed186

Browse filesBrowse files
KronuzKronuz
authored
gh-112075: Fix dict thread safety issues (#119288)
Fix dict thread safety issues
1 parent bf5b646 commit f0ed186
Copy full SHA for f0ed186

File tree

1 file changed

+41
-25
lines changed
Filter options

1 file changed

+41
-25
lines changed

‎Objects/dictobject.c

Copy file name to clipboardExpand all lines: Objects/dictobject.c
+41-25Lines 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

@@ -2580,7 +2587,7 @@ delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
25802587
Py_ssize_t hashpos = lookdict_index(mp->ma_keys, hash, ix);
25812588
assert(hashpos >= 0);
25822589

2583-
FT_ATOMIC_STORE_SSIZE_RELAXED(mp->ma_used, FT_ATOMIC_LOAD_SSIZE(mp->ma_used) - 1);
2590+
STORE_USED(mp, mp->ma_used - 1);
25842591
mp->ma_version_tag = new_version;
25852592
if (_PyDict_HasSplitTable(mp)) {
25862593
assert(old_value == mp->ma_values->values[ix]);
@@ -2752,7 +2759,7 @@ clear_lock_held(PyObject *op)
27522759
// We don't inc ref empty keys because they're immortal
27532760
ensure_shared_on_resize(mp);
27542761
mp->ma_version_tag = new_version;
2755-
mp->ma_used = 0;
2762+
STORE_USED(mp, 0);
27562763
if (oldvalues == NULL) {
27572764
set_keys(mp, Py_EMPTY_KEYS);
27582765
assert(oldkeys->dk_refcnt == 1);
@@ -3191,6 +3198,8 @@ dict_repr_lock_held(PyObject *self)
31913198
_PyUnicodeWriter writer;
31923199
int first;
31933200

3201+
ASSERT_DICT_LOCKED(mp);
3202+
31943203
i = Py_ReprEnter((PyObject *)mp);
31953204
if (i != 0) {
31963205
return i > 0 ? PyUnicode_FromString("{...}") : NULL;
@@ -3279,8 +3288,7 @@ dict_repr(PyObject *self)
32793288
static Py_ssize_t
32803289
dict_length(PyObject *self)
32813290
{
3282-
PyDictObject *mp = (PyDictObject *)self;
3283-
return _Py_atomic_load_ssize_relaxed(&mp->ma_used);
3291+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)self)->ma_used);
32843292
}
32853293

32863294
static PyObject *
@@ -3672,6 +3680,9 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
36723680
static int
36733681
dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *other, int override)
36743682
{
3683+
ASSERT_DICT_LOCKED(mp);
3684+
ASSERT_DICT_LOCKED(other);
3685+
36753686
if (other == mp || other->ma_used == 0)
36763687
/* a.update(a) or a.update({}); nothing to do */
36773688
return 0;
@@ -3699,7 +3710,7 @@ dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *othe
36993710
ensure_shared_on_resize(mp);
37003711
dictkeys_decref(interp, mp->ma_keys, IS_DICT_SHARED(mp));
37013712
mp->ma_keys = keys;
3702-
mp->ma_used = other->ma_used;
3713+
STORE_USED(mp, other->ma_used);
37033714
mp->ma_version_tag = new_version;
37043715
ASSERT_CONSISTENT(mp);
37053716

@@ -4034,7 +4045,7 @@ PyDict_Size(PyObject *mp)
40344045
PyErr_BadInternalCall();
40354046
return -1;
40364047
}
4037-
return ((PyDictObject *)mp)->ma_used;
4048+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)mp)->ma_used);
40384049
}
40394050

40404051
/* Return 1 if dicts equal, 0 if not, -1 if error.
@@ -4291,7 +4302,7 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42914302
}
42924303

42934304
MAINTAIN_TRACKING(mp, key, value);
4294-
mp->ma_used++;
4305+
STORE_USED(mp, mp->ma_used + 1);
42954306
mp->ma_version_tag = new_version;
42964307
assert(mp->ma_keys->dk_usable >= 0);
42974308
ASSERT_CONSISTENT(mp);
@@ -4413,6 +4424,8 @@ dict_popitem_impl(PyDictObject *self)
44134424
uint64_t new_version;
44144425
PyInterpreterState *interp = _PyInterpreterState_GET();
44154426

4427+
ASSERT_DICT_LOCKED(self);
4428+
44164429
/* Allocate the result tuple before checking the size. Believe it
44174430
* or not, this allocation could trigger a garbage collection which
44184431
* could empty the dict, so if we checked the size first and that
@@ -4952,19 +4965,21 @@ typedef struct {
49524965
static PyObject *
49534966
dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
49544967
{
4968+
Py_ssize_t used;
49554969
dictiterobject *di;
49564970
di = PyObject_GC_New(dictiterobject, itertype);
49574971
if (di == NULL) {
49584972
return NULL;
49594973
}
49604974
di->di_dict = (PyDictObject*)Py_NewRef(dict);
4961-
di->di_used = dict->ma_used;
4962-
di->len = dict->ma_used;
4975+
used = FT_ATOMIC_LOAD_SSIZE_RELAXED(dict->ma_used);
4976+
di->di_used = used;
4977+
di->len = used;
49634978
if (itertype == &PyDictRevIterKey_Type ||
49644979
itertype == &PyDictRevIterItem_Type ||
49654980
itertype == &PyDictRevIterValue_Type) {
49664981
if (_PyDict_HasSplitTable(dict)) {
4967-
di->di_pos = dict->ma_used - 1;
4982+
di->di_pos = used - 1;
49684983
}
49694984
else {
49704985
di->di_pos = load_keys_nentries(dict) - 1;
@@ -5013,8 +5028,8 @@ dictiter_len(PyObject *self, PyObject *Py_UNUSED(ignored))
50135028
{
50145029
dictiterobject *di = (dictiterobject *)self;
50155030
Py_ssize_t len = 0;
5016-
if (di->di_dict != NULL && di->di_used == di->di_dict->ma_used)
5017-
len = di->len;
5031+
if (di->di_dict != NULL && di->di_used == FT_ATOMIC_LOAD_SSIZE_RELAXED(di->di_dict->ma_used))
5032+
len = FT_ATOMIC_LOAD_SSIZE_RELAXED(di->len);
50185033
return PyLong_FromSize_t(len);
50195034
}
50205035

@@ -5297,6 +5312,7 @@ dictiter_iternextitem_lock_held(PyDictObject *d, PyObject *self,
52975312
Py_ssize_t i;
52985313

52995314
assert (PyDict_Check(d));
5315+
ASSERT_DICT_LOCKED(d);
53005316

53015317
if (di->di_used != d->ma_used) {
53025318
PyErr_SetString(PyExc_RuntimeError,
@@ -5811,7 +5827,7 @@ dictview_len(PyObject *self)
58115827
_PyDictViewObject *dv = (_PyDictViewObject *)self;
58125828
Py_ssize_t len = 0;
58135829
if (dv->dv_dict != NULL)
5814-
len = dv->dv_dict->ma_used;
5830+
len = FT_ATOMIC_LOAD_SSIZE_RELAXED(dv->dv_dict->ma_used);
58155831
return len;
58165832
}
58175833

@@ -6820,15 +6836,15 @@ store_instance_attr_lock_held(PyObject *obj, PyDictValues *values,
68206836
_PyDictValues_AddToInsertionOrder(values, ix);
68216837
if (dict) {
68226838
assert(dict->ma_values == values);
6823-
dict->ma_used++;
6839+
STORE_USED(dict, dict->ma_used + 1);
68246840
}
68256841
}
68266842
else {
68276843
if (value == NULL) {
68286844
delete_index_from_values(values, ix);
68296845
if (dict) {
68306846
assert(dict->ma_values == values);
6831-
dict->ma_used--;
6847+
STORE_USED(dict, dict->ma_used - 1);
68326848
}
68336849
}
68346850
Py_DECREF(old_value);
@@ -7039,7 +7055,7 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj)
70397055
if (dict == NULL) {
70407056
return 1;
70417057
}
7042-
return ((PyDictObject *)dict)->ma_used == 0;
7058+
return FT_ATOMIC_LOAD_SSIZE_RELAXED(((PyDictObject *)dict)->ma_used) == 0;
70437059
}
70447060

70457061
int

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.