Skip to content

Commit 355a6ce

Browse files
committed
OrderedDict thread safety
1 parent 3217169 commit 355a6ce

File tree

4 files changed

+84
-16
lines changed

4 files changed

+84
-16
lines changed

Include/internal/pycore_dict.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ extern int _PyDict_DelItemIf(PyObject *mp, PyObject *key,
2222
// Export for '_asyncio' shared extension
2323
PyAPI_FUNC(int) _PyDict_SetItem_KnownHash(PyObject *mp, PyObject *key,
2424
PyObject *item, Py_hash_t hash);
25+
int _PyDict_SetItem_KnownHash_LockHeld(PyObject *mp, PyObject *key,
26+
PyObject *item, Py_hash_t hash);
27+
2528
// Export for '_asyncio' shared extension
2629
PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key,
2730
Py_hash_t hash);

Objects/clinic/odictobject.c.h

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Objects/dictobject.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1915,7 +1915,7 @@ PyDict_SetItem(PyObject *op, PyObject *key, PyObject *value)
19151915
}
19161916

19171917
int
1918-
_PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
1918+
_PyDict_SetItem_KnownHash_LockHeld(PyObject *op, PyObject *key, PyObject *value,
19191919
Py_hash_t hash)
19201920
{
19211921
PyDictObject *mp;
@@ -1937,6 +1937,19 @@ _PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
19371937
return insertdict(interp, mp, Py_NewRef(key), hash, Py_NewRef(value));
19381938
}
19391939

