Skip to content

Commit 8942e4d

Browse files
committed
Re-probe for index with shared keys locked before adding split index
1 parent c4ae700 commit 8942e4d

File tree

1 file changed

+113
-28
lines changed

1 file changed

+113
-28
lines changed

Objects/dictobject.c

Lines changed: 113 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,15 +1679,26 @@ insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
16791679
return 0;
16801680
}
16811681

1682+
static int
1683+
split_dict_resize_and_insert(PyInterpreterState *interp, PyDictObject *mp,
1684+
Py_hash_t hash, PyObject *key, PyObject *value)
1685+
{
1686+
/* Need to resize. */
1687+
int ins = insertion_resize(interp, mp, 1);
1688+
if (ins < 0) {
1689+
return -1;
1690+
}
1691+
assert(!_PyDict_HasSplitTable(mp));
1692+
return insert_combined_dict(interp, mp, hash, key, value);
1693+
}
1694+
16821695
static int
16831696
insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
16841697
Py_hash_t hash, PyObject *key, PyObject *value)
16851698
{
16861699
PyDictKeysObject *keys = mp->ma_keys;
1687-
LOCK_KEYS(keys);
16881700
if (keys->dk_usable <= 0) {
16891701
/* Need to resize. */
1690-
UNLOCK_KEYS(keys);
16911702
int ins = insertion_resize(interp, mp, 1);
16921703
if (ins < 0) {
16931704
return -1;
@@ -1710,10 +1721,34 @@ insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
17101721

17111722
split_keys_entry_added(keys);
17121723
assert(keys->dk_usable >= 0);
1713-
UNLOCK_KEYS(keys);
17141724
return 0;
17151725
}
17161726

1727+
#ifdef Py_GIL_DISABLED
1728+
1729+
static inline Py_ssize_t
1730+
splitdict_lookup_threadsafe(PyDictObject *mp, PyDictKeysObject *dk,
1731+
PyObject *key, Py_ssize_t hash,
1732+
PyObject **value)
1733+
{
1734+
ASSERT_DICT_LOCKED(mp);
1735+
1736+
Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
1737+
1738+
if (ix >= 0) {
1739+
*value = mp->ma_values->values[ix];
1740+
}
1741+
else if (ix == DKIX_KEY_CHANGED) {
1742+
ix = _Py_dict_lookup(mp, key, hash, value);
1743+
}
1744+
else {
1745+
*value = 0;
1746+
}
1747+
return ix;
1748+
}
1749+
1750+
#endif
1751+
17171752
/*
17181753
Internal routine to insert a new item into the table.
17191754
Used both by the internal resize routine and by the public insert routine.
@@ -1745,17 +1780,7 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17451780
// half of _Py_dict_lookup_threadsafe and avoids locking the
17461781
// shared keys if we can
17471782
assert(PyUnicode_CheckExact(key));
1748-
ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
1749-
1750-
if (ix >= 0) {
1751-
old_value = mp->ma_values->values[ix];
1752-
}
1753-
else if (ix == DKIX_KEY_CHANGED) {
1754-
ix = _Py_dict_lookup(mp, key, hash, &old_value);
1755-
}
1756-
else {
1757-
old_value = NULL;
1758-
}
1783+
ix = splitdict_lookup_threadsafe(mp, dk, key, hash, &old_value);
17591784
} else {
17601785
ix = _Py_dict_lookup(mp, key, hash, &old_value);
17611786
}
@@ -1768,20 +1793,47 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17681793
MAINTAIN_TRACKING(mp, key, value);
17691794

17701795
if (ix == DKIX_EMPTY) {
1771-
uint64_t new_version = _PyDict_NotifyEvent(
1772-
interp, PyDict_EVENT_ADDED, mp, key, value);
1773-
/* Insert into new slot. */
1774-
mp->ma_keys->dk_version = 0;
1775-
assert(old_value == NULL);
1776-
1796+
uint64_t new_version;
17771797
if (!_PyDict_HasSplitTable(mp)) {
1798+
new_version = _PyDict_NotifyEvent(
1799+
interp, PyDict_EVENT_ADDED, mp, key, value);
1800+
/* Insert into new slot. */
1801+
mp->ma_keys->dk_version = 0;
17781802
if (insert_combined_dict(interp, mp, hash, key, value) < 0) {
17791803
goto Fail;
17801804
}
17811805
}
17821806
else {
1783-
if (insert_split_dict(interp, mp, hash, key, value) < 0)
1784-
goto Fail;
1807+
LOCK_KEYS(mp->ma_keys);
1808+
1809+
#ifdef Py_GIL_DISABLED
1810+
// We could have raced between our lookup before and the insert,
1811+
// so we need to lookup again with the keys locked
1812+
ix = splitdict_lookup_threadsafe(mp, dk, key, hash, &old_value);
1813+
if (ix >= 0) {
1814+
UNLOCK_KEYS(mp->ma_keys);
1815+
goto insert_on_split_race;
1816+
}
1817+
#endif
1818+
new_version = _PyDict_NotifyEvent(
1819+
interp, PyDict_EVENT_ADDED, mp, key, value);
1820+
/* Insert into new slot. */
1821+
mp->ma_keys->dk_version = 0;
1822+
if (mp->ma_keys->dk_usable <= 0) {
1823+
UNLOCK_KEYS(mp->ma_keys);
1824+
1825+
if (split_dict_resize_and_insert(interp, mp, hash, key, value) < 0) {
1826+
goto Fail;
1827+
}
1828+
} else {
1829+
int insert = insert_split_dict(interp, mp, hash, key, value);
1830+
UNLOCK_KEYS(mp->ma_keys);
1831+
1832+
if (insert < 0) {
1833+
goto Fail;
1834+
}
1835+
}
1836+
mp->ma_keys->dk_version = new_version;
17851837
}
17861838

17871839
mp->ma_used++;
@@ -1790,6 +1842,9 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17901842
return 0;
17911843
}
17921844

1845+
#ifdef Py_GIL_DISABLED
1846+
insert_on_split_race:
1847+
#endif
17931848
if (old_value != value) {
17941849
uint64_t new_version = _PyDict_NotifyEvent(
17951850
interp, PyDict_EVENT_MODIFIED, mp, key, value);
@@ -4258,13 +4313,40 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42584313
}
42594314
}
42604315
else {
4261-
if (insert_split_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value)) < 0) {
4262-
Py_DECREF(key);
4263-
Py_DECREF(value);
4264-
if (result) {
4265-
*result = NULL;
4316+
LOCK_KEYS(mp->ma_keys);
4317+
#ifdef Py_GIL_DISABLED
4318+
// We could have raced between our lookup before and the insert,
4319+
// so we need to lookup again with the keys locked
4320+
ix = _Py_dict_lookup(mp, key, hash, &value);
4321+
if (ix >= 0) {
4322+
UNLOCK_KEYS(mp->ma_keys);
4323+
if (value != NULL) {
4324+
if (result) {
4325+
*result = incref_result ? Py_NewRef(value) : value;
4326+
}
4327+
return 0;
4328+
}
4329+
goto insert_on_split_race;
4330+
}
4331+
#endif
4332+
if (mp->ma_keys->dk_usable <= 0) {
4333+
UNLOCK_KEYS(mp->ma_keys);
4334+
4335+
if (split_dict_resize_and_insert(interp, mp, hash, key, value) < 0) {
4336+
return -1;
4337+
}
4338+
} else {
4339+
int insert = insert_split_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value));
4340+
UNLOCK_KEYS(mp->ma_keys);
4341+
4342+
if (insert < 0) {
4343+
Py_DECREF(key);
4344+
Py_DECREF(value);
4345+
if (result) {
4346+
*result = NULL;
4347+
}
4348+
return -1;
42664349
}
4267-
return -1;
42684350
}
42694351
}
42704352

@@ -4279,6 +4361,9 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42794361
return 0;
42804362
}
42814363
else if (value == NULL) {
4364+
#ifdef Py_GIL_DISABLED
4365+
insert_on_split_race:
4366+
#endif
42824367
uint64_t new_version = _PyDict_NotifyEvent(
42834368
interp, PyDict_EVENT_ADDED, mp, key, default_value);
42844369
value = default_value;

0 commit comments

Comments
 (0)