diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 25bb224921aa8b..afd468735b4893 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -30,6 +30,10 @@ PyAPI_FUNC(int) _PyDict_SetItem_KnownHash(PyObject *mp, PyObject *key, // Export for '_asyncio' shared extension PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key, Py_hash_t hash); + +extern int _PyDict_DelItem_KnownHash_LockHeld(PyObject *mp, PyObject *key, + Py_hash_t hash); + extern int _PyDict_Contains_KnownHash(PyObject *, PyObject *, Py_hash_t); // "Id" variants @@ -47,6 +51,8 @@ extern int _PyDict_HasOnlyStringKeys(PyObject *mp); // Export for '_ctypes' shared extension PyAPI_FUNC(Py_ssize_t) _PyDict_SizeOf(PyDictObject *); +extern Py_ssize_t _PyDict_SizeOf_LockHeld(PyDictObject *); + #define _PyDict_HasSplitTable(d) ((d)->ma_values != NULL) /* Like PyDict_Merge, but override can be 0, 1 or 2. If override is 0, diff --git a/Misc/NEWS.d/next/Library/2025-05-10-17-42-03.gh-issue-125996.vaQp0-.rst b/Misc/NEWS.d/next/Library/2025-05-10-17-42-03.gh-issue-125996.vaQp0-.rst new file mode 100644 index 00000000000000..4bbfaa3a5e2d85 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-10-17-42-03.gh-issue-125996.vaQp0-.rst @@ -0,0 +1 @@ +Fix thread safety of :class:`collections.OrderedDict`. Patch by Kumar Aditya. diff --git a/Objects/clinic/odictobject.c.h b/Objects/clinic/odictobject.c.h index e71c29a1b268d0..894e9be91bbdce 100644 --- a/Objects/clinic/odictobject.c.h +++ b/Objects/clinic/odictobject.c.h @@ -6,6 +6,7 @@ preserve # include "pycore_gc.h" // PyGC_Head # include "pycore_runtime.h" // _Py_ID() #endif +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_UnpackKeywords() PyDoc_STRVAR(OrderedDict_fromkeys__doc__, @@ -73,6 +74,53 @@ OrderedDict_fromkeys(PyObject *type, PyObject *const *args, Py_ssize_t nargs, Py return return_value; } +PyDoc_STRVAR(OrderedDict___sizeof____doc__, +"__sizeof__($self, /)\n" +"--\n" +"\n"); + +#define ORDEREDDICT___SIZEOF___METHODDEF \ + {"__sizeof__", (PyCFunction)OrderedDict___sizeof__, METH_NOARGS, OrderedDict___sizeof____doc__}, + +static Py_ssize_t +OrderedDict___sizeof___impl(PyODictObject *self); + +static PyObject * +OrderedDict___sizeof__(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + Py_ssize_t _return_value; + + Py_BEGIN_CRITICAL_SECTION(self); + _return_value = OrderedDict___sizeof___impl((PyODictObject *)self); + Py_END_CRITICAL_SECTION(); + if ((_return_value == -1) && PyErr_Occurred()) { + goto exit; + } + return_value = PyLong_FromSsize_t(_return_value); + +exit: + return return_value; +} + +PyDoc_STRVAR(OrderedDict___reduce____doc__, +"__reduce__($self, /)\n" +"--\n" +"\n" +"Return state information for pickling"); + +#define ORDEREDDICT___REDUCE___METHODDEF \ + {"__reduce__", (PyCFunction)OrderedDict___reduce__, METH_NOARGS, OrderedDict___reduce____doc__}, + +static PyObject * +OrderedDict___reduce___impl(PyODictObject *od); + +static PyObject * +OrderedDict___reduce__(PyObject *od, PyObject *Py_UNUSED(ignored)) +{ + return OrderedDict___reduce___impl((PyODictObject *)od); +} + PyDoc_STRVAR(OrderedDict_setdefault__doc__, "setdefault($self, /, key, default=None)\n" "--\n" @@ -135,7 +183,9 @@ OrderedDict_setdefault(PyObject *self, PyObject *const *args, Py_ssize_t nargs, } default_value = args[1]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = OrderedDict_setdefault_impl((PyODictObject *)self, key, default_value); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -204,7 +254,9 @@ OrderedDict_pop(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObjec } default_value = args[1]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = OrderedDict_pop_impl((PyODictObject *)self, key, default_value); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -272,12 +324,62 @@ OrderedDict_popitem(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyO goto exit; } skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = OrderedDict_popitem_impl((PyODictObject *)self, last); + Py_END_CRITICAL_SECTION(); exit: return return_value; } +PyDoc_STRVAR(OrderedDict_clear__doc__, +"clear($self, /)\n" +"--\n" +"\n" +"Remove all items from ordered dict."); + +#define ORDEREDDICT_CLEAR_METHODDEF \ + {"clear", (PyCFunction)OrderedDict_clear, METH_NOARGS, OrderedDict_clear__doc__}, + +static PyObject * +OrderedDict_clear_impl(PyODictObject *self); + +static PyObject * +OrderedDict_clear(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = OrderedDict_clear_impl((PyODictObject *)self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +PyDoc_STRVAR(OrderedDict_copy__doc__, +"copy($self, /)\n" +"--\n" +"\n" +"A shallow copy of ordered dict."); + +#define ORDEREDDICT_COPY_METHODDEF \ + {"copy", (PyCFunction)OrderedDict_copy, METH_NOARGS, OrderedDict_copy__doc__}, + +static PyObject * +OrderedDict_copy_impl(PyObject *od); + +static PyObject * +OrderedDict_copy(PyObject *od, PyObject *Py_UNUSED(ignored)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(od); + return_value = OrderedDict_copy_impl(od); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(OrderedDict_move_to_end__doc__, "move_to_end($self, /, key, last=True)\n" "--\n" @@ -342,9 +444,11 @@ OrderedDict_move_to_end(PyObject *self, PyObject *const *args, Py_ssize_t nargs, goto exit; } skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = OrderedDict_move_to_end_impl((PyODictObject *)self, key, last); + Py_END_CRITICAL_SECTION(); exit: return return_value; } -/*[clinic end generated code: output=7d8206823bb1f419 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=7bc997ca7900f06f input=a9049054013a1b77]*/ diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 6b7b150f0e2f2c..c4f4e430b00a58 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2770,8 +2770,8 @@ PyDict_DelItem(PyObject *op, PyObject *key) return _PyDict_DelItem_KnownHash(op, key, hash); } -static int -delitem_knownhash_lock_held(PyObject *op, PyObject *key, Py_hash_t hash) +int +_PyDict_DelItem_KnownHash_LockHeld(PyObject *op, PyObject *key, Py_hash_t hash) { Py_ssize_t ix; PyDictObject *mp; @@ -2806,7 +2806,7 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash) { int res; Py_BEGIN_CRITICAL_SECTION(op); - res = delitem_knownhash_lock_held(op, key, hash); + res = _PyDict_DelItem_KnownHash_LockHeld(op, key, hash); Py_END_CRITICAL_SECTION(); return res; } @@ -4653,9 +4653,11 @@ dict_tp_clear(PyObject *op) static PyObject *dictiter_new(PyDictObject *, PyTypeObject *); -static Py_ssize_t -sizeof_lock_held(PyDictObject *mp) +Py_ssize_t +_PyDict_SizeOf_LockHeld(PyDictObject *mp) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(mp); + size_t res = _PyObject_SIZE(Py_TYPE(mp)); if (_PyDict_HasSplitTable(mp)) { res += shared_keys_usable_size(mp->ma_keys) * sizeof(PyObject*); @@ -4674,7 +4676,7 @@ _PyDict_SizeOf(PyDictObject *mp) { Py_ssize_t res; Py_BEGIN_CRITICAL_SECTION(mp); - res = sizeof_lock_held(mp); + res = _PyDict_SizeOf_LockHeld(mp); Py_END_CRITICAL_SECTION(); return res; @@ -6864,7 +6866,7 @@ _PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value) dict_unhashable_type(name); return -1; } - return delitem_knownhash_lock_held((PyObject *)dict, name, hash); + return _PyDict_DelItem_KnownHash_LockHeld((PyObject *)dict, name, hash); } else { return setitem_lock_held(dict, name, value); } diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 02fcbbaa0d4536..ddab1b54d1e5c9 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -536,6 +536,7 @@ struct _odictnode { static Py_ssize_t _odict_get_index_raw(PyODictObject *od, PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); PyObject *value = NULL; PyDictKeysObject *keys = ((PyDictObject *)od)->ma_keys; Py_ssize_t ix; @@ -560,6 +561,7 @@ _odict_get_index_raw(PyODictObject *od, PyObject *key, Py_hash_t hash) static int _odict_resize(PyODictObject *od) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_ssize_t size, i; _ODictNode **fast_nodes, *node; @@ -596,6 +598,7 @@ _odict_resize(PyODictObject *od) static Py_ssize_t _odict_get_index(PyODictObject *od, PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); PyDictKeysObject *keys; assert(key != NULL); @@ -616,6 +619,7 @@ _odict_get_index(PyODictObject *od, PyObject *key, Py_hash_t hash) static _ODictNode * _odict_find_node_hash(PyODictObject *od, PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_ssize_t index; if (_odict_EMPTY(od)) @@ -630,6 +634,7 @@ _odict_find_node_hash(PyODictObject *od, PyObject *key, Py_hash_t hash) static _ODictNode * _odict_find_node(PyODictObject *od, PyObject *key) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_ssize_t index; Py_hash_t hash; @@ -648,6 +653,7 @@ _odict_find_node(PyODictObject *od, PyObject *key) static void _odict_add_head(PyODictObject *od, _ODictNode *node) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); _odictnode_PREV(node) = NULL; _odictnode_NEXT(node) = _odict_FIRST(od); if (_odict_FIRST(od) == NULL) @@ -661,6 +667,7 @@ _odict_add_head(PyODictObject *od, _ODictNode *node) static void _odict_add_tail(PyODictObject *od, _ODictNode *node) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); _odictnode_PREV(node) = _odict_LAST(od); _odictnode_NEXT(node) = NULL; if (_odict_LAST(od) == NULL) @@ -675,6 +682,7 @@ _odict_add_tail(PyODictObject *od, _ODictNode *node) static int _odict_add_new_node(PyODictObject *od, PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_ssize_t i; _ODictNode *node; @@ -719,6 +727,7 @@ _odict_add_new_node(PyODictObject *od, PyObject *key, Py_hash_t hash) static void _odict_remove_node(PyODictObject *od, _ODictNode *node) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); if (_odict_FIRST(od) == node) _odict_FIRST(od) = _odictnode_NEXT(node); else if (_odictnode_PREV(node) != NULL) @@ -754,6 +763,7 @@ static int _odict_clear_node(PyODictObject *od, _ODictNode *node, PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_ssize_t i; assert(key != NULL); @@ -951,31 +961,34 @@ OrderedDict_fromkeys_impl(PyTypeObject *type, PyObject *seq, PyObject *value) return _PyDict_FromKeys((PyObject *)type, seq, value); } -/* __sizeof__() */ - -/* OrderedDict.__sizeof__() does not have a docstring. */ -PyDoc_STRVAR(odict_sizeof__doc__, ""); +/*[clinic input] +@critical_section +OrderedDict.__sizeof__ -> Py_ssize_t +[clinic start generated code]*/ -static PyObject * -odict_sizeof(PyObject *op, PyObject *Py_UNUSED(ignored)) +static Py_ssize_t +OrderedDict___sizeof___impl(PyODictObject *self) +/*[clinic end generated code: output=1a8560db8cf83ac5 input=655e989ae24daa6a]*/ { - PyODictObject *od = _PyODictObject_CAST(op); - Py_ssize_t res = _PyDict_SizeOf((PyDictObject *)od); - res += sizeof(_ODictNode *) * od->od_fast_nodes_size; /* od_fast_nodes */ - if (!_odict_EMPTY(od)) { - res += sizeof(_ODictNode) * PyODict_SIZE(od); /* linked-list */ + Py_ssize_t res = _PyDict_SizeOf_LockHeld((PyDictObject *)self); + res += sizeof(_ODictNode *) * self->od_fast_nodes_size; /* od_fast_nodes */ + if (!_odict_EMPTY(self)) { + res += sizeof(_ODictNode) * PyODict_SIZE(self); /* linked-list */ } - return PyLong_FromSsize_t(res); + return res; } -/* __reduce__() */ +/*[clinic input] +OrderedDict.__reduce__ + self as od: self(type="PyODictObject *") -PyDoc_STRVAR(odict_reduce__doc__, "Return state information for pickling"); +Return state information for pickling +[clinic start generated code]*/ static PyObject * -odict_reduce(PyObject *op, PyObject *Py_UNUSED(ignored)) +OrderedDict___reduce___impl(PyODictObject *od) +/*[clinic end generated code: output=71eeb81f760a6a8e input=b0467c7ec400fe5e]*/ { - register PyODictObject *od = _PyODictObject_CAST(op); PyObject *state, *result = NULL; PyObject *items_iter, *items, *args = NULL; @@ -1010,8 +1023,10 @@ odict_reduce(PyObject *op, PyObject *Py_UNUSED(ignored)) /* setdefault(): Skips __missing__() calls. */ +static int PyODict_SetItem_LockHeld(PyObject *self, PyObject *key, PyObject *value); /*[clinic input] +@critical_section OrderedDict.setdefault key: object @@ -1025,7 +1040,7 @@ Return the value for key if key is in the dictionary, else default. static PyObject * OrderedDict_setdefault_impl(PyODictObject *self, PyObject *key, PyObject *default_value) -/*[clinic end generated code: output=97537cb7c28464b6 input=38e098381c1efbc6]*/ +/*[clinic end generated code: output=97537cb7c28464b6 input=d7b93e92734f99b5]*/ { PyObject *result = NULL; @@ -1035,7 +1050,7 @@ OrderedDict_setdefault_impl(PyODictObject *self, PyObject *key, if (PyErr_Occurred()) return NULL; assert(_odict_find_node(self, key) == NULL); - if (PyODict_SetItem((PyObject *)self, key, default_value) >= 0) { + if (PyODict_SetItem_LockHeld((PyObject *)self, key, default_value) >= 0) { result = Py_NewRef(default_value); } } @@ -1065,10 +1080,9 @@ static PyObject * _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj, Py_hash_t hash) { - PyObject *value = NULL; - - Py_BEGIN_CRITICAL_SECTION(od); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); + PyObject *value = NULL; _ODictNode *node = _odict_find_node_hash(_PyODictObject_CAST(od), key, hash); if (node != NULL) { /* Pop the node first to avoid a possible dict resize (due to @@ -1093,7 +1107,6 @@ _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj, PyErr_SetObject(PyExc_KeyError, key); } } - Py_END_CRITICAL_SECTION(); done: return value; @@ -1101,6 +1114,7 @@ _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj, /* Skips __missing__() calls. */ /*[clinic input] +@critical_section OrderedDict.pop key: object @@ -1115,7 +1129,7 @@ raise a KeyError. static PyObject * OrderedDict_pop_impl(PyODictObject *self, PyObject *key, PyObject *default_value) -/*[clinic end generated code: output=7a6447d104e7494b input=7efe36601007dff7]*/ +/*[clinic end generated code: output=7a6447d104e7494b input=a79988887b4a651f]*/ { Py_hash_t hash = PyObject_Hash(key); if (hash == -1) @@ -1127,6 +1141,7 @@ OrderedDict_pop_impl(PyODictObject *self, PyObject *key, /* popitem() */ /*[clinic input] +@critical_section OrderedDict.popitem last: bool = True @@ -1138,7 +1153,7 @@ Pairs are returned in LIFO order if last is true or FIFO order if false. static PyObject * OrderedDict_popitem_impl(PyODictObject *self, int last) -/*[clinic end generated code: output=98e7d986690d49eb input=d992ac5ee8305e1a]*/ +/*[clinic end generated code: output=98e7d986690d49eb input=8aafc7433e0a40e7]*/ { PyObject *key, *value, *item = NULL; _ODictNode *node; @@ -1167,6 +1182,9 @@ OrderedDict_popitem_impl(PyODictObject *self, int last) PyDoc_STRVAR(odict_keys__doc__, ""); static PyObject * odictkeys_new(PyObject *od, PyObject *Py_UNUSED(ignored)); /* forward */ +static int +_PyODict_SetItem_KnownHash_LockHeld(PyObject *od, PyObject *key, PyObject *value, + Py_hash_t hash); /* forward */ /* values() */ @@ -1192,32 +1210,36 @@ static PyObject * mutablemapping_update(PyObject *, PyObject *, PyObject *); #define odict_update mutablemapping_update -/* clear() */ +/*[clinic input] +@critical_section +OrderedDict.clear -PyDoc_STRVAR(odict_clear__doc__, - "od.clear() -> None. Remove all items from od."); +Remove all items from ordered dict. +[clinic start generated code]*/ static PyObject * -odict_clear(PyObject *op, PyObject *Py_UNUSED(ignored)) +OrderedDict_clear_impl(PyODictObject *self) +/*[clinic end generated code: output=a1a76d1322f556c5 input=08b12322e74c535c]*/ { - register PyODictObject *od = _PyODictObject_CAST(op); - PyDict_Clear(op); - _odict_clear_nodes(od); + _PyDict_Clear_LockHeld((PyObject *)self); + _odict_clear_nodes(self); Py_RETURN_NONE; } /* copy() */ -/* forward */ -static int _PyODict_SetItem_KnownHash(PyObject *, PyObject *, PyObject *, - Py_hash_t); +/*[clinic input] +@critical_section +OrderedDict.copy + self as od: self -PyDoc_STRVAR(odict_copy__doc__, "od.copy() -> a shallow copy of od"); +A shallow copy of ordered dict. +[clinic start generated code]*/ static PyObject * -odict_copy(PyObject *op, PyObject *Py_UNUSED(ignored)) +OrderedDict_copy_impl(PyObject *od) +/*[clinic end generated code: output=9cdbe7394aecc576 input=e329951ae617ed48]*/ { - register PyODictObject *od = _PyODictObject_CAST(op); _ODictNode *node; PyObject *od_copy; @@ -1237,8 +1259,8 @@ odict_copy(PyObject *op, PyObject *Py_UNUSED(ignored)) PyErr_SetObject(PyExc_KeyError, key); goto fail; } - if (_PyODict_SetItem_KnownHash((PyObject *)od_copy, key, value, - _odictnode_HASH(node)) != 0) + if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value, + _odictnode_HASH(node)) != 0) goto fail; } } @@ -1286,6 +1308,7 @@ odict_reversed(PyObject *op, PyObject *Py_UNUSED(ignored)) /* move_to_end() */ /*[clinic input] +@critical_section OrderedDict.move_to_end key: object @@ -1298,7 +1321,7 @@ Raise KeyError if the element does not exist. static PyObject * OrderedDict_move_to_end_impl(PyODictObject *self, PyObject *key, int last) -/*[clinic end generated code: output=fafa4c5cc9b92f20 input=d6ceff7132a2fcd7]*/ +/*[clinic end generated code: output=fafa4c5cc9b92f20 input=09f8bc7053c0f6d4]*/ { _ODictNode *node; @@ -1339,10 +1362,8 @@ static PyMethodDef odict_methods[] = { /* overridden dict methods */ ORDEREDDICT_FROMKEYS_METHODDEF - {"__sizeof__", odict_sizeof, METH_NOARGS, - odict_sizeof__doc__}, - {"__reduce__", odict_reduce, METH_NOARGS, - odict_reduce__doc__}, + ORDEREDDICT___SIZEOF___METHODDEF + ORDEREDDICT___REDUCE___METHODDEF ORDEREDDICT_SETDEFAULT_METHODDEF ORDEREDDICT_POP_METHODDEF ORDEREDDICT_POPITEM_METHODDEF @@ -1354,11 +1375,8 @@ static PyMethodDef odict_methods[] = { odict_items__doc__}, {"update", _PyCFunction_CAST(odict_update), METH_VARARGS | METH_KEYWORDS, odict_update__doc__}, - {"clear", odict_clear, METH_NOARGS, - odict_clear__doc__}, - {"copy", odict_copy, METH_NOARGS, - odict_copy__doc__}, - + ORDEREDDICT_CLEAR_METHODDEF + ORDEREDDICT_COPY_METHODDEF /* new methods */ {"__reversed__", odict_reversed, METH_NOARGS, odict_reversed__doc__}, @@ -1457,7 +1475,8 @@ odict_tp_clear(PyObject *op) { PyODictObject *od = _PyODictObject_CAST(op); Py_CLEAR(od->od_inst_dict); - PyDict_Clear((PyObject *)od); + // cannot use lock held variant as critical section is not held here + PyDict_Clear(op); _odict_clear_nodes(od); return 0; } @@ -1465,7 +1484,7 @@ odict_tp_clear(PyObject *op) /* tp_richcompare */ static PyObject * -odict_richcompare(PyObject *v, PyObject *w, int op) +odict_richcompare_lock_held(PyObject *v, PyObject *w, int op) { if (!PyODict_Check(v) || !PyDict_Check(w)) { Py_RETURN_NOTIMPLEMENTED; @@ -1498,6 +1517,16 @@ odict_richcompare(PyObject *v, PyObject *w, int op) } } +static PyObject * +odict_richcompare(PyObject *v, PyObject *w, int op) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION2(v, w); + res = odict_richcompare_lock_held(v, w, op); + Py_END_CRITICAL_SECTION2(); + return res; +} + /* tp_iter */ static PyObject * @@ -1588,10 +1617,11 @@ PyODict_New(void) } static int -_PyODict_SetItem_KnownHash(PyObject *od, PyObject *key, PyObject *value, - Py_hash_t hash) +_PyODict_SetItem_KnownHash_LockHeld(PyObject *od, PyObject *key, PyObject *value, + Py_hash_t hash) { - int res = _PyDict_SetItem_KnownHash(od, key, value, hash); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); + int res = _PyDict_SetItem_KnownHash_LockHeld((PyDictObject *)od, key, value, hash); if (res == 0) { res = _odict_add_new_node(_PyODictObject_CAST(od), key, hash); if (res < 0) { @@ -1604,18 +1634,32 @@ _PyODict_SetItem_KnownHash(PyObject *od, PyObject *key, PyObject *value, return res; } -int -PyODict_SetItem(PyObject *od, PyObject *key, PyObject *value) + +static int +PyODict_SetItem_LockHeld(PyObject *od, PyObject *key, PyObject *value) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); Py_hash_t hash = PyObject_Hash(key); - if (hash == -1) + if (hash == -1) { return -1; - return _PyODict_SetItem_KnownHash(od, key, value, hash); + } + return _PyODict_SetItem_KnownHash_LockHeld(od, key, value, hash); } int -PyODict_DelItem(PyObject *od, PyObject *key) +PyODict_SetItem(PyObject *od, PyObject *key, PyObject *value) +{ + int res; + Py_BEGIN_CRITICAL_SECTION(od); + res = PyODict_SetItem_LockHeld(od, key, value); + Py_END_CRITICAL_SECTION(); + return res; +} + +int +PyODict_DelItem_LockHeld(PyObject *od, PyObject *key) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(od); int res; Py_hash_t hash = PyObject_Hash(key); if (hash == -1) @@ -1623,9 +1667,18 @@ PyODict_DelItem(PyObject *od, PyObject *key) res = _odict_clear_node(_PyODictObject_CAST(od), NULL, key, hash); if (res < 0) return -1; - return _PyDict_DelItem_KnownHash(od, key, hash); + return _PyDict_DelItem_KnownHash_LockHeld(od, key, hash); } +int +PyODict_DelItem(PyObject *od, PyObject *key) +{ + int res; + Py_BEGIN_CRITICAL_SECTION(od); + res = PyODict_DelItem_LockHeld(od, key); + Py_END_CRITICAL_SECTION(); + return res; +} /* ------------------------------------------- * The OrderedDict views (keys/values/items) @@ -1667,14 +1720,14 @@ odictiter_traverse(PyObject *op, visitproc visit, void *arg) /* In order to protect against modifications during iteration, we track * the current key instead of the current node. */ static PyObject * -odictiter_nextkey(odictiterobject *di) +odictiter_nextkey_lock_held(odictiterobject *di) { + assert(di->di_odict != NULL); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(di->di_odict); PyObject *key = NULL; _ODictNode *node; int reversed = di->kind & _odict_ITER_REVERSED; - if (di->di_odict == NULL) - return NULL; if (di->di_current == NULL) goto done; /* We're already done. */ @@ -1719,8 +1772,23 @@ odictiter_nextkey(odictiterobject *di) return key; } + +static PyObject * +odictiter_nextkey(odictiterobject *di) +{ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(di); + if (di->di_odict == NULL) { + return NULL; + } + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(di->di_odict); + res = odictiter_nextkey_lock_held(di); + Py_END_CRITICAL_SECTION(); + return res; +} + static PyObject * -odictiter_iternext(PyObject *op) +odictiter_iternext_lock_held(PyObject *op) { odictiterobject *di = (odictiterobject*)op; PyObject *result, *value; @@ -1734,14 +1802,12 @@ odictiter_iternext(PyObject *op) return key; } - value = PyODict_GetItem((PyObject *)di->di_odict, key); /* borrowed */ - if (value == NULL) { + if (PyDict_GetItemRef((PyObject *)di->di_odict, key, &value) != 1) { if (!PyErr_Occurred()) PyErr_SetObject(PyExc_KeyError, key); Py_DECREF(key); goto done; } - Py_INCREF(value); /* Handle the values case. */ if (!(di->kind & _odict_ITER_KEYS)) { @@ -1752,7 +1818,7 @@ odictiter_iternext(PyObject *op) /* Handle the items case. */ result = di->di_result; - if (Py_REFCNT(result) == 1) { + if (_PyObject_IsUniquelyReferenced(result) == 1) { /* not in use so we can reuse it * (the common case during iteration) */ Py_INCREF(result); @@ -1781,6 +1847,17 @@ odictiter_iternext(PyObject *op) return NULL; } +static PyObject * +odictiter_iternext(PyObject *op) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(op); + res = odictiter_iternext_lock_held(op); + Py_END_CRITICAL_SECTION(); + return res; +} + + /* No need for tp_clear because odictiterobject is not mutable. */ PyDoc_STRVAR(reduce_doc, "Return state information for pickling");