1940+
int
1941+
_PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
1942+
Py_hash_t hash)
1943+
{
1944+
int res;
1945+
1946+
Py_BEGIN_CRITICAL_SECTION(op);
1947+
res = _PyDict_SetItem_KnownHash_LockHeld(op, key, value, hash);
1948+
1949+
Py_END_CRITICAL_SECTION();
1950+
return res;
1951+
}
1952+
19401953
static void
19411954
delete_index_from_values(PyDictValues *values, Py_ssize_t ix)
19421955
{

Objects/odictobject.c

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ Potential Optimizations
467467
#include "Python.h"
468468
#include "pycore_call.h" // _PyObject_CallNoArgs()
469469
#include "pycore_ceval.h" // _PyEval_GetBuiltin()
470+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION
470471
#include "pycore_dict.h" // _Py_dict_lookup()
471472
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
472473
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
@@ -984,6 +985,7 @@ odict_reduce(register PyODictObject *od, PyObject *Py_UNUSED(ignored))
984985

985986

986987
/*[clinic input]
988+
@critical_section
987989
OrderedDict.setdefault
988990
989991
key: object
@@ -997,7 +999,7 @@ Return the value for key if key is in the dictionary, else default.
997999
static PyObject *
9981000
OrderedDict_setdefault_impl(PyODictObject *self, PyObject *key,
9991001
PyObject *default_value)
1000-
/*[clinic end generated code: output=97537cb7c28464b6 input=38e098381c1efbc6]*/
1002+
/*[clinic end generated code: output=97537cb7c28464b6 input=d7b93e92734f99b5]*/
10011003
{
10021004
PyObject *result = NULL;
10031005

@@ -1069,6 +1071,7 @@ _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj,
10691071

10701072
/* Skips __missing__() calls. */
10711073
/*[clinic input]
1074+
@critical_section
10721075
OrderedDict.pop
10731076
10741077
key: object
@@ -1083,7 +1086,7 @@ raise a KeyError.
10831086
static PyObject *
10841087
OrderedDict_pop_impl(PyODictObject *self, PyObject *key,
10851088
PyObject *default_value)
1086-
/*[clinic end generated code: output=7a6447d104e7494b input=7efe36601007dff7]*/
1089+
/*[clinic end generated code: output=7a6447d104e7494b input=a79988887b4a651f]*/
10871090
{
10881091
Py_hash_t hash = PyObject_Hash(key);
10891092
if (hash == -1)
@@ -1095,6 +1098,7 @@ OrderedDict_pop_impl(PyODictObject *self, PyObject *key,
10951098
/* popitem() */
10961099

10971100
/*[clinic input]
1101+
@critical_section
10981102
OrderedDict.popitem
10991103
11001104
last: bool = True
@@ -1106,7 +1110,7 @@ Pairs are returned in LIFO order if last is true or FIFO order if false.
11061110

11071111
static PyObject *
11081112
OrderedDict_popitem_impl(PyODictObject *self, int last)
1109-
/*[clinic end generated code: output=98e7d986690d49eb input=d992ac5ee8305e1a]*/
1113+
/*[clinic end generated code: output=98e7d986690d49eb input=8aafc7433e0a40e7]*/
11101114
{
11111115
PyObject *key, *value, *item = NULL;
11121116
_ODictNode *node;
@@ -1251,6 +1255,7 @@ odict_reversed(PyODictObject *od, PyObject *Py_UNUSED(ignored))
12511255
/* move_to_end() */
12521256

12531257
/*[clinic input]
1258+
@critical_section
12541259
OrderedDict.move_to_end
12551260
12561261
key: object
@@ -1263,7 +1268,7 @@ Raise KeyError if the element does not exist.
12631268

12641269
static PyObject *
12651270
OrderedDict_move_to_end_impl(PyODictObject *self, PyObject *key, int last)
1266-
/*[clinic end generated code: output=fafa4c5cc9b92f20 input=d6ceff7132a2fcd7]*/
1271+
/*[clinic end generated code: output=fafa4c5cc9b92f20 input=09f8bc7053c0f6d4]*/
12671272
{
12681273
_ODictNode *node;
12691274

@@ -1556,7 +1561,10 @@ static int
15561561
_PyODict_SetItem_KnownHash(PyObject *od, PyObject *key, PyObject *value,
15571562
Py_hash_t hash)
15581563
{
1559-
int res = _PyDict_SetItem_KnownHash(od, key, value, hash);
1564+
int res;
1565+
Py_BEGIN_CRITICAL_SECTION(od);
1566+
1567+
res = _PyDict_SetItem_KnownHash_LockHeld(od, key, value, hash);
15601568
if (res == 0) {
15611569
res = _odict_add_new_node((PyODictObject *)od, key, hash);
15621570
if (res < 0) {
@@ -1566,11 +1574,13 @@ _PyODict_SetItem_KnownHash(PyObject *od, PyObject *key, PyObject *value,
15661574
_PyErr_ChainExceptions1(exc);
15671575
}
15681576
}
1577+
Py_END_CRITICAL_SECTION();
15691578
return res;
15701579
}
15711580

1572-
int
1573-
PyODict_SetItem(PyObject *od, PyObject *key, PyObject *value)
1581+
1582+
static int
1583+
setitem_lock_held(PyObject *od, PyObject *key, PyObject *value)
15741584
{
15751585
Py_hash_t hash = PyObject_Hash(key);
15761586
if (hash == -1)
@@ -1579,7 +1589,17 @@ PyODict_SetItem(PyObject *od, PyObject *key, PyObject *value)
15791589
}
15801590

15811591
int
1582-
PyODict_DelItem(PyObject *od, PyObject *key)
1592+
PyODict_SetItem(PyObject *od, PyObject *key, PyObject *value)
1593+
{
1594+
int res;
1595+
Py_BEGIN_CRITICAL_SECTION(od);
1596+
res = setitem_lock_held(od, key, value);
1597+
Py_END_CRITICAL_SECTION();
1598+
return res;
1599+
}
1600+
1601+
static int
1602+
del_item_lock_held(PyObject *od, PyObject *key)
15831603
{
15841604
int res;
15851605
Py_hash_t hash = PyObject_Hash(key);
@@ -1591,6 +1611,16 @@ PyODict_DelItem(PyObject *od, PyObject *key)
15911611
return _PyDict_DelItem_KnownHash(od, key, hash);
15921612
}
15931613

1614+
int
1615+
PyODict_DelItem(PyObject *od, PyObject *key)
1616+
{
1617+
int res;
1618+
Py_BEGIN_CRITICAL_SECTION(od);
1619+
res = del_item_lock_held(od, key);
1620+
Py_END_CRITICAL_SECTION();
1621+
return res;
1622+
}
1623+
15941624

15951625
/* -------------------------------------------
15961626
* The OrderedDict views (keys/values/items)
@@ -1630,17 +1660,12 @@ odictiter_traverse(odictiterobject *di, visitproc visit, void *arg)
16301660
/* In order to protect against modifications during iteration, we track
16311661
* the current key instead of the current node. */
16321662
static PyObject *
1633-
odictiter_nextkey(odictiterobject *di)
1663+
odictiter_nextkey_lock_held(odictiterobject *di)
16341664
{
16351665
PyObject *key = NULL;
16361666
_ODictNode *node;
16371667
int reversed = di->kind & _odict_ITER_REVERSED;
16381668

1639-
if (di->di_odict == NULL)
1640-
return NULL;
1641-
if (di->di_current == NULL)
1642-
goto done; /* We're already done. */
1643-
16441669
/* Check for unsupported changes. */
16451670
if (di->di_odict->od_state != di->di_state) {
16461671
PyErr_SetString(PyExc_RuntimeError,
@@ -1682,6 +1707,24 @@ odictiter_nextkey(odictiterobject *di)
16821707
return key;
16831708
}
16841709

1710+
static PyObject *
1711+
odictiter_nextkey(odictiterobject *di)
1712+
{
1713+
if (di->di_odict == NULL)
1714+
return NULL;
1715+
if (di->di_current == NULL) {
1716+
Py_CLEAR(di->di_odict);
1717+
return NULL;
1718+
}
1719+
1720+
1721+
PyObject *res;
1722+
Py_BEGIN_CRITICAL_SECTION(di->di_odict);
1723+
res = odictiter_nextkey_lock_held(di);
1724+
Py_END_CRITICAL_SECTION();
1725+
return res;
1726+
}
1727+
16851728
static PyObject *
16861729
odictiter_iternext(odictiterobject *di)
16871730
{

0 commit comments

Comments
 (0)