Skip to content

Commit 2f5f5a1

Browse files
committed
Re-probe for index with shared keys locked before adding split index
1 parent f001e4c commit 2f5f5a1

File tree

1 file changed

+113
-28
lines changed

1 file changed

+113
-28
lines changed

Objects/dictobject.c

+113-28
Original file line numberDiff line numberDiff line change
@@ -1690,15 +1690,26 @@ insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
16901690
return 0;
16911691
}
16921692

1693+
static int
1694+
split_dict_resize_and_insert(PyInterpreterState *interp, PyDictObject *mp,
1695+
Py_hash_t hash, PyObject *key, PyObject *value)
1696+
{
1697+
/* Need to resize. */
1698+
int ins = insertion_resize(interp, mp, 1);
1699+
if (ins < 0) {
1700+
return -1;
1701+
}
1702+
assert(!_PyDict_HasSplitTable(mp));
1703+
return insert_combined_dict(interp, mp, hash, key, value);
1704+
}
1705+
16931706
static int
16941707
insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
16951708
Py_hash_t hash, PyObject *key, PyObject *value)
16961709
{
16971710
PyDictKeysObject *keys = mp->ma_keys;
1698-
LOCK_KEYS(keys);
16991711
if (keys->dk_usable <= 0) {
17001712
/* Need to resize. */
1701-
UNLOCK_KEYS(keys);
17021713
int ins = insertion_resize(interp, mp, 1);
17031714
if (ins < 0) {
17041715
return -1;
@@ -1721,10 +1732,34 @@ insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
17211732

17221733
split_keys_entry_added(keys);
17231734
assert(keys->dk_usable >= 0);
1724-
UNLOCK_KEYS(keys);
17251735
return 0;
17261736
}
17271737

1738+
#ifdef Py_GIL_DISABLED
1739+
1740+
static inline Py_ssize_t
1741+
splitdict_lookup_threadsafe(PyDictObject *mp, PyDictKeysObject *dk,
1742+
PyObject *key, Py_ssize_t hash,
1743+
PyObject **value)
1744+
{
1745+
ASSERT_DICT_LOCKED(mp);
1746+
1747+
Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
1748+
1749+
if (ix >= 0) {
1750+
*value = mp->ma_values->values[ix];
1751+
}
1752+
else if (ix == DKIX_KEY_CHANGED) {
1753+
ix = _Py_dict_lookup(mp, key, hash, value);
1754+
}
1755+
else {
1756+
*value = 0;
1757+
}
1758+
return ix;
1759+
}
1760+
1761+
#endif
1762+
17281763
/*
17291764
Internal routine to insert a new item into the table.
17301765
Used both by the internal resize routine and by the public insert routine.
@@ -1756,17 +1791,7 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17561791
// half of _Py_dict_lookup_threadsafe and avoids locking the
17571792
// shared keys if we can
17581793
assert(PyUnicode_CheckExact(key));
1759-
ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
1760-
1761-
if (ix >= 0) {
1762-
old_value = mp->ma_values->values[ix];
1763-
}
1764-
else if (ix == DKIX_KEY_CHANGED) {
1765-
ix = _Py_dict_lookup(mp, key, hash, &old_value);
1766-
}
1767-
else {
1768-
old_value = NULL;
1769-
}
1794+
ix = splitdict_lookup_threadsafe(mp, dk, key, hash, &old_value);
17701795
} else {
17711796
ix = _Py_dict_lookup(mp, key, hash, &old_value);
17721797
}
@@ -1779,20 +1804,47 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
17791804
MAINTAIN_TRACKING(mp, key, value);
17801805

17811806
if (ix == DKIX_EMPTY) {
1782-
uint64_t new_version = _PyDict_NotifyEvent(
1783-
interp, PyDict_EVENT_ADDED, mp, key, value);
1784-
/* Insert into new slot. */
1785-
mp->ma_keys->dk_version = 0;
1786-
assert(old_value == NULL);
1787-
1807+
uint64_t new_version;
17881808
if (!_PyDict_HasSplitTable(mp)) {
1809+
new_version = _PyDict_NotifyEvent(
1810+
interp, PyDict_EVENT_ADDED, mp, key, value);
1811+
/* Insert into new slot. */
1812+
mp->ma_keys->dk_version = 0;
17891813
if (insert_combined_dict(interp, mp, hash, key, value) < 0) {
17901814
goto Fail;
17911815
}
17921816
}
17931817
else {
1794-
if (insert_split_dict(interp, mp, hash, key, value) < 0)
1795-
goto Fail;
1818+
LOCK_KEYS(mp->ma_keys);
1819+
1820+
#ifdef Py_GIL_DISABLED
1821+
// We could have raced between our lookup before and the insert,
1822+
// so we need to lookup again with the keys locked
1823+
ix = splitdict_lookup_threadsafe(mp, dk, key, hash, &old_value);
1824+
if (ix >= 0) {
1825+
UNLOCK_KEYS(mp->ma_keys);
1826+
goto insert_on_split_race;
1827+
}
1828+
#endif
1829+
new_version = _PyDict_NotifyEvent(
1830+
interp, PyDict_EVENT_ADDED, mp, key, value);
1831+
/* Insert into new slot. */
1832+
mp->ma_keys->dk_version = 0;
1833+
if (mp->ma_keys->dk_usable <= 0) {
1834+
UNLOCK_KEYS(mp->ma_keys);
1835+
1836+
if (split_dict_resize_and_insert(interp, mp, hash, key, value) < 0) {
1837+
goto Fail;
1838+
}
1839+
} else {
1840+
int insert = insert_split_dict(interp, mp, hash, key, value);
1841+
UNLOCK_KEYS(mp->ma_keys);
1842+
1843+
if (insert < 0) {
1844+
goto Fail;
1845+
}
1846+
}
1847+
mp->ma_keys->dk_version = new_version;
17961848
}
17971849

17981850
mp->ma_used++;
@@ -1801,6 +1853,9 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
18011853
return 0;
18021854
}
18031855

1856+
#ifdef Py_GIL_DISABLED
1857+
insert_on_split_race:
1858+
#endif
18041859
if (old_value != value) {
18051860
uint64_t new_version = _PyDict_NotifyEvent(
18061861
interp, PyDict_EVENT_MODIFIED, mp, key, value);
@@ -4251,13 +4306,40 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42514306
}
42524307
}
42534308
else {
4254-
if (insert_split_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value)) < 0) {
4255-
Py_DECREF(key);
4256-
Py_DECREF(value);
4257-
if (result) {
4258-
*result = NULL;
4309+
LOCK_KEYS(mp->ma_keys);
4310+
#ifdef Py_GIL_DISABLED
4311+
// We could have raced between our lookup before and the insert,
4312+
// so we need to lookup again with the keys locked
4313+
ix = _Py_dict_lookup(mp, key, hash, &value);
4314+
if (ix >= 0) {
4315+
UNLOCK_KEYS(mp->ma_keys);
4316+
if (value != NULL) {
4317+
if (result) {
4318+
*result = incref_result ? Py_NewRef(value) : value;
4319+
}
4320+
return 0;
4321+
}
4322+
goto insert_on_split_race;
4323+
}
4324+
#endif
4325+
if (mp->ma_keys->dk_usable <= 0) {
4326+
UNLOCK_KEYS(mp->ma_keys);
4327+
4328+
if (split_dict_resize_and_insert(interp, mp, hash, key, value) < 0) {
4329+
return -1;
4330+
}
4331+
} else {
4332+
int insert = insert_split_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value));
4333+
UNLOCK_KEYS(mp->ma_keys);
4334+
4335+
if (insert < 0) {
4336+
Py_DECREF(key);
4337+
Py_DECREF(value);
4338+
if (result) {
4339+
*result = NULL;
4340+
}
4341+
return -1;
42594342
}
4260-
return -1;
42614343
}
42624344
}
42634345

@@ -4272,6 +4354,9 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
42724354
return 0;
42734355
}
42744356
else if (value == NULL) {
4357+
#ifdef Py_GIL_DISABLED
4358+
insert_on_split_race:
4359+
#endif
42754360
uint64_t new_version = _PyDict_NotifyEvent(
42764361
interp, PyDict_EVENT_ADDED, mp, key, default_value);
42774362
value = default_value;

0 commit comments

Comments
 (0)