From de9044036bc6408e50b0f7b5466c77b0f90f04c7 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 21 Dec 2023 19:49:10 -0800 Subject: [PATCH 01/51] Convert most collections.deque method use clinic Convert most methods on collections.deque to use clinic. This will allow us to use clinic's `@critical_section` directive when making deques thread-safe for `--gil-disabled`, simplifying the implementation. --- .../pycore_global_objects_fini_generated.h | 1 + Include/internal/pycore_global_strings.h | 1 + .../internal/pycore_runtime_init_generated.h | 1 + .../internal/pycore_unicodeobject_generated.h | 3 + Modules/_collectionsmodule.c | 462 +++++++++++------- Modules/clinic/_collectionsmodule.c.h | 420 +++++++++++++++- 6 files changed, 712 insertions(+), 176 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 89ec8cbbbcd649..affed52d989344 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -1043,6 +1043,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(max_length)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxdigits)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxevents)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxlen)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxmem)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxsplit)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(maxvalue)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 62c3ee3ae2a0bd..8fc0f154737ba0 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -532,6 +532,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(max_length) STRUCT_FOR_ID(maxdigits) STRUCT_FOR_ID(maxevents) + STRUCT_FOR_ID(maxlen) STRUCT_FOR_ID(maxmem) STRUCT_FOR_ID(maxsplit) STRUCT_FOR_ID(maxvalue) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 1defa39f816e78..1b596c874e370e 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -1041,6 +1041,7 @@ extern "C" { INIT_ID(max_length), \ INIT_ID(maxdigits), \ INIT_ID(maxevents), \ + INIT_ID(maxlen), \ INIT_ID(maxmem), \ INIT_ID(maxsplit), \ INIT_ID(maxvalue), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index be9baa3eebecfc..519e8d23d69840 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -1437,6 +1437,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { string = &_Py_ID(maxevents); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); + string = &_Py_ID(maxlen); + assert(_PyUnicode_CheckConsistency(string, 1)); + _PyUnicode_InternInPlace(interp, &string); string = &_Py_ID(maxmem); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index c8cd53de5e2262..50f1091f93b036 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -44,8 +44,11 @@ find_module_state_by_def(PyTypeObject *type) /*[clinic input] module _collections class _tuplegetter "_tuplegetterobject *" "clinic_state()->tuplegetter_type" +class _collections.deque "dequeobject *" "clinic_state()->deque_type" [clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=7356042a89862e0e]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=a033cc2a8476b3f1]*/ + +typedef struct dequeobject dequeobject; /* We can safely assume type to be the defining class, * since tuplegetter is not a base type */ @@ -53,6 +56,12 @@ class _tuplegetter "_tuplegetterobject *" "clinic_state()->tuplegetter_type" #include "clinic/_collectionsmodule.c.h" #undef clinic_state +/*[python input] +class dequeobject_converter(self_converter): + type = "dequeobject *" +[python start generated code]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=b6ae4a3ff852be2f]*/ + /* collections module implementation of a deque() datatype Written and maintained by Raymond D. Hettinger */ @@ -121,7 +130,7 @@ typedef struct BLOCK { struct BLOCK *rightlink; } block; -typedef struct { +struct dequeobject { PyObject_VAR_HEAD block *leftblock; block *rightblock; @@ -132,7 +141,7 @@ typedef struct { Py_ssize_t numfreeblocks; block *freeblocks[MAXFREEBLOCKS]; PyObject *weakreflist; -} dequeobject; +}; /* For debug builds, add error checking to track the endpoints * in the chain of links. The goal is to make sure that link @@ -219,8 +228,17 @@ deque_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return (PyObject *)deque; } +/*[clinic input] +_collections.deque.pop + + deque: dequeobject + +Remove and return the rightmost element. +[clinic start generated code]*/ + static PyObject * -deque_pop(dequeobject *deque, PyObject *unused) +_collections_deque_pop_impl(dequeobject *deque) +/*[clinic end generated code: output=2d4ef1dcd5113ae6 input=b4873fc20283d8d6]*/ { PyObject *item; block *prevblock; @@ -254,10 +272,17 @@ deque_pop(dequeobject *deque, PyObject *unused) return item; } -PyDoc_STRVAR(pop_doc, "Remove and return the rightmost element."); +/*[clinic input] +_collections.deque.popleft + + deque: dequeobject + +Remove and return the rightmost element. +[clinic start generated code]*/ static PyObject * -deque_popleft(dequeobject *deque, PyObject *unused) +_collections_deque_popleft_impl(dequeobject *deque) +/*[clinic end generated code: output=8cd77178b5116aba input=39d64df4664392d3]*/ { PyObject *item; block *prevblock; @@ -292,8 +317,6 @@ deque_popleft(dequeobject *deque, PyObject *unused) return item; } -PyDoc_STRVAR(popleft_doc, "Remove and return the leftmost element."); - /* The deque's size limit is d.maxlen. The limit can be zero or positive. * If there is no limit, then d.maxlen == -1. * @@ -326,7 +349,7 @@ deque_append_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) deque->rightindex++; deque->rightblock->data[deque->rightindex] = item; if (NEEDS_TRIM(deque, maxlen)) { - PyObject *olditem = deque_popleft(deque, NULL); + PyObject *olditem = _collections_deque_popleft_impl(deque); Py_DECREF(olditem); } else { deque->state++; @@ -334,16 +357,25 @@ deque_append_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) return 0; } +/*[clinic input] +_collections.deque.append + + deque: dequeobject + item: object + / + +Add an element to the right side of the deque. +[clinic start generated code]*/ + static PyObject * -deque_append(dequeobject *deque, PyObject *item) +_collections_deque_append(dequeobject *deque, PyObject *item) +/*[clinic end generated code: output=fc44cc7b9dcb0180 input=803e0d976a2e2620]*/ { if (deque_append_internal(deque, Py_NewRef(item), deque->maxlen) < 0) return NULL; Py_RETURN_NONE; } -PyDoc_STRVAR(append_doc, "Add an element to the right side of the deque."); - static inline int deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) { @@ -362,7 +394,7 @@ deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) deque->leftindex--; deque->leftblock->data[deque->leftindex] = item; if (NEEDS_TRIM(deque, deque->maxlen)) { - PyObject *olditem = deque_pop(deque, NULL); + PyObject *olditem = _collections_deque_pop_impl(deque); Py_DECREF(olditem); } else { deque->state++; @@ -370,16 +402,25 @@ deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) return 0; } +/*[clinic input] +_collections.deque.appendleft + + deque: dequeobject + item: object + / + +Add an element to the left side of the deque. +[clinic start generated code]*/ + static PyObject * -deque_appendleft(dequeobject *deque, PyObject *item) +_collections_deque_appendleft(dequeobject *deque, PyObject *item) +/*[clinic end generated code: output=f1b75022fbccf8bb input=481442915f0f6465]*/ { if (deque_appendleft_internal(deque, Py_NewRef(item), deque->maxlen) < 0) return NULL; Py_RETURN_NONE; } -PyDoc_STRVAR(appendleft_doc, "Add an element to the left side of the deque."); - static PyObject* finalize_iterator(PyObject *it) { @@ -410,8 +451,19 @@ consume_iterator(PyObject *it) return finalize_iterator(it); } +/*[clinic input] +_collections.deque.extend + + deque: dequeobject + iterable: object + / + +Extend the right side of the deque with elements from the iterable +[clinic start generated code]*/ + static PyObject * -deque_extend(dequeobject *deque, PyObject *iterable) +_collections_deque_extend(dequeobject *deque, PyObject *iterable) +/*[clinic end generated code: output=a58014bf32cb0b9d input=5a75e68f72ed8f09]*/ { PyObject *it, *item; PyObject *(*iternext)(PyObject *); @@ -423,7 +475,7 @@ deque_extend(dequeobject *deque, PyObject *iterable) PyObject *s = PySequence_List(iterable); if (s == NULL) return NULL; - result = deque_extend(deque, s); + result = _collections_deque_extend(deque, s); Py_DECREF(s); return result; } @@ -454,11 +506,19 @@ deque_extend(dequeobject *deque, PyObject *iterable) return finalize_iterator(it); } -PyDoc_STRVAR(extend_doc, -"Extend the right side of the deque with elements from the iterable"); +/*[clinic input] +_collections.deque.extendleft + + deque: dequeobject + iterable: object + / + +Extend the left side of the deque with elements from the iterable +[clinic start generated code]*/ static PyObject * -deque_extendleft(dequeobject *deque, PyObject *iterable) +_collections_deque_extendleft(dequeobject *deque, PyObject *iterable) +/*[clinic end generated code: output=0a0df3269097f284 input=8dae4c4f9d852a4c]*/ { PyObject *it, *item; PyObject *(*iternext)(PyObject *); @@ -470,7 +530,7 @@ deque_extendleft(dequeobject *deque, PyObject *iterable) PyObject *s = PySequence_List(iterable); if (s == NULL) return NULL; - result = deque_extendleft(deque, s); + result = _collections_deque_extendleft(deque, s); Py_DECREF(s); return result; } @@ -501,15 +561,12 @@ deque_extendleft(dequeobject *deque, PyObject *iterable) return finalize_iterator(it); } -PyDoc_STRVAR(extendleft_doc, -"Extend the left side of the deque with elements from the iterable"); - static PyObject * deque_inplace_concat(dequeobject *deque, PyObject *other) { PyObject *result; - result = deque_extend(deque, other); + result = _collections_deque_extend(deque, other); if (result == NULL) return result; Py_INCREF(deque); @@ -517,8 +574,18 @@ deque_inplace_concat(dequeobject *deque, PyObject *other) return (PyObject *)deque; } + +/*[clinic input] +_collections.deque.copy + + deque: dequeobject + +Return a shallow copy of a deque. +[clinic start generated code]*/ + static PyObject * -deque_copy(PyObject *deque, PyObject *Py_UNUSED(ignored)) +_collections_deque_copy_impl(dequeobject *deque) +/*[clinic end generated code: output=af1e3831be813117 input=f575fc72c00333d4]*/ { PyObject *result; dequeobject *old_deque = (dequeobject *)deque; @@ -535,9 +602,9 @@ deque_copy(PyObject *deque, PyObject *Py_UNUSED(ignored)) /* Fast path for the deque_repeat() common case where len(deque) == 1 */ if (Py_SIZE(deque) == 1) { PyObject *item = old_deque->leftblock->data[old_deque->leftindex]; - rv = deque_append(new_deque, item); + rv = _collections_deque_append(new_deque, item); } else { - rv = deque_extend(new_deque, deque); + rv = _collections_deque_extend(new_deque, (PyObject *) deque); } if (rv != NULL) { Py_DECREF(rv); @@ -547,7 +614,7 @@ deque_copy(PyObject *deque, PyObject *Py_UNUSED(ignored)) return NULL; } if (old_deque->maxlen < 0) - result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)), deque); + result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)), (PyObject *) deque); else result = PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", deque, old_deque->maxlen, NULL); @@ -561,7 +628,18 @@ deque_copy(PyObject *deque, PyObject *Py_UNUSED(ignored)) return result; } -PyDoc_STRVAR(copy_doc, "Return a shallow copy of a deque."); +/*[clinic input] +_collections.deque.__copy__ = _collections.deque.copy + +Return a shallow copy of a deque. +[clinic start generated code]*/ + +static PyObject * +_collections_deque___copy___impl(dequeobject *deque) +/*[clinic end generated code: output=c4c31949334138fd input=9d78c00375929799]*/ +{ + return _collections_deque_copy_impl(deque); +} static PyObject * deque_concat(dequeobject *deque, PyObject *other) @@ -580,10 +658,10 @@ deque_concat(dequeobject *deque, PyObject *other) return NULL; } - new_deque = deque_copy((PyObject *)deque, NULL); + new_deque = _collections_deque_copy_impl(deque); if (new_deque == NULL) return NULL; - result = deque_extend((dequeobject *)new_deque, other); + result = _collections_deque_extend((dequeobject *)new_deque, other); if (result == NULL) { Py_DECREF(new_deque); return NULL; @@ -669,22 +747,29 @@ deque_clear(dequeobject *deque) alternate_method: while (Py_SIZE(deque)) { - item = deque_pop(deque, NULL); + item = _collections_deque_pop_impl(deque); assert (item != NULL); Py_DECREF(item); } return 0; } +/*[clinic input] +_collections.deque.clear + + deque: dequeobject + +Remove all elements from the deque. +[clinic start generated code]*/ + static PyObject * -deque_clearmethod(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +_collections_deque_clear_impl(dequeobject *deque) +/*[clinic end generated code: output=0f0b9d60188bf83b input=9c003117680a7abf]*/ { deque_clear(deque); Py_RETURN_NONE; } -PyDoc_STRVAR(clear_doc, "Remove all elements from the deque."); - static PyObject * deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) { @@ -750,7 +835,7 @@ deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) n = (deque->maxlen + size - 1) / size; for (i = 0 ; i < n-1 ; i++) { - rv = deque_extend(deque, seq); + rv = _collections_deque_extend(deque, seq); if (rv == NULL) { Py_DECREF(seq); return NULL; @@ -768,7 +853,7 @@ deque_repeat(dequeobject *deque, Py_ssize_t n) dequeobject *new_deque; PyObject *rv; - new_deque = (dequeobject *)deque_copy((PyObject *) deque, NULL); + new_deque = (dequeobject *) _collections_deque_copy_impl(deque); if (new_deque == NULL) return NULL; rv = deque_inplace_repeat(new_deque, n); @@ -925,36 +1010,36 @@ _deque_rotate(dequeobject *deque, Py_ssize_t n) return rv; } -static PyObject * -deque_rotate(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) -{ - Py_ssize_t n=1; +/*[clinic input] +_collections.deque.rotate - if (!_PyArg_CheckPositional("deque.rotate", nargs, 0, 1)) { - return NULL; - } - if (nargs) { - PyObject *index = _PyNumber_Index(args[0]); - if (index == NULL) { - return NULL; - } - n = PyLong_AsSsize_t(index); - Py_DECREF(index); - if (n == -1 && PyErr_Occurred()) { - return NULL; - } - } + deque: dequeobject + n: Py_ssize_t = 1 + / +Rotate the deque n steps to the right. If n is negative, rotates left. +[clinic start generated code]*/ + +static PyObject * +_collections_deque_rotate_impl(dequeobject *deque, Py_ssize_t n) +/*[clinic end generated code: output=5a9df290cc0d3adf input=0d7f4900fe866917]*/ +{ if (!_deque_rotate(deque, n)) Py_RETURN_NONE; return NULL; } -PyDoc_STRVAR(rotate_doc, -"Rotate the deque n steps to the right (default n=1). If n is negative, rotates left."); +/*[clinic input] +_collections.deque.reverse + + deque: dequeobject + +Reverse *IN PLACE* +[clinic start generated code]*/ static PyObject * -deque_reverse(dequeobject *deque, PyObject *unused) +_collections_deque_reverse_impl(dequeobject *deque) +/*[clinic end generated code: output=8f859d206158686e input=651e0257414fac22]*/ { block *leftblock = deque->leftblock; block *rightblock = deque->rightblock; @@ -991,11 +1076,19 @@ deque_reverse(dequeobject *deque, PyObject *unused) Py_RETURN_NONE; } -PyDoc_STRVAR(reverse_doc, -"D.reverse() -- reverse *IN PLACE*"); +/*[clinic input] +_collections.deque.count + + deque: dequeobject + v: object + / + +Return number of occurrences of v +[clinic start generated code]*/ static PyObject * -deque_count(dequeobject *deque, PyObject *v) +_collections_deque_count(dequeobject *deque, PyObject *v) +/*[clinic end generated code: output=4fd47b6bf522f071 input=38d3b9f0f9993e26]*/ { block *b = deque->leftblock; Py_ssize_t index = deque->leftindex; @@ -1030,9 +1123,6 @@ deque_count(dequeobject *deque, PyObject *v) return PyLong_FromSsize_t(count); } -PyDoc_STRVAR(count_doc, -"D.count(value) -- return number of occurrences of value"); - static int deque_contains(dequeobject *deque, PyObject *v) { @@ -1071,22 +1161,33 @@ deque_len(dequeobject *deque) return Py_SIZE(deque); } +/*[clinic input] +@text_signature "($self, value, [start, [stop]])" +_collections.deque.index + + deque: dequeobject + v: object + start: object(converter='_PyEval_SliceIndexNotNone', type='Py_ssize_t', c_default='0') = NULL + stop: object(converter='_PyEval_SliceIndexNotNone', type='Py_ssize_t', c_default='Py_SIZE(deque)') = NULL + / + +Return first index of value. + +Raises ValueError if the value is not present. +[clinic start generated code]*/ + static PyObject * -deque_index(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) +_collections_deque_index_impl(dequeobject *deque, PyObject *v, + Py_ssize_t start, Py_ssize_t stop) +/*[clinic end generated code: output=5b2a991d7315b3cf input=b31d3a5c49cb8725]*/ { - Py_ssize_t i, n, start=0, stop=Py_SIZE(deque); - PyObject *v, *item; + Py_ssize_t i, n; + PyObject *item; block *b = deque->leftblock; Py_ssize_t index = deque->leftindex; size_t start_state = deque->state; int cmp; - if (!_PyArg_ParseStack(args, nargs, "O|O&O&:index", &v, - _PyEval_SliceIndexNotNone, &start, - _PyEval_SliceIndexNotNone, &stop)) { - return NULL; - } - if (start < 0) { start += Py_SIZE(deque); if (start < 0) @@ -1138,10 +1239,6 @@ deque_index(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) return NULL; } -PyDoc_STRVAR(index_doc, -"D.index(value, [start, [stop]]) -- return first index of value.\n" -"Raises ValueError if the value is not present."); - /* insert(), remove(), and delitem() are implemented in terms of rotate() for simplicity and reasonable performance near the end points. If for some reason these methods become popular, it is not @@ -1150,32 +1247,39 @@ PyDoc_STRVAR(index_doc, boost (by moving each pointer only once instead of twice). */ +/*[clinic input] +_collections.deque.insert + + deque: dequeobject + index: Py_ssize_t + value: object + / + +Insert value before index +[clinic start generated code]*/ + static PyObject * -deque_insert(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) +_collections_deque_insert_impl(dequeobject *deque, Py_ssize_t index, + PyObject *value) +/*[clinic end generated code: output=f913d56fc97caddf input=0593cc27bffa766a]*/ { - Py_ssize_t index; Py_ssize_t n = Py_SIZE(deque); - PyObject *value; PyObject *rv; - if (!_PyArg_ParseStack(args, nargs, "nO:insert", &index, &value)) { - return NULL; - } - if (deque->maxlen == Py_SIZE(deque)) { PyErr_SetString(PyExc_IndexError, "deque already at its maximum size"); return NULL; } if (index >= n) - return deque_append(deque, value); + return _collections_deque_append(deque, value); if (index <= -n || index == 0) - return deque_appendleft(deque, value); + return _collections_deque_appendleft(deque, value); if (_deque_rotate(deque, -index)) return NULL; if (index < 0) - rv = deque_append(deque, value); + rv = _collections_deque_append(deque, value); else - rv = deque_appendleft(deque, value); + rv = _collections_deque_appendleft(deque, value); if (rv == NULL) return NULL; Py_DECREF(rv); @@ -1184,12 +1288,6 @@ deque_insert(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) Py_RETURN_NONE; } -PyDoc_STRVAR(insert_doc, -"D.insert(index, object) -- insert object before index"); - -PyDoc_STRVAR(remove_doc, -"D.remove(value) -- remove first occurrence of value."); - static int valid_index(Py_ssize_t i, Py_ssize_t limit) { @@ -1246,15 +1344,26 @@ deque_del_item(dequeobject *deque, Py_ssize_t i) assert (i >= 0 && i < Py_SIZE(deque)); if (_deque_rotate(deque, -i)) return -1; - item = deque_popleft(deque, NULL); + item = _collections_deque_popleft_impl(deque); rv = _deque_rotate(deque, i); assert (item != NULL); Py_DECREF(item); return rv; } +/*[clinic input] +_collections.deque.remove + + deque: dequeobject + value: object + / + +Remove first occurrence of value. +[clinic start generated code]*/ + static PyObject * -deque_remove(dequeobject *deque, PyObject *value) +_collections_deque_remove(dequeobject *deque, PyObject *value) +/*[clinic end generated code: output=6e44d24b93f7109e input=d53d4a0b082137f6]*/ { PyObject *item; block *b = deque->leftblock; @@ -1375,8 +1484,17 @@ deque_traverse(dequeobject *deque, visitproc visit, void *arg) return 0; } +/*[clinic input] +_collections.deque.__reduce__ + + deque: dequeobject + +Return state information for pickling. +[clinic start generated code]*/ + static PyObject * -deque_reduce(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +_collections_deque___reduce___impl(dequeobject *deque) +/*[clinic end generated code: output=98e9eed251df2133 input=4210e061fc57e988]*/ { PyObject *state, *it; @@ -1510,40 +1628,38 @@ deque_richcompare(PyObject *v, PyObject *w, int op) return NULL; } +/*[clinic input] +@text_signature "($self, iterable=None, maxlen=None)" +_collections.deque.__init__ + + deque: dequeobject + iterable: object = NULL + maxlen: object = NULL + +A list-like sequence optimized for data accesses near its endpoints. +[clinic start generated code]*/ + static int -deque_init(dequeobject *deque, PyObject *args, PyObject *kwdargs) -{ - PyObject *iterable = NULL; - PyObject *maxlenobj = NULL; - Py_ssize_t maxlen = -1; - char *kwlist[] = {"iterable", "maxlen", 0}; +_collections_deque___init___impl(dequeobject *deque, PyObject *iterable, + PyObject *maxlen) +/*[clinic end generated code: output=9fbb306da99f6694 input=2966a2a0176e4506]*/ - if (kwdargs == NULL && PyTuple_GET_SIZE(args) <= 2) { - if (PyTuple_GET_SIZE(args) > 0) { - iterable = PyTuple_GET_ITEM(args, 0); - } - if (PyTuple_GET_SIZE(args) > 1) { - maxlenobj = PyTuple_GET_ITEM(args, 1); - } - } else { - if (!PyArg_ParseTupleAndKeywords(args, kwdargs, "|OO:deque", kwlist, - &iterable, &maxlenobj)) - return -1; - } - if (maxlenobj != NULL && maxlenobj != Py_None) { - maxlen = PyLong_AsSsize_t(maxlenobj); - if (maxlen == -1 && PyErr_Occurred()) +{ + Py_ssize_t maxlenval = -1; + if (maxlen != NULL && maxlen != Py_None) { + maxlenval = PyLong_AsSsize_t(maxlen); + if (maxlenval == -1 && PyErr_Occurred()) return -1; - if (maxlen < 0) { + if (maxlenval < 0) { PyErr_SetString(PyExc_ValueError, "maxlen must be non-negative"); return -1; } } - deque->maxlen = maxlen; + deque->maxlen = maxlenval; if (Py_SIZE(deque) > 0) deque_clear(deque); if (iterable != NULL) { - PyObject *rv = deque_extend(deque, iterable); + PyObject *rv = _collections_deque_extend(deque, iterable); if (rv == NULL) return -1; Py_DECREF(rv); @@ -1551,8 +1667,17 @@ deque_init(dequeobject *deque, PyObject *args, PyObject *kwdargs) return 0; } +/*[clinic input] +_collections.deque.__sizeof__ + + deque: dequeobject + +Return the size of the deque in memory, in bytes +[clinic start generated code]*/ + static PyObject * -deque_sizeof(dequeobject *deque, void *unused) +_collections_deque___sizeof___impl(dequeobject *deque) +/*[clinic end generated code: output=1a66234430a294a3 input=c0c535e64766f446]*/ { size_t res = _PyObject_SIZE(Py_TYPE(deque)); size_t blocks; @@ -1563,9 +1688,6 @@ deque_sizeof(dequeobject *deque, void *unused) return PyLong_FromSize_t(res); } -PyDoc_STRVAR(sizeof_doc, -"D.__sizeof__() -- size of D in memory, in bytes"); - static PyObject * deque_get_maxlen(dequeobject *deque, void *Py_UNUSED(ignored)) { @@ -1574,6 +1696,22 @@ deque_get_maxlen(dequeobject *deque, void *Py_UNUSED(ignored)) return PyLong_FromSsize_t(deque->maxlen); } +static PyObject *deque_reviter(dequeobject *deque); + +/*[clinic input] +_collections.deque.__reversed__ + + deque: dequeobject + +Return a reverse iterator over the deque. +[clinic start generated code]*/ + +static PyObject * +_collections_deque___reversed___impl(dequeobject *deque) +/*[clinic end generated code: output=c6980fed84a53cc6 input=8b6299d6d60ea01a]*/ +{ + return deque_reviter(deque); +} /* deque object ********************************************************/ @@ -1584,47 +1722,26 @@ static PyGetSetDef deque_getset[] = { }; static PyObject *deque_iter(dequeobject *deque); -static PyObject *deque_reviter(dequeobject *deque, PyObject *Py_UNUSED(ignored)); -PyDoc_STRVAR(reversed_doc, - "D.__reversed__() -- return a reverse iterator over the deque"); static PyMethodDef deque_methods[] = { - {"append", (PyCFunction)deque_append, - METH_O, append_doc}, - {"appendleft", (PyCFunction)deque_appendleft, - METH_O, appendleft_doc}, - {"clear", (PyCFunction)deque_clearmethod, - METH_NOARGS, clear_doc}, - {"__copy__", deque_copy, - METH_NOARGS, copy_doc}, - {"copy", deque_copy, - METH_NOARGS, copy_doc}, - {"count", (PyCFunction)deque_count, - METH_O, count_doc}, - {"extend", (PyCFunction)deque_extend, - METH_O, extend_doc}, - {"extendleft", (PyCFunction)deque_extendleft, - METH_O, extendleft_doc}, - {"index", _PyCFunction_CAST(deque_index), - METH_FASTCALL, index_doc}, - {"insert", _PyCFunction_CAST(deque_insert), - METH_FASTCALL, insert_doc}, - {"pop", (PyCFunction)deque_pop, - METH_NOARGS, pop_doc}, - {"popleft", (PyCFunction)deque_popleft, - METH_NOARGS, popleft_doc}, - {"__reduce__", (PyCFunction)deque_reduce, - METH_NOARGS, reduce_doc}, - {"remove", (PyCFunction)deque_remove, - METH_O, remove_doc}, - {"__reversed__", (PyCFunction)deque_reviter, - METH_NOARGS, reversed_doc}, - {"reverse", (PyCFunction)deque_reverse, - METH_NOARGS, reverse_doc}, - {"rotate", _PyCFunction_CAST(deque_rotate), - METH_FASTCALL, rotate_doc}, - {"__sizeof__", (PyCFunction)deque_sizeof, - METH_NOARGS, sizeof_doc}, + _COLLECTIONS_DEQUE_APPEND_METHODDEF + _COLLECTIONS_DEQUE_APPENDLEFT_METHODDEF + _COLLECTIONS_DEQUE_CLEAR_METHODDEF + _COLLECTIONS_DEQUE___COPY___METHODDEF + _COLLECTIONS_DEQUE_COPY_METHODDEF + _COLLECTIONS_DEQUE_COUNT_METHODDEF + _COLLECTIONS_DEQUE_EXTEND_METHODDEF + _COLLECTIONS_DEQUE_EXTENDLEFT_METHODDEF + _COLLECTIONS_DEQUE_INDEX_METHODDEF + _COLLECTIONS_DEQUE_INSERT_METHODDEF + _COLLECTIONS_DEQUE_POP_METHODDEF + _COLLECTIONS_DEQUE_POPLEFT_METHODDEF + _COLLECTIONS_DEQUE___REDUCE___METHODDEF + _COLLECTIONS_DEQUE_REMOVE_METHODDEF + _COLLECTIONS_DEQUE___REVERSED___METHODDEF + _COLLECTIONS_DEQUE_REVERSE_METHODDEF + _COLLECTIONS_DEQUE_ROTATE_METHODDEF + _COLLECTIONS_DEQUE___SIZEOF___METHODDEF {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, {NULL, NULL} /* sentinel */ @@ -1635,23 +1752,18 @@ static PyMemberDef deque_members[] = { {NULL}, }; -PyDoc_STRVAR(deque_doc, -"deque([iterable[, maxlen]]) --> deque object\n\ -\n\ -A list-like sequence optimized for data accesses near its endpoints."); - static PyType_Slot deque_slots[] = { {Py_tp_dealloc, deque_dealloc}, {Py_tp_repr, deque_repr}, {Py_tp_hash, PyObject_HashNotImplemented}, {Py_tp_getattro, PyObject_GenericGetAttr}, - {Py_tp_doc, (void *)deque_doc}, + {Py_tp_doc, (void *)_collections_deque___init____doc__}, {Py_tp_traverse, deque_traverse}, {Py_tp_clear, deque_clear}, {Py_tp_richcompare, deque_richcompare}, {Py_tp_iter, deque_iter}, {Py_tp_getset, deque_getset}, - {Py_tp_init, deque_init}, + {Py_tp_init, _collections_deque___init__}, {Py_tp_alloc, PyType_GenericAlloc}, {Py_tp_new, deque_new}, {Py_tp_free, PyObject_GC_Del}, @@ -1834,7 +1946,7 @@ static PyType_Spec dequeiter_spec = { /*********************** Deque Reverse Iterator **************************/ static PyObject * -deque_reviter(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +deque_reviter(dequeobject *deque) { dequeiterobject *it; collections_state *state = find_module_state_by_def(Py_TYPE(deque)); @@ -1889,7 +2001,7 @@ dequereviter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; assert(type == state->dequereviter_type); - it = (dequeiterobject*)deque_reviter((dequeobject *)deque, NULL); + it = (dequeiterobject*)deque_reviter((dequeobject *)deque); if (!it) return NULL; /* consume items from the queue */ diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 591ab50c76a8e8..f6302b6019ab3c 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -2,9 +2,427 @@ preserve [clinic start generated code]*/ +#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) +# include "pycore_gc.h" // PyGC_Head +# include "pycore_runtime.h" // _Py_ID() +#endif #include "pycore_abstract.h" // _PyNumber_Index() #include "pycore_modsupport.h" // _PyArg_CheckPositional() +PyDoc_STRVAR(_collections_deque_pop__doc__, +"pop($self, /)\n" +"--\n" +"\n" +"Remove and return the rightmost element."); + +#define _COLLECTIONS_DEQUE_POP_METHODDEF \ + {"pop", (PyCFunction)_collections_deque_pop, METH_NOARGS, _collections_deque_pop__doc__}, + +static PyObject * +_collections_deque_pop_impl(dequeobject *deque); + +static PyObject * +_collections_deque_pop(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque_pop_impl(deque); +} + +PyDoc_STRVAR(_collections_deque_popleft__doc__, +"popleft($self, /)\n" +"--\n" +"\n" +"Remove and return the rightmost element."); + +#define _COLLECTIONS_DEQUE_POPLEFT_METHODDEF \ + {"popleft", (PyCFunction)_collections_deque_popleft, METH_NOARGS, _collections_deque_popleft__doc__}, + +static PyObject * +_collections_deque_popleft_impl(dequeobject *deque); + +static PyObject * +_collections_deque_popleft(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque_popleft_impl(deque); +} + +PyDoc_STRVAR(_collections_deque_append__doc__, +"append($self, item, /)\n" +"--\n" +"\n" +"Add an element to the right side of the deque."); + +#define _COLLECTIONS_DEQUE_APPEND_METHODDEF \ + {"append", (PyCFunction)_collections_deque_append, METH_O, _collections_deque_append__doc__}, + +PyDoc_STRVAR(_collections_deque_appendleft__doc__, +"appendleft($self, item, /)\n" +"--\n" +"\n" +"Add an element to the left side of the deque."); + +#define _COLLECTIONS_DEQUE_APPENDLEFT_METHODDEF \ + {"appendleft", (PyCFunction)_collections_deque_appendleft, METH_O, _collections_deque_appendleft__doc__}, + +PyDoc_STRVAR(_collections_deque_extend__doc__, +"extend($self, iterable, /)\n" +"--\n" +"\n" +"Extend the right side of the deque with elements from the iterable"); + +#define _COLLECTIONS_DEQUE_EXTEND_METHODDEF \ + {"extend", (PyCFunction)_collections_deque_extend, METH_O, _collections_deque_extend__doc__}, + +PyDoc_STRVAR(_collections_deque_extendleft__doc__, +"extendleft($self, iterable, /)\n" +"--\n" +"\n" +"Extend the left side of the deque with elements from the iterable"); + +#define _COLLECTIONS_DEQUE_EXTENDLEFT_METHODDEF \ + {"extendleft", (PyCFunction)_collections_deque_extendleft, METH_O, _collections_deque_extendleft__doc__}, + +PyDoc_STRVAR(_collections_deque_copy__doc__, +"copy($self, /)\n" +"--\n" +"\n" +"Return a shallow copy of a deque."); + +#define _COLLECTIONS_DEQUE_COPY_METHODDEF \ + {"copy", (PyCFunction)_collections_deque_copy, METH_NOARGS, _collections_deque_copy__doc__}, + +static PyObject * +_collections_deque_copy_impl(dequeobject *deque); + +static PyObject * +_collections_deque_copy(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque_copy_impl(deque); +} + +PyDoc_STRVAR(_collections_deque___copy____doc__, +"__copy__($self, /)\n" +"--\n" +"\n" +"Return a shallow copy of a deque."); + +#define _COLLECTIONS_DEQUE___COPY___METHODDEF \ + {"__copy__", (PyCFunction)_collections_deque___copy__, METH_NOARGS, _collections_deque___copy____doc__}, + +static PyObject * +_collections_deque___copy___impl(dequeobject *deque); + +static PyObject * +_collections_deque___copy__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque___copy___impl(deque); +} + +PyDoc_STRVAR(_collections_deque_clear__doc__, +"clear($self, /)\n" +"--\n" +"\n" +"Remove all elements from the deque."); + +#define _COLLECTIONS_DEQUE_CLEAR_METHODDEF \ + {"clear", (PyCFunction)_collections_deque_clear, METH_NOARGS, _collections_deque_clear__doc__}, + +static PyObject * +_collections_deque_clear_impl(dequeobject *deque); + +static PyObject * +_collections_deque_clear(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque_clear_impl(deque); +} + +PyDoc_STRVAR(_collections_deque_rotate__doc__, +"rotate($self, n=1, /)\n" +"--\n" +"\n" +"Rotate the deque n steps to the right. If n is negative, rotates left."); + +#define _COLLECTIONS_DEQUE_ROTATE_METHODDEF \ + {"rotate", _PyCFunction_CAST(_collections_deque_rotate), METH_FASTCALL, _collections_deque_rotate__doc__}, + +static PyObject * +_collections_deque_rotate_impl(dequeobject *deque, Py_ssize_t n); + +static PyObject * +_collections_deque_rotate(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + Py_ssize_t n = 1; + + if (!_PyArg_CheckPositional("rotate", nargs, 0, 1)) { + goto exit; + } + if (nargs < 1) { + goto skip_optional; + } + { + Py_ssize_t ival = -1; + PyObject *iobj = _PyNumber_Index(args[0]); + if (iobj != NULL) { + ival = PyLong_AsSsize_t(iobj); + Py_DECREF(iobj); + } + if (ival == -1 && PyErr_Occurred()) { + goto exit; + } + n = ival; + } +skip_optional: + return_value = _collections_deque_rotate_impl(deque, n); + +exit: + return return_value; +} + +PyDoc_STRVAR(_collections_deque_reverse__doc__, +"reverse($self, /)\n" +"--\n" +"\n" +"Reverse *IN PLACE*"); + +#define _COLLECTIONS_DEQUE_REVERSE_METHODDEF \ + {"reverse", (PyCFunction)_collections_deque_reverse, METH_NOARGS, _collections_deque_reverse__doc__}, + +static PyObject * +_collections_deque_reverse_impl(dequeobject *deque); + +static PyObject * +_collections_deque_reverse(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque_reverse_impl(deque); +} + +PyDoc_STRVAR(_collections_deque_count__doc__, +"count($self, v, /)\n" +"--\n" +"\n" +"Return number of occurrences of v"); + +#define _COLLECTIONS_DEQUE_COUNT_METHODDEF \ + {"count", (PyCFunction)_collections_deque_count, METH_O, _collections_deque_count__doc__}, + +PyDoc_STRVAR(_collections_deque_index__doc__, +"index($self, value, [start, [stop]])\n" +"--\n" +"\n" +"Return first index of value.\n" +"\n" +"Raises ValueError if the value is not present."); + +#define _COLLECTIONS_DEQUE_INDEX_METHODDEF \ + {"index", _PyCFunction_CAST(_collections_deque_index), METH_FASTCALL, _collections_deque_index__doc__}, + +static PyObject * +_collections_deque_index_impl(dequeobject *deque, PyObject *v, + Py_ssize_t start, Py_ssize_t stop); + +static PyObject * +_collections_deque_index(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + PyObject *v; + Py_ssize_t start = 0; + Py_ssize_t stop = Py_SIZE(deque); + + if (!_PyArg_CheckPositional("index", nargs, 1, 3)) { + goto exit; + } + v = args[0]; + if (nargs < 2) { + goto skip_optional; + } + if (!_PyEval_SliceIndexNotNone(args[1], &start)) { + goto exit; + } + if (nargs < 3) { + goto skip_optional; + } + if (!_PyEval_SliceIndexNotNone(args[2], &stop)) { + goto exit; + } +skip_optional: + return_value = _collections_deque_index_impl(deque, v, start, stop); + +exit: + return return_value; +} + +PyDoc_STRVAR(_collections_deque_insert__doc__, +"insert($self, index, value, /)\n" +"--\n" +"\n" +"Insert value before index"); + +#define _COLLECTIONS_DEQUE_INSERT_METHODDEF \ + {"insert", _PyCFunction_CAST(_collections_deque_insert), METH_FASTCALL, _collections_deque_insert__doc__}, + +static PyObject * +_collections_deque_insert_impl(dequeobject *deque, Py_ssize_t index, + PyObject *value); + +static PyObject * +_collections_deque_insert(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + Py_ssize_t index; + PyObject *value; + + if (!_PyArg_CheckPositional("insert", nargs, 2, 2)) { + goto exit; + } + { + Py_ssize_t ival = -1; + PyObject *iobj = _PyNumber_Index(args[0]); + if (iobj != NULL) { + ival = PyLong_AsSsize_t(iobj); + Py_DECREF(iobj); + } + if (ival == -1 && PyErr_Occurred()) { + goto exit; + } + index = ival; + } + value = args[1]; + return_value = _collections_deque_insert_impl(deque, index, value); + +exit: + return return_value; +} + +PyDoc_STRVAR(_collections_deque_remove__doc__, +"remove($self, value, /)\n" +"--\n" +"\n" +"Remove first occurrence of value."); + +#define _COLLECTIONS_DEQUE_REMOVE_METHODDEF \ + {"remove", (PyCFunction)_collections_deque_remove, METH_O, _collections_deque_remove__doc__}, + +PyDoc_STRVAR(_collections_deque___reduce____doc__, +"__reduce__($self, /)\n" +"--\n" +"\n" +"Return state information for pickling."); + +#define _COLLECTIONS_DEQUE___REDUCE___METHODDEF \ + {"__reduce__", (PyCFunction)_collections_deque___reduce__, METH_NOARGS, _collections_deque___reduce____doc__}, + +static PyObject * +_collections_deque___reduce___impl(dequeobject *deque); + +static PyObject * +_collections_deque___reduce__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque___reduce___impl(deque); +} + +PyDoc_STRVAR(_collections_deque___init____doc__, +"deque($self, iterable=None, maxlen=None)\n" +"--\n" +"\n" +"A list-like sequence optimized for data accesses near its endpoints."); + +static int +_collections_deque___init___impl(dequeobject *deque, PyObject *iterable, + PyObject *maxlen); + +static int +_collections_deque___init__(PyObject *deque, PyObject *args, PyObject *kwargs) +{ + int return_value = -1; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 2 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { &_Py_ID(iterable), &_Py_ID(maxlen), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"iterable", "maxlen", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "deque", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[2]; + PyObject * const *fastargs; + Py_ssize_t nargs = PyTuple_GET_SIZE(args); + Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 0; + PyObject *iterable = NULL; + PyObject *maxlen = NULL; + + fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, 0, 2, 0, argsbuf); + if (!fastargs) { + goto exit; + } + if (!noptargs) { + goto skip_optional_pos; + } + if (fastargs[0]) { + iterable = fastargs[0]; + if (!--noptargs) { + goto skip_optional_pos; + } + } + maxlen = fastargs[1]; +skip_optional_pos: + return_value = _collections_deque___init___impl((dequeobject *)deque, iterable, maxlen); + +exit: + return return_value; +} + +PyDoc_STRVAR(_collections_deque___sizeof____doc__, +"__sizeof__($self, /)\n" +"--\n" +"\n" +"Return the size of the deque in memory, in bytes"); + +#define _COLLECTIONS_DEQUE___SIZEOF___METHODDEF \ + {"__sizeof__", (PyCFunction)_collections_deque___sizeof__, METH_NOARGS, _collections_deque___sizeof____doc__}, + +static PyObject * +_collections_deque___sizeof___impl(dequeobject *deque); + +static PyObject * +_collections_deque___sizeof__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque___sizeof___impl(deque); +} + +PyDoc_STRVAR(_collections_deque___reversed____doc__, +"__reversed__($self, /)\n" +"--\n" +"\n" +"Return a reverse iterator over the deque."); + +#define _COLLECTIONS_DEQUE___REVERSED___METHODDEF \ + {"__reversed__", (PyCFunction)_collections_deque___reversed__, METH_NOARGS, _collections_deque___reversed____doc__}, + +static PyObject * +_collections_deque___reversed___impl(dequeobject *deque); + +static PyObject * +_collections_deque___reversed__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) +{ + return _collections_deque___reversed___impl(deque); +} + PyDoc_STRVAR(_collections__count_elements__doc__, "_count_elements($module, mapping, iterable, /)\n" "--\n" @@ -72,4 +490,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=c896a72f8c45930d input=a9049054013a1b77]*/ +/*[clinic end generated code: output=bfdddc7a5fbf9b08 input=a9049054013a1b77]*/ From 010d12c5d657ba5ab9e2b7d43dbe6c7361c652d1 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 21 Dec 2023 20:07:08 -0800 Subject: [PATCH 02/51] Make deque.pop thread-safe --- Modules/_collectionsmodule.c | 33 +++++++++++++++++---------- Modules/clinic/_collectionsmodule.c.h | 11 +++++++-- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 50f1091f93b036..7f5a8701b56453 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -228,18 +228,10 @@ deque_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return (PyObject *)deque; } -/*[clinic input] -_collections.deque.pop - - deque: dequeobject - -Remove and return the rightmost element. -[clinic start generated code]*/ - static PyObject * -_collections_deque_pop_impl(dequeobject *deque) -/*[clinic end generated code: output=2d4ef1dcd5113ae6 input=b4873fc20283d8d6]*/ +deque_pop_locked(dequeobject *deque) { + PyObject *item; block *prevblock; @@ -273,6 +265,23 @@ _collections_deque_pop_impl(dequeobject *deque) } /*[clinic input] +@critical_section +_collections.deque.pop + + deque: dequeobject + +Remove and return the rightmost element. +[clinic start generated code]*/ + +static PyObject * +_collections_deque_pop_impl(dequeobject *deque) +/*[clinic end generated code: output=2d4ef1dcd5113ae6 input=95605871b5f856d5]*/ +{ + return deque_pop_locked(deque); +} + +/*[clinic input] +@critical_section _collections.deque.popleft deque: dequeobject @@ -394,7 +403,7 @@ deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) deque->leftindex--; deque->leftblock->data[deque->leftindex] = item; if (NEEDS_TRIM(deque, deque->maxlen)) { - PyObject *olditem = _collections_deque_pop_impl(deque); + PyObject *olditem = deque_pop_locked(deque); Py_DECREF(olditem); } else { deque->state++; @@ -747,7 +756,7 @@ deque_clear(dequeobject *deque) alternate_method: while (Py_SIZE(deque)) { - item = _collections_deque_pop_impl(deque); + item = deque_pop_locked(deque); assert (item != NULL); Py_DECREF(item); } diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index f6302b6019ab3c..45fa6c874f8e2a 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -7,6 +7,7 @@ preserve # include "pycore_runtime.h" // _Py_ID() #endif #include "pycore_abstract.h" // _PyNumber_Index() +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_CheckPositional() PyDoc_STRVAR(_collections_deque_pop__doc__, @@ -24,7 +25,13 @@ _collections_deque_pop_impl(dequeobject *deque); static PyObject * _collections_deque_pop(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque_pop_impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_pop_impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque_popleft__doc__, @@ -490,4 +497,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=bfdddc7a5fbf9b08 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1c8f5848173225af input=a9049054013a1b77]*/ From 32452db14d22ff1d45d6202e90fd5bf8812143ca Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 21 Dec 2023 20:10:51 -0800 Subject: [PATCH 03/51] Make deque.popleft thread-safe --- Modules/_collectionsmodule.c | 32 ++++++++++++++++----------- Modules/clinic/_collectionsmodule.c.h | 10 +++++++-- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 7f5a8701b56453..b9a86c31bb397b 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -280,18 +280,8 @@ _collections_deque_pop_impl(dequeobject *deque) return deque_pop_locked(deque); } -/*[clinic input] -@critical_section -_collections.deque.popleft - - deque: dequeobject - -Remove and return the rightmost element. -[clinic start generated code]*/ - static PyObject * -_collections_deque_popleft_impl(dequeobject *deque) -/*[clinic end generated code: output=8cd77178b5116aba input=39d64df4664392d3]*/ +deque_popleft_locked(dequeobject *deque) { PyObject *item; block *prevblock; @@ -326,6 +316,22 @@ _collections_deque_popleft_impl(dequeobject *deque) return item; } +/*[clinic input] +@critical_section +_collections.deque.popleft + + deque: dequeobject + +Remove and return the rightmost element. +[clinic start generated code]*/ + +static PyObject * +_collections_deque_popleft_impl(dequeobject *deque) +/*[clinic end generated code: output=8cd77178b5116aba input=a59eae3f0b0b9796]*/ +{ + return deque_popleft_locked(deque); +} + /* The deque's size limit is d.maxlen. The limit can be zero or positive. * If there is no limit, then d.maxlen == -1. * @@ -358,7 +364,7 @@ deque_append_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) deque->rightindex++; deque->rightblock->data[deque->rightindex] = item; if (NEEDS_TRIM(deque, maxlen)) { - PyObject *olditem = _collections_deque_popleft_impl(deque); + PyObject *olditem = deque_popleft_locked(deque); Py_DECREF(olditem); } else { deque->state++; @@ -1353,7 +1359,7 @@ deque_del_item(dequeobject *deque, Py_ssize_t i) assert (i >= 0 && i < Py_SIZE(deque)); if (_deque_rotate(deque, -i)) return -1; - item = _collections_deque_popleft_impl(deque); + item = deque_popleft_locked(deque); rv = _deque_rotate(deque, i); assert (item != NULL); Py_DECREF(item); diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 45fa6c874f8e2a..b681fb8f73b5f6 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -49,7 +49,13 @@ _collections_deque_popleft_impl(dequeobject *deque); static PyObject * _collections_deque_popleft(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque_popleft_impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_popleft_impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque_append__doc__, @@ -497,4 +503,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=1c8f5848173225af input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ff9bb5a7aabafb91 input=a9049054013a1b77]*/ From a73aa108eaa7ededc00357def080b625daccefb6 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 21 Dec 2023 21:47:36 -0800 Subject: [PATCH 04/51] Make deque.append thread-safe --- Modules/_collectionsmodule.c | 47 ++++++++++++++++++--------- Modules/clinic/_collectionsmodule.c.h | 17 +++++++++- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index b9a86c31bb397b..86450ad2b076ec 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -344,15 +344,25 @@ _collections_deque_popleft_impl(dequeobject *deque) * unsigned test that returns true whenever 0 <= maxlen < Py_SIZE(deque). */ -#define NEEDS_TRIM(deque, maxlen) ((size_t)(maxlen) < (size_t)(Py_SIZE(deque))) +#define NEEDS_TRIM(deque) ((size_t)((deque)->maxlen) < (size_t)(Py_SIZE(deque))) -static inline int -deque_append_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) +typedef struct { + // 1 if an error occurred, 0 otherwise + int err; + + // A value that was trimmed if the operation would exceed maxlen, NULL + // otherwise + PyObject *trimmed; +} AppendResult; + +static inline AppendResult +deque_append_locked(dequeobject *deque, PyObject *item) { if (deque->rightindex == BLOCKLEN - 1) { block *b = newblock(deque); - if (b == NULL) - return -1; + if (b == NULL) { + return (AppendResult){1, NULL}; + } b->leftlink = deque->rightblock; CHECK_END(deque->rightblock->rightlink); deque->rightblock->rightlink = b; @@ -362,17 +372,19 @@ deque_append_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) } Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->rightindex++; + Py_INCREF(item); deque->rightblock->data[deque->rightindex] = item; - if (NEEDS_TRIM(deque, maxlen)) { - PyObject *olditem = deque_popleft_locked(deque); - Py_DECREF(olditem); + if (NEEDS_TRIM(deque)) { + PyObject *trimmed = deque_popleft_locked(deque); + return (AppendResult){0, trimmed}; } else { deque->state++; + return (AppendResult){0, NULL}; } - return 0; } /*[clinic input] +@critical_section _collections.deque.append deque: dequeobject @@ -383,11 +395,14 @@ Add an element to the right side of the deque. [clinic start generated code]*/ static PyObject * -_collections_deque_append(dequeobject *deque, PyObject *item) -/*[clinic end generated code: output=fc44cc7b9dcb0180 input=803e0d976a2e2620]*/ +_collections_deque_append_impl(dequeobject *deque, PyObject *item) +/*[clinic end generated code: output=f8339396b82cbb97 input=29d2c1ea535d8fa6]*/ { - if (deque_append_internal(deque, Py_NewRef(item), deque->maxlen) < 0) + AppendResult res = deque_append_locked(deque, item); + Py_XDECREF(res.trimmed); + if (res.err) { return NULL; + } Py_RETURN_NONE; } @@ -408,7 +423,7 @@ deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->leftindex--; deque->leftblock->data[deque->leftindex] = item; - if (NEEDS_TRIM(deque, deque->maxlen)) { + if (NEEDS_TRIM(deque)) { PyObject *olditem = deque_pop_locked(deque); Py_DECREF(olditem); } else { @@ -512,8 +527,10 @@ _collections_deque_extend(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - if (deque_append_internal(deque, item, maxlen) == -1) { - Py_DECREF(item); + AppendResult res = deque_append_locked(deque, item); + Py_DECREF(item); + Py_XDECREF(res.trimmed); + if (res.err) { Py_DECREF(it); return NULL; } diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index b681fb8f73b5f6..6c3bfba99ec80e 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -67,6 +67,21 @@ PyDoc_STRVAR(_collections_deque_append__doc__, #define _COLLECTIONS_DEQUE_APPEND_METHODDEF \ {"append", (PyCFunction)_collections_deque_append, METH_O, _collections_deque_append__doc__}, +static PyObject * +_collections_deque_append_impl(dequeobject *deque, PyObject *item); + +static PyObject * +_collections_deque_append(dequeobject *deque, PyObject *item) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_append_impl(deque, item); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque_appendleft__doc__, "appendleft($self, item, /)\n" "--\n" @@ -503,4 +518,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=ff9bb5a7aabafb91 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=579df8454c63c2e4 input=a9049054013a1b77]*/ From 759d149569cf5a9ad113c6a615076fe116d74990 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 21 Dec 2023 21:52:11 -0800 Subject: [PATCH 05/51] Make deque.appendleft thread-safe --- Modules/_collectionsmodule.c | 32 +++++++++++++++++---------- Modules/clinic/_collectionsmodule.c.h | 17 +++++++++++++- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 86450ad2b076ec..d4af5844fbe702 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -406,13 +406,14 @@ _collections_deque_append_impl(dequeobject *deque, PyObject *item) Py_RETURN_NONE; } -static inline int -deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) +static inline AppendResult +deque_appendleft_locked(dequeobject *deque, PyObject *item) { if (deque->leftindex == 0) { block *b = newblock(deque); - if (b == NULL) - return -1; + if (b == NULL) { + return (AppendResult){1, NULL}; + } b->rightlink = deque->leftblock; CHECK_END(deque->leftblock->leftlink); deque->leftblock->leftlink = b; @@ -422,17 +423,19 @@ deque_appendleft_internal(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) } Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->leftindex--; + Py_INCREF(item); deque->leftblock->data[deque->leftindex] = item; if (NEEDS_TRIM(deque)) { - PyObject *olditem = deque_pop_locked(deque); - Py_DECREF(olditem); + PyObject *trimmed = deque_pop_locked(deque); + return (AppendResult){0, trimmed}; } else { deque->state++; + return (AppendResult){0, NULL}; } - return 0; } /*[clinic input] +@critical_section _collections.deque.appendleft deque: dequeobject @@ -443,11 +446,14 @@ Add an element to the left side of the deque. [clinic start generated code]*/ static PyObject * -_collections_deque_appendleft(dequeobject *deque, PyObject *item) -/*[clinic end generated code: output=f1b75022fbccf8bb input=481442915f0f6465]*/ +_collections_deque_appendleft_impl(dequeobject *deque, PyObject *item) +/*[clinic end generated code: output=5d64c3723b5f0b02 input=78335f6b0654b2fd]*/ { - if (deque_appendleft_internal(deque, Py_NewRef(item), deque->maxlen) < 0) + AppendResult res = deque_appendleft_locked(deque, item); + Py_XDECREF(res.trimmed); + if (res.err) { return NULL; + } Py_RETURN_NONE; } @@ -584,8 +590,10 @@ _collections_deque_extendleft(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - if (deque_appendleft_internal(deque, item, maxlen) == -1) { - Py_DECREF(item); + AppendResult res = deque_appendleft_locked(deque, item); + Py_DECREF(item); + Py_XDECREF(res.trimmed); + if (res.err) { Py_DECREF(it); return NULL; } diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 6c3bfba99ec80e..5611ad758701f6 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -91,6 +91,21 @@ PyDoc_STRVAR(_collections_deque_appendleft__doc__, #define _COLLECTIONS_DEQUE_APPENDLEFT_METHODDEF \ {"appendleft", (PyCFunction)_collections_deque_appendleft, METH_O, _collections_deque_appendleft__doc__}, +static PyObject * +_collections_deque_appendleft_impl(dequeobject *deque, PyObject *item); + +static PyObject * +_collections_deque_appendleft(dequeobject *deque, PyObject *item) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_appendleft_impl(deque, item); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque_extend__doc__, "extend($self, iterable, /)\n" "--\n" @@ -518,4 +533,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=579df8454c63c2e4 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=d07b2cf01ca914eb input=a9049054013a1b77]*/ From c0b14ea7d0517cc85f96eca7ac93bfd485190a8d Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 10:46:08 -0800 Subject: [PATCH 06/51] Make deque.count thread-safe --- Modules/_collectionsmodule.c | 5 +++-- Modules/clinic/_collectionsmodule.c.h | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index d4af5844fbe702..d56b05f7f89b85 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1117,6 +1117,7 @@ _collections_deque_reverse_impl(dequeobject *deque) } /*[clinic input] +@critical_section _collections.deque.count deque: dequeobject @@ -1127,8 +1128,8 @@ Return number of occurrences of v [clinic start generated code]*/ static PyObject * -_collections_deque_count(dequeobject *deque, PyObject *v) -/*[clinic end generated code: output=4fd47b6bf522f071 input=38d3b9f0f9993e26]*/ +_collections_deque_count_impl(dequeobject *deque, PyObject *v) +/*[clinic end generated code: output=9c70678be9804685 input=f430e22a70390922]*/ { block *b = deque->leftblock; Py_ssize_t index = deque->leftindex; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 5611ad758701f6..9412d038224f5f 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -248,6 +248,21 @@ PyDoc_STRVAR(_collections_deque_count__doc__, #define _COLLECTIONS_DEQUE_COUNT_METHODDEF \ {"count", (PyCFunction)_collections_deque_count, METH_O, _collections_deque_count__doc__}, +static PyObject * +_collections_deque_count_impl(dequeobject *deque, PyObject *v); + +static PyObject * +_collections_deque_count(dequeobject *deque, PyObject *v) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_count_impl(deque, v); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque_index__doc__, "index($self, value, [start, [stop]])\n" "--\n" @@ -533,4 +548,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=d07b2cf01ca914eb input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e82801f66853ad40 input=a9049054013a1b77]*/ From 72bee62ea89be365526774a453ca629c25f41873 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 10:57:33 -0800 Subject: [PATCH 07/51] Make deque.index thread-safe --- Modules/_collectionsmodule.c | 3 ++- Modules/clinic/_collectionsmodule.c.h | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index d56b05f7f89b85..173e146cdee685 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1203,6 +1203,7 @@ deque_len(dequeobject *deque) } /*[clinic input] +@critical_section @text_signature "($self, value, [start, [stop]])" _collections.deque.index @@ -1220,7 +1221,7 @@ Raises ValueError if the value is not present. static PyObject * _collections_deque_index_impl(dequeobject *deque, PyObject *v, Py_ssize_t start, Py_ssize_t stop) -/*[clinic end generated code: output=5b2a991d7315b3cf input=b31d3a5c49cb8725]*/ +/*[clinic end generated code: output=5b2a991d7315b3cf input=3f189081c79a28a5]*/ { Py_ssize_t i, n; PyObject *item; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 9412d038224f5f..ded268745ba460 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -303,7 +303,9 @@ _collections_deque_index(dequeobject *deque, PyObject *const *args, Py_ssize_t n goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(deque); return_value = _collections_deque_index_impl(deque, v, start, stop); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -548,4 +550,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=e82801f66853ad40 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=246da3f1a260075d input=a9049054013a1b77]*/ From 2fd397af377395ac249b7eb22ff55a15a67588da Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:08:07 -0800 Subject: [PATCH 08/51] Make deque.insert thread-safe --- Modules/_collectionsmodule.c | 13 +++++++------ Modules/clinic/_collectionsmodule.c.h | 4 +++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 173e146cdee685..ba374ef71d3854 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1290,6 +1290,7 @@ _collections_deque_index_impl(dequeobject *deque, PyObject *v, */ /*[clinic input] +@critical_section _collections.deque.insert deque: dequeobject @@ -1303,25 +1304,25 @@ Insert value before index static PyObject * _collections_deque_insert_impl(dequeobject *deque, Py_ssize_t index, PyObject *value) -/*[clinic end generated code: output=f913d56fc97caddf input=0593cc27bffa766a]*/ +/*[clinic end generated code: output=f913d56fc97caddf input=5366fe3bf43c5231]*/ { Py_ssize_t n = Py_SIZE(deque); PyObject *rv; - if (deque->maxlen == Py_SIZE(deque)) { + if (deque->maxlen == n) { PyErr_SetString(PyExc_IndexError, "deque already at its maximum size"); return NULL; } if (index >= n) - return _collections_deque_append(deque, value); + return _collections_deque_append_impl(deque, value); if (index <= -n || index == 0) - return _collections_deque_appendleft(deque, value); + return _collections_deque_appendleft_impl(deque, value); if (_deque_rotate(deque, -index)) return NULL; if (index < 0) - rv = _collections_deque_append(deque, value); + rv = _collections_deque_append_impl(deque, value); else - rv = _collections_deque_appendleft(deque, value); + rv = _collections_deque_appendleft_impl(deque, value); if (rv == NULL) return NULL; Py_DECREF(rv); diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index ded268745ba460..b7575b837c4ec5 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -347,7 +347,9 @@ _collections_deque_insert(dequeobject *deque, PyObject *const *args, Py_ssize_t index = ival; } value = args[1]; + Py_BEGIN_CRITICAL_SECTION(deque); return_value = _collections_deque_insert_impl(deque, index, value); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -550,4 +552,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=246da3f1a260075d input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1b7104017b44333e input=a9049054013a1b77]*/ From dbe49a0c0f7af5bfd37468453598fa00bb094474 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:12:55 -0800 Subject: [PATCH 09/51] Make deque.remove thread-safe --- Modules/_collectionsmodule.c | 5 +++-- Modules/clinic/_collectionsmodule.c.h | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index ba374ef71d3854..c69c80f31ef4a3 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1395,6 +1395,7 @@ deque_del_item(dequeobject *deque, Py_ssize_t i) } /*[clinic input] +@critical_section _collections.deque.remove deque: dequeobject @@ -1405,8 +1406,8 @@ Remove first occurrence of value. [clinic start generated code]*/ static PyObject * -_collections_deque_remove(dequeobject *deque, PyObject *value) -/*[clinic end generated code: output=6e44d24b93f7109e input=d53d4a0b082137f6]*/ +_collections_deque_remove_impl(dequeobject *deque, PyObject *value) +/*[clinic end generated code: output=a841fda79ad70372 input=ad91dbbfb64e677a]*/ { PyObject *item; block *b = deque->leftblock; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index b7575b837c4ec5..8775f26346356f 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -364,6 +364,21 @@ PyDoc_STRVAR(_collections_deque_remove__doc__, #define _COLLECTIONS_DEQUE_REMOVE_METHODDEF \ {"remove", (PyCFunction)_collections_deque_remove, METH_O, _collections_deque_remove__doc__}, +static PyObject * +_collections_deque_remove_impl(dequeobject *deque, PyObject *value); + +static PyObject * +_collections_deque_remove(dequeobject *deque, PyObject *value) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_remove_impl(deque, value); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque___reduce____doc__, "__reduce__($self, /)\n" "--\n" @@ -552,4 +567,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=1b7104017b44333e input=a9049054013a1b77]*/ +/*[clinic end generated code: output=523c22da39db3275 input=a9049054013a1b77]*/ From 5af87e4370a7ea152997d3aec032baf0a9a8df02 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:22:37 -0800 Subject: [PATCH 10/51] Make deque.rotate thread-safe --- Modules/_collectionsmodule.c | 3 ++- Modules/clinic/_collectionsmodule.c.h | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index c69c80f31ef4a3..b495b7b619800f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1051,6 +1051,7 @@ _deque_rotate(dequeobject *deque, Py_ssize_t n) } /*[clinic input] +@critical_section _collections.deque.rotate deque: dequeobject @@ -1062,7 +1063,7 @@ Rotate the deque n steps to the right. If n is negative, rotates left. static PyObject * _collections_deque_rotate_impl(dequeobject *deque, Py_ssize_t n) -/*[clinic end generated code: output=5a9df290cc0d3adf input=0d7f4900fe866917]*/ +/*[clinic end generated code: output=5a9df290cc0d3adf input=0b3d3912d59abb9f]*/ { if (!_deque_rotate(deque, n)) Py_RETURN_NONE; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 8775f26346356f..4823ecf9a6ab96 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -215,7 +215,9 @@ _collections_deque_rotate(dequeobject *deque, PyObject *const *args, Py_ssize_t n = ival; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(deque); return_value = _collections_deque_rotate_impl(deque, n); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -567,4 +569,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=523c22da39db3275 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=048f7e8a64c412ea input=a9049054013a1b77]*/ From 6fcd551ca066ec80215dcb85164570efc175b36b Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:30:28 -0800 Subject: [PATCH 11/51] Make deque.reverse thread-safe --- Modules/_collectionsmodule.c | 3 ++- Modules/clinic/_collectionsmodule.c.h | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index b495b7b619800f..35562412f8ee47 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1071,6 +1071,7 @@ _collections_deque_rotate_impl(dequeobject *deque, Py_ssize_t n) } /*[clinic input] +@critical_section _collections.deque.reverse deque: dequeobject @@ -1080,7 +1081,7 @@ Reverse *IN PLACE* static PyObject * _collections_deque_reverse_impl(dequeobject *deque) -/*[clinic end generated code: output=8f859d206158686e input=651e0257414fac22]*/ +/*[clinic end generated code: output=8f859d206158686e input=40b6ff9d466609da]*/ { block *leftblock = deque->leftblock; block *rightblock = deque->rightblock; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 4823ecf9a6ab96..cbdc8dc9760996 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -238,7 +238,13 @@ _collections_deque_reverse_impl(dequeobject *deque); static PyObject * _collections_deque_reverse(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque_reverse_impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_reverse_impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque_count__doc__, @@ -569,4 +575,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=048f7e8a64c412ea input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b5cbcfc19c5a2f7e input=a9049054013a1b77]*/ From 70f07a712a6664d27c3a9effe95be8549702fdad Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:39:34 -0800 Subject: [PATCH 12/51] Make deque.contains thread-safe --- Modules/_collectionsmodule.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 35562412f8ee47..88346599e5eb07 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1167,7 +1167,7 @@ _collections_deque_count_impl(dequeobject *deque, PyObject *v) } static int -deque_contains(dequeobject *deque, PyObject *v) +deque_contains_locked(dequeobject *deque, PyObject *v) { block *b = deque->leftblock; Py_ssize_t index = deque->leftindex; @@ -1198,6 +1198,15 @@ deque_contains(dequeobject *deque, PyObject *v) return 0; } +static int +deque_contains(dequeobject *deque, PyObject *v) +{ + Py_BEGIN_CRITICAL_SECTION(deque); + int result = deque_contains_locked(deque, v); + Py_END_CRITICAL_SECTION(); + return result; +} + static Py_ssize_t deque_len(dequeobject *deque) { From 16bbe58a9592cd87ed3a13d82595b566c4c31b6c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:48:07 -0800 Subject: [PATCH 13/51] Make deque indexing thread-safe --- Modules/_collectionsmodule.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 88346599e5eb07..e05c212a90748f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1351,7 +1351,7 @@ valid_index(Py_ssize_t i, Py_ssize_t limit) } static PyObject * -deque_item(dequeobject *deque, Py_ssize_t i) +deque_item_locked(dequeobject *deque, Py_ssize_t i) { block *b; PyObject *item; @@ -1389,6 +1389,15 @@ deque_item(dequeobject *deque, Py_ssize_t i) return Py_NewRef(item); } +static PyObject * +deque_item(dequeobject *deque, Py_ssize_t i) +{ + Py_BEGIN_CRITICAL_SECTION(deque); + PyObject *result = deque_item_locked(deque, i); + Py_END_CRITICAL_SECTION(); + return result; +} + static int deque_del_item(dequeobject *deque, Py_ssize_t i) { From cbd01c7972e5d78e853eb3b492f5dc9bc5c12e37 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 11:54:27 -0800 Subject: [PATCH 14/51] Make deque item assignment thread-safe --- Modules/_collectionsmodule.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index e05c212a90748f..d53cd17c4e97f7 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1468,7 +1468,7 @@ _collections_deque_remove_impl(dequeobject *deque, PyObject *value) } static int -deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) +deque_ass_item_locked(dequeobject *deque, Py_ssize_t i, PyObject *v) { block *b; Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i; @@ -1499,6 +1499,15 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) return 0; } +static int +deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) +{ + Py_BEGIN_CRITICAL_SECTION(deque); + int result = deque_ass_item_locked(deque, i, v); + Py_END_CRITICAL_SECTION(); + return result; +} + static void deque_dealloc(dequeobject *deque) { From 92782b1acc34ed6b27a856c33009845464e114d4 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 14:00:58 -0800 Subject: [PATCH 15/51] Make deque.extend thread-safe --- Modules/_collectionsmodule.c | 33 +++++++++++++++------------ Modules/clinic/_collectionsmodule.c.h | 17 +++++++++++++- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index d53cd17c4e97f7..6aa3185a684126 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -488,6 +488,7 @@ consume_iterator(PyObject *it) } /*[clinic input] +@critical_section _collections.deque.extend deque: dequeobject @@ -498,30 +499,30 @@ Extend the right side of the deque with elements from the iterable [clinic start generated code]*/ static PyObject * -_collections_deque_extend(dequeobject *deque, PyObject *iterable) -/*[clinic end generated code: output=a58014bf32cb0b9d input=5a75e68f72ed8f09]*/ +_collections_deque_extend_impl(dequeobject *deque, PyObject *iterable) +/*[clinic end generated code: output=f3e9dc9c46c1743e input=dc36931049537123]*/ { PyObject *it, *item; PyObject *(*iternext)(PyObject *); - Py_ssize_t maxlen = deque->maxlen; + PyObject *list = NULL; + PyObject *result = NULL; /* Handle case where id(deque) == id(iterable) */ if ((PyObject *)deque == iterable) { - PyObject *result; - PyObject *s = PySequence_List(iterable); - if (s == NULL) + list = PySequence_List(iterable); + if (list == NULL) return NULL; - result = _collections_deque_extend(deque, s); - Py_DECREF(s); - return result; + iterable = list; } it = PyObject_GetIter(iterable); if (it == NULL) - return NULL; + goto done; - if (maxlen == 0) - return consume_iterator(it); + if (deque->maxlen == 0) { + result = consume_iterator(it); + goto done; + } /* Space saving heuristic. Start filling from the left */ if (Py_SIZE(deque) == 0) { @@ -538,10 +539,14 @@ _collections_deque_extend(dequeobject *deque, PyObject *iterable) Py_XDECREF(res.trimmed); if (res.err) { Py_DECREF(it); - return NULL; + goto done; } } - return finalize_iterator(it); + result = finalize_iterator(it); + +done: + Py_XDECREF(list); + return result; } /*[clinic input] diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index cbdc8dc9760996..44f46a6813c239 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -115,6 +115,21 @@ PyDoc_STRVAR(_collections_deque_extend__doc__, #define _COLLECTIONS_DEQUE_EXTEND_METHODDEF \ {"extend", (PyCFunction)_collections_deque_extend, METH_O, _collections_deque_extend__doc__}, +static PyObject * +_collections_deque_extend_impl(dequeobject *deque, PyObject *iterable); + +static PyObject * +_collections_deque_extend(dequeobject *deque, PyObject *iterable) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_extend_impl(deque, iterable); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque_extendleft__doc__, "extendleft($self, iterable, /)\n" "--\n" @@ -575,4 +590,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=b5cbcfc19c5a2f7e input=a9049054013a1b77]*/ +/*[clinic end generated code: output=871228ba5b3cd690 input=a9049054013a1b77]*/ From 112b6c9adf83e7d7e2daefb022aa3b6480fa8dec Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 14:10:43 -0800 Subject: [PATCH 16/51] Make deque.extendleft thread-safe --- Modules/_collectionsmodule.c | 33 +++++++++++++++------------ Modules/clinic/_collectionsmodule.c.h | 17 +++++++++++++- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 6aa3185a684126..3ef97fa3fd72c9 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -550,6 +550,7 @@ _collections_deque_extend_impl(dequeobject *deque, PyObject *iterable) } /*[clinic input] +@critical_section _collections.deque.extendleft deque: dequeobject @@ -560,30 +561,30 @@ Extend the left side of the deque with elements from the iterable [clinic start generated code]*/ static PyObject * -_collections_deque_extendleft(dequeobject *deque, PyObject *iterable) -/*[clinic end generated code: output=0a0df3269097f284 input=8dae4c4f9d852a4c]*/ +_collections_deque_extendleft_impl(dequeobject *deque, PyObject *iterable) +/*[clinic end generated code: output=216e82176138d893 input=1c996be5e36cff80]*/ { PyObject *it, *item; PyObject *(*iternext)(PyObject *); - Py_ssize_t maxlen = deque->maxlen; + PyObject *list = NULL; + PyObject *result = NULL; /* Handle case where id(deque) == id(iterable) */ if ((PyObject *)deque == iterable) { - PyObject *result; - PyObject *s = PySequence_List(iterable); - if (s == NULL) + list = PySequence_List(iterable); + if (list == NULL) return NULL; - result = _collections_deque_extendleft(deque, s); - Py_DECREF(s); - return result; + iterable = list; } it = PyObject_GetIter(iterable); if (it == NULL) - return NULL; + goto done; - if (maxlen == 0) - return consume_iterator(it); + if (deque->maxlen == 0) { + result = consume_iterator(it); + goto done; + } /* Space saving heuristic. Start filling from the right */ if (Py_SIZE(deque) == 0) { @@ -600,10 +601,14 @@ _collections_deque_extendleft(dequeobject *deque, PyObject *iterable) Py_XDECREF(res.trimmed); if (res.err) { Py_DECREF(it); - return NULL; + goto done; } } - return finalize_iterator(it); + result = finalize_iterator(it); + +done: + Py_XDECREF(list); + return result; } static PyObject * diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 44f46a6813c239..00429986e035e3 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -139,6 +139,21 @@ PyDoc_STRVAR(_collections_deque_extendleft__doc__, #define _COLLECTIONS_DEQUE_EXTENDLEFT_METHODDEF \ {"extendleft", (PyCFunction)_collections_deque_extendleft, METH_O, _collections_deque_extendleft__doc__}, +static PyObject * +_collections_deque_extendleft_impl(dequeobject *deque, PyObject *iterable); + +static PyObject * +_collections_deque_extendleft(dequeobject *deque, PyObject *iterable) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_extendleft_impl(deque, iterable); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(_collections_deque_copy__doc__, "copy($self, /)\n" "--\n" @@ -590,4 +605,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=871228ba5b3cd690 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=9eb2569a0ac6fa76 input=a9049054013a1b77]*/ From a4201f1bd5bd83f0a06a999b1fd77a99e15c13a8 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 14:14:20 -0800 Subject: [PATCH 17/51] Add note about why inplace concat doesn't need a critical section --- Modules/_collectionsmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 3ef97fa3fd72c9..303c2366aa4cba 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -616,6 +616,7 @@ deque_inplace_concat(dequeobject *deque, PyObject *other) { PyObject *result; + // _collections_deque_extend is thread-safe result = _collections_deque_extend(deque, other); if (result == NULL) return result; From 1e7d0b8f42496eb63fef364147287922cbdc3d80 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 14:31:42 -0800 Subject: [PATCH 18/51] Make deque.clear thread-safe --- Modules/_collectionsmodule.c | 14 ++++++++++++-- Modules/clinic/_collectionsmodule.c.h | 10 ++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 303c2366aa4cba..9e8e767f16c43d 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -805,7 +805,17 @@ deque_clear(dequeobject *deque) return 0; } +static int +deque_tp_clear(dequeobject *deque) +{ + Py_BEGIN_CRITICAL_SECTION(deque); + int result = deque_clear(deque); + Py_END_CRITICAL_SECTION(); + return result; +} + /*[clinic input] +@critical_section _collections.deque.clear deque: dequeobject @@ -815,7 +825,7 @@ Remove all elements from the deque. static PyObject * _collections_deque_clear_impl(dequeobject *deque) -/*[clinic end generated code: output=0f0b9d60188bf83b input=9c003117680a7abf]*/ +/*[clinic end generated code: output=0f0b9d60188bf83b input=3b39b5d2ae8b3dca]*/ { deque_clear(deque); Py_RETURN_NONE; @@ -1843,7 +1853,7 @@ static PyType_Slot deque_slots[] = { {Py_tp_getattro, PyObject_GenericGetAttr}, {Py_tp_doc, (void *)_collections_deque___init____doc__}, {Py_tp_traverse, deque_traverse}, - {Py_tp_clear, deque_clear}, + {Py_tp_clear, deque_tp_clear}, {Py_tp_richcompare, deque_richcompare}, {Py_tp_iter, deque_iter}, {Py_tp_getset, deque_getset}, diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 00429986e035e3..e5f37e6640ea7f 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -205,7 +205,13 @@ _collections_deque_clear_impl(dequeobject *deque); static PyObject * _collections_deque_clear(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque_clear_impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_clear_impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque_rotate__doc__, @@ -605,4 +611,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=9eb2569a0ac6fa76 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=29df3c964eaaab55 input=a9049054013a1b77]*/ From 54975b9c1d8a8927db12988a2f31dac286fbb344 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 16:57:17 -0800 Subject: [PATCH 19/51] Make deque's inplace repeat thread-safe --- Modules/_collectionsmodule.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 9e8e767f16c43d..6f0fdbd2e8c619 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -832,7 +832,7 @@ _collections_deque_clear_impl(dequeobject *deque) } static PyObject * -deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) +deque_inplace_repeat_locked(dequeobject *deque, Py_ssize_t n) { Py_ssize_t i, m, size; PyObject *seq; @@ -896,7 +896,7 @@ deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) n = (deque->maxlen + size - 1) / size; for (i = 0 ; i < n-1 ; i++) { - rv = _collections_deque_extend(deque, seq); + rv = _collections_deque_extend_impl(deque, seq); if (rv == NULL) { Py_DECREF(seq); return NULL; @@ -908,6 +908,15 @@ deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) return (PyObject *)deque; } +static PyObject * +deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) +{ + Py_BEGIN_CRITICAL_SECTION(deque); + PyObject *result = deque_inplace_repeat_locked(deque, n); + Py_END_CRITICAL_SECTION(); + return result; +} + static PyObject * deque_repeat(dequeobject *deque, Py_ssize_t n) { From 1deb61c92f72b298dafa1703491af739a1898dc5 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 17:45:23 -0800 Subject: [PATCH 20/51] Make deque.copy thread-safe --- Modules/_collectionsmodule.c | 16 +++++++++++----- Modules/clinic/_collectionsmodule.c.h | 18 +++++++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 6f0fdbd2e8c619..51c7c21aea7ce1 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -627,6 +627,7 @@ deque_inplace_concat(dequeobject *deque, PyObject *other) /*[clinic input] +@critical_section _collections.deque.copy deque: dequeobject @@ -636,7 +637,7 @@ Return a shallow copy of a deque. static PyObject * _collections_deque_copy_impl(dequeobject *deque) -/*[clinic end generated code: output=af1e3831be813117 input=f575fc72c00333d4]*/ +/*[clinic end generated code: output=af1e3831be813117 input=471ed29263775e83]*/ { PyObject *result; dequeobject *old_deque = (dequeobject *)deque; @@ -650,12 +651,16 @@ _collections_deque_copy_impl(dequeobject *deque) if (new_deque == NULL) return NULL; new_deque->maxlen = old_deque->maxlen; - /* Fast path for the deque_repeat() common case where len(deque) == 1 */ + /* Fast path for the deque_repeat() common case where len(deque) == 1 + * + * It's safe to not acquire the per-object lock for new_deque; it's + * invisible to other threads. + */ if (Py_SIZE(deque) == 1) { PyObject *item = old_deque->leftblock->data[old_deque->leftindex]; - rv = _collections_deque_append(new_deque, item); + rv = _collections_deque_append_impl(new_deque, item); } else { - rv = _collections_deque_extend(new_deque, (PyObject *) deque); + rv = _collections_deque_extend_impl(new_deque, (PyObject *) deque); } if (rv != NULL) { Py_DECREF(rv); @@ -680,6 +685,7 @@ _collections_deque_copy_impl(dequeobject *deque) } /*[clinic input] +@critical_section _collections.deque.__copy__ = _collections.deque.copy Return a shallow copy of a deque. @@ -687,7 +693,7 @@ Return a shallow copy of a deque. static PyObject * _collections_deque___copy___impl(dequeobject *deque) -/*[clinic end generated code: output=c4c31949334138fd input=9d78c00375929799]*/ +/*[clinic end generated code: output=c4c31949334138fd input=8775d5bafd63f5ee]*/ { return _collections_deque_copy_impl(deque); } diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index e5f37e6640ea7f..af7ad3a03065c3 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -169,7 +169,13 @@ _collections_deque_copy_impl(dequeobject *deque); static PyObject * _collections_deque_copy(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque_copy_impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque_copy_impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque___copy____doc__, @@ -187,7 +193,13 @@ _collections_deque___copy___impl(dequeobject *deque); static PyObject * _collections_deque___copy__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque___copy___impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque___copy___impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque_clear__doc__, @@ -611,4 +623,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=29df3c964eaaab55 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=92186babdf20cea0 input=a9049054013a1b77]*/ From 6b1cb4329fb91481a0816645542a6ceabe28ddac Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 17:49:49 -0800 Subject: [PATCH 21/51] Make deque.repeat thread-safe --- Modules/_collectionsmodule.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 51c7c21aea7ce1..14b8689100f9e4 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -929,10 +929,14 @@ deque_repeat(dequeobject *deque, Py_ssize_t n) dequeobject *new_deque; PyObject *rv; + Py_BEGIN_CRITICAL_SECTION(deque); new_deque = (dequeobject *) _collections_deque_copy_impl(deque); + Py_END_CRITICAL_SECTION(); if (new_deque == NULL) return NULL; - rv = deque_inplace_repeat(new_deque, n); + // It's safe to not acquire the per-object lock for new_deque; it's + // invisible to other threads. + rv = deque_inplace_repeat_locked(new_deque, n); Py_DECREF(new_deque); return rv; } From 1957c92723d57cd03896e1c521ac2910015a4aff Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 17:56:40 -0800 Subject: [PATCH 22/51] Make deque concat thread-safe --- Modules/_collectionsmodule.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 14b8689100f9e4..52666532c055a0 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -704,9 +704,11 @@ deque_concat(dequeobject *deque, PyObject *other) PyObject *new_deque, *result; int rv; + Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); rv = PyObject_IsInstance(other, (PyObject *)state->deque_type); if (rv <= 0) { + Py_END_CRITICAL_SECTION(); if (rv == 0) { PyErr_Format(PyExc_TypeError, "can only concatenate deque (not \"%.200s\") to deque", @@ -716,9 +718,12 @@ deque_concat(dequeobject *deque, PyObject *other) } new_deque = _collections_deque_copy_impl(deque); + Py_END_CRITICAL_SECTION(); if (new_deque == NULL) return NULL; - result = _collections_deque_extend((dequeobject *)new_deque, other); + // It's safe to not acquire the per-object lock for new_deque; it's + // invisible to other threads. + result = _collections_deque_extend_impl((dequeobject *)new_deque, other); if (result == NULL) { Py_DECREF(new_deque); return NULL; From 6141ce5ecc068d03f97b1074c798592f360d4007 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 18:00:04 -0800 Subject: [PATCH 23/51] Make deque.__reduce__ thread-safe --- Modules/_collectionsmodule.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 52666532c055a0..a6cd8d2d44d49d 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1627,11 +1627,16 @@ _collections_deque___reduce___impl(dequeobject *deque) return NULL; } - if (deque->maxlen < 0) { - return Py_BuildValue("O()NN", Py_TYPE(deque), state, it); + Py_BEGIN_CRITICAL_SECTION(deque); + Py_ssize_t maxlen = deque->maxlen; + PyTypeObject *typ = Py_TYPE(deque); + Py_END_CRITICAL_SECTION(); + + if (maxlen < 0) { + return Py_BuildValue("O()NN", typ, state, it); } else { - return Py_BuildValue("O(()n)NN", Py_TYPE(deque), deque->maxlen, state, it); + return Py_BuildValue("O(()n)NN", typ, maxlen, state, it); } } From 182a0344e7e4176748c4e953cd0f688610bbfe2e Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 18:03:40 -0800 Subject: [PATCH 24/51] Make deque.len thread-safe --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index a6cd8d2d44d49d..b52f87804ee5a1 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1255,7 +1255,7 @@ deque_contains(dequeobject *deque, PyObject *v) static Py_ssize_t deque_len(dequeobject *deque) { - return Py_SIZE(deque); + return _Py_atomic_load_ssize(&((PyVarObject *)deque)->ob_size); } /*[clinic input] From c8fdcece41d88b6ef3769665bbdacdbf154c66e1 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 18:06:23 -0800 Subject: [PATCH 25/51] Make deque.__init__ thread-safe --- Modules/_collectionsmodule.c | 5 +++-- Modules/clinic/_collectionsmodule.c.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index b52f87804ee5a1..ec887192c03021 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1752,6 +1752,7 @@ deque_richcompare(PyObject *v, PyObject *w, int op) } /*[clinic input] +@critical_section @text_signature "($self, iterable=None, maxlen=None)" _collections.deque.__init__ @@ -1765,7 +1766,7 @@ A list-like sequence optimized for data accesses near its endpoints. static int _collections_deque___init___impl(dequeobject *deque, PyObject *iterable, PyObject *maxlen) -/*[clinic end generated code: output=9fbb306da99f6694 input=2966a2a0176e4506]*/ +/*[clinic end generated code: output=9fbb306da99f6694 input=028e41dfaecb9617]*/ { Py_ssize_t maxlenval = -1; @@ -1782,7 +1783,7 @@ _collections_deque___init___impl(dequeobject *deque, PyObject *iterable, if (Py_SIZE(deque) > 0) deque_clear(deque); if (iterable != NULL) { - PyObject *rv = _collections_deque_extend(deque, iterable); + PyObject *rv = _collections_deque_extend_impl(deque, iterable); if (rv == NULL) return -1; Py_DECREF(rv); diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index af7ad3a03065c3..4660d2b186ea98 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -514,7 +514,9 @@ _collections_deque___init__(PyObject *deque, PyObject *args, PyObject *kwargs) } maxlen = fastargs[1]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(deque); return_value = _collections_deque___init___impl((dequeobject *)deque, iterable, maxlen); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -623,4 +625,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=92186babdf20cea0 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=af5cda2d6ca0e23b input=a9049054013a1b77]*/ From 125d25317cc6069781cb437f2d7114403e1d7dc2 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 18:11:38 -0800 Subject: [PATCH 26/51] Make deque.__sizeof__ thread-safe --- Modules/_collectionsmodule.c | 3 ++- Modules/clinic/_collectionsmodule.c.h | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index ec887192c03021..231122bd9014f7 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1792,6 +1792,7 @@ _collections_deque___init___impl(dequeobject *deque, PyObject *iterable, } /*[clinic input] +@critical_section _collections.deque.__sizeof__ deque: dequeobject @@ -1801,7 +1802,7 @@ Return the size of the deque in memory, in bytes static PyObject * _collections_deque___sizeof___impl(dequeobject *deque) -/*[clinic end generated code: output=1a66234430a294a3 input=c0c535e64766f446]*/ +/*[clinic end generated code: output=1a66234430a294a3 input=88e45a563a92ab4d]*/ { size_t res = _PyObject_SIZE(Py_TYPE(deque)); size_t blocks; diff --git a/Modules/clinic/_collectionsmodule.c.h b/Modules/clinic/_collectionsmodule.c.h index 4660d2b186ea98..2c7e3a625c3671 100644 --- a/Modules/clinic/_collectionsmodule.c.h +++ b/Modules/clinic/_collectionsmodule.c.h @@ -537,7 +537,13 @@ _collections_deque___sizeof___impl(dequeobject *deque); static PyObject * _collections_deque___sizeof__(dequeobject *deque, PyObject *Py_UNUSED(ignored)) { - return _collections_deque___sizeof___impl(deque); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(deque); + return_value = _collections_deque___sizeof___impl(deque); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_collections_deque___reversed____doc__, @@ -625,4 +631,4 @@ tuplegetter_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=af5cda2d6ca0e23b input=a9049054013a1b77]*/ +/*[clinic end generated code: output=a34c1f2f2c576c0b input=a9049054013a1b77]*/ From 7cea52d533dfdea76ea7bc86a6071f1579f88f68 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 20:33:54 -0800 Subject: [PATCH 27/51] Remove critical section around deque's tp_clear Clear is used only by the GC; no other threads are running. --- Modules/_collectionsmodule.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 231122bd9014f7..f080cc98fa00ab 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -816,15 +816,6 @@ deque_clear(dequeobject *deque) return 0; } -static int -deque_tp_clear(dequeobject *deque) -{ - Py_BEGIN_CRITICAL_SECTION(deque); - int result = deque_clear(deque); - Py_END_CRITICAL_SECTION(); - return result; -} - /*[clinic input] @critical_section _collections.deque.clear @@ -1884,7 +1875,7 @@ static PyType_Slot deque_slots[] = { {Py_tp_getattro, PyObject_GenericGetAttr}, {Py_tp_doc, (void *)_collections_deque___init____doc__}, {Py_tp_traverse, deque_traverse}, - {Py_tp_clear, deque_tp_clear}, + {Py_tp_clear, deque_clear}, {Py_tp_richcompare, deque_richcompare}, {Py_tp_iter, deque_iter}, {Py_tp_getset, deque_getset}, From f0f86d283d5c40892919a532418f054dc569545c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 20:41:39 -0800 Subject: [PATCH 28/51] Make dequeiter ctor thread-safe --- Modules/_collectionsmodule.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index f080cc98fa00ab..bd212339ae3e4a 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1923,15 +1923,19 @@ deque_iter(dequeobject *deque) { dequeiterobject *it; + Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); it = PyObject_GC_New(dequeiterobject, state->dequeiter_type); - if (it == NULL) + if (it == NULL) { + Py_END_CRITICAL_SECTION(); return NULL; + } it->b = deque->leftblock; it->index = deque->leftindex; it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); + Py_END_CRITICAL_SECTION(); PyObject_GC_Track(it); return (PyObject *)it; } From 02efe9b536127da9d1f94905bb9b6f3b668a6000 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 20:51:40 -0800 Subject: [PATCH 29/51] Make dequeiter_next thread-safe --- Modules/_collectionsmodule.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index bd212339ae3e4a..55ef49fa4395c3 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1971,14 +1971,22 @@ dequeiter_next(dequeiterobject *it) { PyObject *item; - if (it->deque->state != it->state) { + Py_BEGIN_CRITICAL_SECTION(it); + dequeobject *deque = it->deque; + Py_END_CRITICAL_SECTION(); + + Py_BEGIN_CRITICAL_SECTION2(it, deque); + if (it->deque != deque || it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, "deque mutated during iteration"); + Py_END_CRITICAL_SECTION2(); return NULL; } - if (it->counter == 0) + if (it->counter == 0) { + Py_END_CRITICAL_SECTION2(); return NULL; + } assert (!(it->b == it->deque->rightblock && it->index > it->deque->rightindex)); @@ -1990,6 +1998,7 @@ dequeiter_next(dequeiterobject *it) it->b = it->b->rightlink; it->index = 0; } + Py_END_CRITICAL_SECTION2(); return Py_NewRef(item); } From 99ffbbae54d89e5fc6edefad6dd3af09c856445c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 20:57:25 -0800 Subject: [PATCH 30/51] Make dequiter_len thread-safe --- Modules/_collectionsmodule.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 55ef49fa4395c3..5fcc7d8cec4199 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2022,6 +2022,11 @@ dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (item) { Py_DECREF(item); } else { + /* + * It's safe to read directly from it without acquiring the + * per-object lock; the iterator isn't visible to any other threads + * yet. + */ if (it->counter) { Py_DECREF(it); return NULL; @@ -2035,7 +2040,8 @@ dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) static PyObject * dequeiter_len(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { - return PyLong_FromSsize_t(it->counter); + Py_ssize_t len = _Py_atomic_load_ssize(&it->counter); + return PyLong_FromSsize_t(len); } PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list(it))."); From edb6ffa2e6037d461ff2a81a88f4f0d14c359be3 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 20:59:31 -0800 Subject: [PATCH 31/51] Make dequeiter_reduce thread-safe --- Modules/_collectionsmodule.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5fcc7d8cec4199..2ce7d65fb7d4b9 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2049,7 +2049,13 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * dequeiter_reduce(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { - return Py_BuildValue("O(On)", Py_TYPE(it), it->deque, Py_SIZE(it->deque) - it->counter); + Py_BEGIN_CRITICAL_SECTION(it); + PyTypeObject *ty = Py_TYPE(it); + dequeobject *deque = it->deque; + Py_ssize_t size = Py_SIZE(deque); + Py_ssize_t counter = it->counter; + Py_END_CRITICAL_SECTION(); + return Py_BuildValue("O(On)", ty, deque, size - counter); } static PyMethodDef dequeiter_methods[] = { From 0beaf7279f3c7fcbd0fb69bf1565c431b6c44c27 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 21:27:13 -0800 Subject: [PATCH 32/51] Fix usage of critical section --- Modules/_collectionsmodule.c | 89 +++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 2ce7d65fb7d4b9..3785accce72148 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -699,16 +699,14 @@ _collections_deque___copy___impl(dequeobject *deque) } static PyObject * -deque_concat(dequeobject *deque, PyObject *other) +deque_concat_locked(dequeobject *deque, PyObject *other) { PyObject *new_deque, *result; int rv; - Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); rv = PyObject_IsInstance(other, (PyObject *)state->deque_type); if (rv <= 0) { - Py_END_CRITICAL_SECTION(); if (rv == 0) { PyErr_Format(PyExc_TypeError, "can only concatenate deque (not \"%.200s\") to deque", @@ -718,9 +716,9 @@ deque_concat(dequeobject *deque, PyObject *other) } new_deque = _collections_deque_copy_impl(deque); - Py_END_CRITICAL_SECTION(); if (new_deque == NULL) return NULL; + // It's safe to not acquire the per-object lock for new_deque; it's // invisible to other threads. result = _collections_deque_extend_impl((dequeobject *)new_deque, other); @@ -732,6 +730,16 @@ deque_concat(dequeobject *deque, PyObject *other) return new_deque; } +static PyObject * +deque_concat(dequeobject *deque, PyObject *other) +{ + PyObject *result = NULL; + Py_BEGIN_CRITICAL_SECTION(deque); + result = deque_concat_locked(deque, other); + Py_END_CRITICAL_SECTION(); + return result; +} + static int deque_clear(dequeobject *deque) { @@ -913,8 +921,9 @@ deque_inplace_repeat_locked(dequeobject *deque, Py_ssize_t n) static PyObject * deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) { + PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - PyObject *result = deque_inplace_repeat_locked(deque, n); + result = deque_inplace_repeat_locked(deque, n); Py_END_CRITICAL_SECTION(); return result; } @@ -1237,8 +1246,9 @@ deque_contains_locked(dequeobject *deque, PyObject *v) static int deque_contains(dequeobject *deque, PyObject *v) { + int result = -1; Py_BEGIN_CRITICAL_SECTION(deque); - int result = deque_contains_locked(deque, v); + result = deque_contains_locked(deque, v); Py_END_CRITICAL_SECTION(); return result; } @@ -1428,8 +1438,9 @@ deque_item_locked(dequeobject *deque, Py_ssize_t i) static PyObject * deque_item(dequeobject *deque, Py_ssize_t i) { + PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - PyObject *result = deque_item_locked(deque, i); + result = deque_item_locked(deque, i); Py_END_CRITICAL_SECTION(); return result; } @@ -1538,8 +1549,9 @@ deque_ass_item_locked(dequeobject *deque, Py_ssize_t i, PyObject *v) static int deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) { + int result = -1; Py_BEGIN_CRITICAL_SECTION(deque); - int result = deque_ass_item_locked(deque, i, v); + result = deque_ass_item_locked(deque, i, v); Py_END_CRITICAL_SECTION(); return result; } @@ -1618,9 +1630,11 @@ _collections_deque___reduce___impl(dequeobject *deque) return NULL; } + Py_ssize_t maxlen; + PyTypeObject *typ = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - Py_ssize_t maxlen = deque->maxlen; - PyTypeObject *typ = Py_TYPE(deque); + maxlen = deque->maxlen; + typ = Py_TYPE(deque); Py_END_CRITICAL_SECTION(); if (maxlen < 0) { @@ -1927,16 +1941,18 @@ deque_iter(dequeobject *deque) collections_state *state = find_module_state_by_def(Py_TYPE(deque)); it = PyObject_GC_New(dequeiterobject, state->dequeiter_type); if (it == NULL) { - Py_END_CRITICAL_SECTION(); - return NULL; + goto done; } it->b = deque->leftblock; it->index = deque->leftindex; it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); +done: Py_END_CRITICAL_SECTION(); - PyObject_GC_Track(it); + if (it != NULL) { + PyObject_GC_Track(it); + } return (PyObject *)it; } @@ -1967,30 +1983,20 @@ dequeiter_dealloc(dequeiterobject *dio) } static PyObject * -dequeiter_next(dequeiterobject *it) +dequeiter_next_locked(dequeiterobject *it, dequeobject *deque) { - PyObject *item; - - Py_BEGIN_CRITICAL_SECTION(it); - dequeobject *deque = it->deque; - Py_END_CRITICAL_SECTION(); - - Py_BEGIN_CRITICAL_SECTION2(it, deque); if (it->deque != deque || it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, "deque mutated during iteration"); - Py_END_CRITICAL_SECTION2(); return NULL; } - if (it->counter == 0) { - Py_END_CRITICAL_SECTION2(); + if (it->counter == 0) return NULL; - } assert (!(it->b == it->deque->rightblock && it->index > it->deque->rightindex)); - item = it->b->data[it->index]; + PyObject *item = it->b->data[it->index]; it->index++; it->counter--; if (it->index == BLOCKLEN && it->counter > 0) { @@ -1998,10 +2004,26 @@ dequeiter_next(dequeiterobject *it) it->b = it->b->rightlink; it->index = 0; } - Py_END_CRITICAL_SECTION2(); return Py_NewRef(item); } +static PyObject * +dequeiter_next(dequeiterobject *it) +{ + + dequeobject *deque = NULL; + Py_BEGIN_CRITICAL_SECTION(it); + deque = it->deque; + Py_END_CRITICAL_SECTION(); + + PyObject *result = NULL; + Py_BEGIN_CRITICAL_SECTION2(it, deque); + result = dequeiter_next_locked(it, deque); + Py_END_CRITICAL_SECTION2(); + + return result; +} + static PyObject * dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { @@ -2049,11 +2071,14 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * dequeiter_reduce(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { + PyTypeObject *ty = NULL; + dequeobject *deque = NULL; + Py_ssize_t size, counter; Py_BEGIN_CRITICAL_SECTION(it); - PyTypeObject *ty = Py_TYPE(it); - dequeobject *deque = it->deque; - Py_ssize_t size = Py_SIZE(deque); - Py_ssize_t counter = it->counter; + ty = Py_TYPE(it); + deque = it->deque; + size = Py_SIZE(deque); + counter = it->counter; Py_END_CRITICAL_SECTION(); return Py_BuildValue("O(On)", ty, deque, size - counter); } @@ -2090,6 +2115,7 @@ static PyObject * deque_reviter(dequeobject *deque) { dequeiterobject *it; + Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); it = PyObject_GC_New(dequeiterobject, state->dequereviter_type); @@ -2100,6 +2126,7 @@ deque_reviter(dequeobject *deque) it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); + Py_END_CRITICAL_SECTION(); PyObject_GC_Track(it); return (PyObject *)it; } From d9729572a8d80c6ea7769cb970502bcaed504a23 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 5 Jan 2024 21:34:54 -0800 Subject: [PATCH 33/51] Make dequereviter_next thread-safe --- Modules/_collectionsmodule.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 3785accce72148..0b0da925a4d0e6 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2132,13 +2132,9 @@ deque_reviter(dequeobject *deque) } static PyObject * -dequereviter_next(dequeiterobject *it) +dequereviter_next_locked(dequeiterobject *it, dequeobject *deque) { - PyObject *item; - if (it->counter == 0) - return NULL; - - if (it->deque->state != it->state) { + if (it->deque != deque || it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, "deque mutated during iteration"); @@ -2147,7 +2143,7 @@ dequereviter_next(dequeiterobject *it) assert (!(it->b == it->deque->leftblock && it->index < it->deque->leftindex)); - item = it->b->data[it->index]; + PyObject *item = it->b->data[it->index]; it->index--; it->counter--; if (it->index < 0 && it->counter > 0) { @@ -2158,6 +2154,28 @@ dequereviter_next(dequeiterobject *it) return Py_NewRef(item); } +static PyObject * +dequereviter_next(dequeiterobject *it) +{ + dequeobject *deque = NULL; + Py_ssize_t counter; + + Py_BEGIN_CRITICAL_SECTION(it); + counter = it->counter; + deque = it->deque; + Py_END_CRITICAL_SECTION(); + + if (counter == 0) + return NULL; + + PyObject *item = NULL; + Py_BEGIN_CRITICAL_SECTION2(it, deque); + item = dequereviter_next_locked(it, deque); + Py_END_CRITICAL_SECTION2(); + + return item; +} + static PyObject * dequereviter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { From ac2ee0e71e8678a01dfe05eee3b88f38c4e596f0 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 8 Jan 2024 10:11:05 -0800 Subject: [PATCH 34/51] Detail why we don't need to lock in new --- Modules/_collectionsmodule.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 0b0da925a4d0e6..82b4d32c82fb53 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2044,11 +2044,11 @@ dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (item) { Py_DECREF(item); } else { - /* - * It's safe to read directly from it without acquiring the - * per-object lock; the iterator isn't visible to any other threads - * yet. - */ + /* + * It's safe to read directly from it without acquiring the + * per-object lock; the iterator isn't visible to any other threads + * yet. + */ if (it->counter) { Py_DECREF(it); return NULL; @@ -2196,6 +2196,11 @@ dequereviter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (item) { Py_DECREF(item); } else { + /* + * It's safe to read directly from it without acquiring the + * per-object lock; the iterator isn't visible to any other threads + * yet. + */ if (it->counter) { Py_DECREF(it); return NULL; From ce6861368538306a097dc54744411e1765226506 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 8 Jan 2024 10:25:41 -0800 Subject: [PATCH 35/51] Formatting fixes for PEP-007 --- Modules/_collectionsmodule.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 82b4d32c82fb53..11bc6dedc2aedc 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -231,7 +231,6 @@ deque_new(PyTypeObject *type, PyObject *args, PyObject *kwds) static PyObject * deque_pop_locked(dequeobject *deque) { - PyObject *item; block *prevblock; @@ -344,7 +343,8 @@ _collections_deque_popleft_impl(dequeobject *deque) * unsigned test that returns true whenever 0 <= maxlen < Py_SIZE(deque). */ -#define NEEDS_TRIM(deque) ((size_t)((deque)->maxlen) < (size_t)(Py_SIZE(deque))) +#define NEEDS_TRIM(deque) \ + ((size_t)((deque)->maxlen) < (size_t)(Py_SIZE(deque))) typedef struct { // 1 if an error occurred, 0 otherwise @@ -377,7 +377,8 @@ deque_append_locked(dequeobject *deque, PyObject *item) if (NEEDS_TRIM(deque)) { PyObject *trimmed = deque_popleft_locked(deque); return (AppendResult){0, trimmed}; - } else { + } + else { deque->state++; return (AppendResult){0, NULL}; } @@ -428,7 +429,8 @@ deque_appendleft_locked(dequeobject *deque, PyObject *item) if (NEEDS_TRIM(deque)) { PyObject *trimmed = deque_pop_locked(deque); return (AppendResult){0, trimmed}; - } else { + } + else { deque->state++; return (AppendResult){0, NULL}; } @@ -625,7 +627,6 @@ deque_inplace_concat(dequeobject *deque, PyObject *other) return (PyObject *)deque; } - /*[clinic input] @critical_section _collections.deque.copy @@ -660,7 +661,7 @@ _collections_deque_copy_impl(dequeobject *deque) PyObject *item = old_deque->leftblock->data[old_deque->leftindex]; rv = _collections_deque_append_impl(new_deque, item); } else { - rv = _collections_deque_extend_impl(new_deque, (PyObject *) deque); + rv = _collections_deque_extend_impl(new_deque, (PyObject *)deque); } if (rv != NULL) { Py_DECREF(rv); @@ -670,7 +671,8 @@ _collections_deque_copy_impl(dequeobject *deque) return NULL; } if (old_deque->maxlen < 0) - result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)), (PyObject *) deque); + result = PyObject_CallOneArg((PyObject *)(Py_TYPE(deque)), + (PyObject *)deque); else result = PyObject_CallFunction((PyObject *)(Py_TYPE(deque)), "Oi", deque, old_deque->maxlen, NULL); @@ -935,7 +937,7 @@ deque_repeat(dequeobject *deque, Py_ssize_t n) PyObject *rv; Py_BEGIN_CRITICAL_SECTION(deque); - new_deque = (dequeobject *) _collections_deque_copy_impl(deque); + new_deque = (dequeobject *)_collections_deque_copy_impl(deque); Py_END_CRITICAL_SECTION(); if (new_deque == NULL) return NULL; @@ -1826,7 +1828,8 @@ deque_get_maxlen(dequeobject *deque, void *Py_UNUSED(ignored)) return PyLong_FromSsize_t(deque->maxlen); } -static PyObject *deque_reviter(dequeobject *deque); +static PyObject * +deque_reviter(dequeobject *deque); /*[clinic input] _collections.deque.__reversed__ @@ -2010,7 +2013,6 @@ dequeiter_next_locked(dequeiterobject *it, dequeobject *deque) static PyObject * dequeiter_next(dequeiterobject *it) { - dequeobject *deque = NULL; Py_BEGIN_CRITICAL_SECTION(it); deque = it->deque; @@ -2187,7 +2189,7 @@ dequereviter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; assert(type == state->dequereviter_type); - it = (dequeiterobject*)deque_reviter((dequeobject *)deque); + it = (dequeiterobject *)deque_reviter((dequeobject *)deque); if (!it) return NULL; /* consume items from the queue */ From 17d19da2cf62e452a2da4717741f54b464e70607 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 8 Jan 2024 21:57:42 +0000 Subject: [PATCH 36/51] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst new file mode 100644 index 00000000000000..d55a8aaa797978 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst @@ -0,0 +1 @@ +Make methods on `collections.deque` thread-safe when the GIL is disabled. When the GIL is disabled, methods that operate a deque's internal state will do so only while holding the object's lock. From db02d54f87a1767caa37ea5ca4db1a5cba858942 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 8 Jan 2024 14:06:22 -0800 Subject: [PATCH 37/51] Fix formatting of NEWS entry --- .../2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst index d55a8aaa797978..26de1ef460eb1d 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst @@ -1 +1 @@ -Make methods on `collections.deque` thread-safe when the GIL is disabled. When the GIL is disabled, methods that operate a deque's internal state will do so only while holding the object's lock. +Make methods on ``collections.deque`` thread-safe when the GIL is disabled. When the GIL is disabled, methods that operate a deque's internal state will do so only while holding the object's lock. From 6e68cadae6cb4753a72f882b7e9309738d795e99 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 16:02:20 -0800 Subject: [PATCH 38/51] Use `_lock_held` suffix for functions that require the per-object lock --- Modules/_collectionsmodule.c | 42 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 2e1fadf819241b..a45c1098771171 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -344,7 +344,7 @@ typedef struct { } AppendResult; static inline AppendResult -deque_append_locked(dequeobject *deque, PyObject *item) +deque_append_lock_held(dequeobject *deque, PyObject *item) { if (deque->rightindex == BLOCKLEN - 1) { block *b = newblock(deque); @@ -387,7 +387,7 @@ static PyObject * deque_append_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9c7bcb8b599c6362 input=b0eeeb09b9f5cf18]*/ { - AppendResult res = deque_append_locked(deque, item); + AppendResult res = deque_append_lock_held(deque, item); Py_XDECREF(res.trimmed); if (res.err) { return NULL; @@ -396,7 +396,7 @@ deque_append_impl(dequeobject *deque, PyObject *item) } static inline AppendResult -deque_appendleft_locked(dequeobject *deque, PyObject *item) +deque_appendleft_lock_held(dequeobject *deque, PyObject *item) { if (deque->leftindex == 0) { block *b = newblock(deque); @@ -439,7 +439,7 @@ static PyObject * deque_appendleft_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9a192edbcd0f20db input=236c2fbceaf08e14]*/ { - AppendResult res = deque_appendleft_locked(deque, item); + AppendResult res = deque_appendleft_lock_held(deque, item); Py_XDECREF(res.trimmed); if (res.err) { return NULL; @@ -524,7 +524,7 @@ deque_extend_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - AppendResult res = deque_append_locked(deque, item); + AppendResult res = deque_append_lock_held(deque, item); Py_DECREF(item); Py_XDECREF(res.trimmed); if (res.err) { @@ -586,7 +586,7 @@ deque_extendleft_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - AppendResult res = deque_appendleft_locked(deque, item); + AppendResult res = deque_appendleft_lock_held(deque, item); Py_DECREF(item); Py_XDECREF(res.trimmed); if (res.err) { @@ -689,7 +689,7 @@ deque___copy___impl(dequeobject *deque) } static PyObject * -deque_concat_locked(dequeobject *deque, PyObject *other) +deque_concat_lock_held(dequeobject *deque, PyObject *other) { PyObject *new_deque, *result; int rv; @@ -725,7 +725,7 @@ deque_concat(dequeobject *deque, PyObject *other) { PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - result = deque_concat_locked(deque, other); + result = deque_concat_lock_held(deque, other); Py_END_CRITICAL_SECTION(); return result; } @@ -832,7 +832,7 @@ deque_clearmethod_impl(dequeobject *deque) } static PyObject * -deque_inplace_repeat_locked(dequeobject *deque, Py_ssize_t n) +deque_inplace_repeat_lock_held(dequeobject *deque, Py_ssize_t n) { Py_ssize_t i, m, size; PyObject *seq; @@ -913,7 +913,7 @@ deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) { PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - result = deque_inplace_repeat_locked(deque, n); + result = deque_inplace_repeat_lock_held(deque, n); Py_END_CRITICAL_SECTION(); return result; } @@ -931,7 +931,7 @@ deque_repeat(dequeobject *deque, Py_ssize_t n) return NULL; // It's safe to not acquire the per-object lock for new_deque; it's // invisible to other threads. - rv = deque_inplace_repeat_locked(new_deque, n); + rv = deque_inplace_repeat_lock_held(new_deque, n); Py_DECREF(new_deque); return rv; } @@ -1202,7 +1202,7 @@ deque_count_impl(dequeobject *deque, PyObject *v) } static int -deque_contains_locked(dequeobject *deque, PyObject *v) +deque_contains_lock_held(dequeobject *deque, PyObject *v) { block *b = deque->leftblock; Py_ssize_t index = deque->leftindex; @@ -1238,7 +1238,7 @@ deque_contains(dequeobject *deque, PyObject *v) { int result = -1; Py_BEGIN_CRITICAL_SECTION(deque); - result = deque_contains_locked(deque, v); + result = deque_contains_lock_held(deque, v); Py_END_CRITICAL_SECTION(); return result; } @@ -1386,7 +1386,7 @@ valid_index(Py_ssize_t i, Py_ssize_t limit) } static PyObject * -deque_item_locked(dequeobject *deque, Py_ssize_t i) +deque_item_lock_held(dequeobject *deque, Py_ssize_t i) { block *b; PyObject *item; @@ -1429,7 +1429,7 @@ deque_item(dequeobject *deque, Py_ssize_t i) { PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION(deque); - result = deque_item_locked(deque, i); + result = deque_item_lock_held(deque, i); Py_END_CRITICAL_SECTION(); return result; } @@ -1504,7 +1504,7 @@ deque_remove_impl(dequeobject *deque, PyObject *value) } static int -deque_ass_item_locked(dequeobject *deque, Py_ssize_t i, PyObject *v) +deque_ass_item_lock_held(dequeobject *deque, Py_ssize_t i, PyObject *v) { block *b; Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i; @@ -1540,7 +1540,7 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) { int result = -1; Py_BEGIN_CRITICAL_SECTION(deque); - result = deque_ass_item_locked(deque, i, v); + result = deque_ass_item_lock_held(deque, i, v); Py_END_CRITICAL_SECTION(); return result; } @@ -1970,7 +1970,7 @@ dequeiter_dealloc(dequeiterobject *dio) } static PyObject * -dequeiter_next_locked(dequeiterobject *it, dequeobject *deque) +dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) { if (it->deque != deque || it->deque->state != it->state) { it->counter = 0; @@ -2004,7 +2004,7 @@ dequeiter_next(dequeiterobject *it) PyObject *result = NULL; Py_BEGIN_CRITICAL_SECTION2(it, deque); - result = dequeiter_next_locked(it, deque); + result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); return result; @@ -2118,7 +2118,7 @@ deque_reviter(dequeobject *deque) } static PyObject * -dequereviter_next_locked(dequeiterobject *it, dequeobject *deque) +dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) { if (it->deque != deque || it->deque->state != it->state) { it->counter = 0; @@ -2156,7 +2156,7 @@ dequereviter_next(dequeiterobject *it) PyObject *item = NULL; Py_BEGIN_CRITICAL_SECTION2(it, deque); - item = dequereviter_next_locked(it, deque); + item = dequereviter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); return item; From 72ea9eb81fd961ecf9380bbb37e51d48d5452629 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 16:08:20 -0800 Subject: [PATCH 39/51] Shorten NEWS entry --- .../2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst index 26de1ef460eb1d..db52c5a8ee913a 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst @@ -1 +1 @@ -Make methods on ``collections.deque`` thread-safe when the GIL is disabled. When the GIL is disabled, methods that operate a deque's internal state will do so only while holding the object's lock. +Make methods on ``collections.deque`` thread-safe when the GIL is disabled. From 9f80af829a707602c4434a6edb4fbc0c5781e80d Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 20:20:34 -0800 Subject: [PATCH 40/51] Revert changes to append helpers to minimize churn --- Modules/_collectionsmodule.c | 59 +++++++++++------------------------- 1 file changed, 18 insertions(+), 41 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index a45c1098771171..491008ccf14e32 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -334,23 +334,13 @@ deque_popleft_impl(dequeobject *deque) #define NEEDS_TRIM(deque) \ ((size_t)((deque)->maxlen) < (size_t)(Py_SIZE(deque))) -typedef struct { - // 1 if an error occurred, 0 otherwise - int err; - - // A value that was trimmed if the operation would exceed maxlen, NULL - // otherwise - PyObject *trimmed; -} AppendResult; - -static inline AppendResult +static inline int deque_append_lock_held(dequeobject *deque, PyObject *item) { if (deque->rightindex == BLOCKLEN - 1) { block *b = newblock(deque); - if (b == NULL) { - return (AppendResult){1, NULL}; - } + if (b == NULL) + return -1; b->leftlink = deque->rightblock; CHECK_END(deque->rightblock->rightlink); deque->rightblock->rightlink = b; @@ -360,16 +350,15 @@ deque_append_lock_held(dequeobject *deque, PyObject *item) } Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->rightindex++; - Py_INCREF(item); deque->rightblock->data[deque->rightindex] = item; if (NEEDS_TRIM(deque)) { - PyObject *trimmed = deque_popleft_impl(deque); - return (AppendResult){0, trimmed}; + PyObject *olditem = deque_popleft_impl(deque); + Py_DECREF(olditem); } else { deque->state++; - return (AppendResult){0, NULL}; } + return 0; } /*[clinic input] @@ -387,22 +376,18 @@ static PyObject * deque_append_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9c7bcb8b599c6362 input=b0eeeb09b9f5cf18]*/ { - AppendResult res = deque_append_lock_held(deque, item); - Py_XDECREF(res.trimmed); - if (res.err) { + if (deque_append_lock_held(deque, Py_NewRef(item)) < 0) return NULL; - } Py_RETURN_NONE; } -static inline AppendResult +static inline int deque_appendleft_lock_held(dequeobject *deque, PyObject *item) { if (deque->leftindex == 0) { block *b = newblock(deque); - if (b == NULL) { - return (AppendResult){1, NULL}; - } + if (b == NULL) + return -1; b->rightlink = deque->leftblock; CHECK_END(deque->leftblock->leftlink); deque->leftblock->leftlink = b; @@ -412,16 +397,15 @@ deque_appendleft_lock_held(dequeobject *deque, PyObject *item) } Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->leftindex--; - Py_INCREF(item); deque->leftblock->data[deque->leftindex] = item; if (NEEDS_TRIM(deque)) { - PyObject *trimmed = deque_pop_impl(deque); - return (AppendResult){0, trimmed}; + PyObject *olditem = deque_pop_impl(deque); + Py_DECREF(olditem); } else { deque->state++; - return (AppendResult){0, NULL}; } + return 0; } /*[clinic input] @@ -439,11 +423,8 @@ static PyObject * deque_appendleft_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9a192edbcd0f20db input=236c2fbceaf08e14]*/ { - AppendResult res = deque_appendleft_lock_held(deque, item); - Py_XDECREF(res.trimmed); - if (res.err) { + if (deque_appendleft_lock_held(deque, Py_NewRef(item)) < 0) return NULL; - } Py_RETURN_NONE; } @@ -524,10 +505,8 @@ deque_extend_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - AppendResult res = deque_append_lock_held(deque, item); - Py_DECREF(item); - Py_XDECREF(res.trimmed); - if (res.err) { + if (deque_append_lock_held(deque, item) == -1) { + Py_DECREF(item); Py_DECREF(it); goto done; } @@ -586,10 +565,8 @@ deque_extendleft_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - AppendResult res = deque_appendleft_lock_held(deque, item); - Py_DECREF(item); - Py_XDECREF(res.trimmed); - if (res.err) { + if (deque_appendleft_lock_held(deque, item) == -1) { + Py_DECREF(item); Py_DECREF(it); goto done; } From fc91a8331e35d6d56d00401a5cf5fa042619bacb Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 21:00:44 -0800 Subject: [PATCH 41/51] Use recursive impl for extendleft and extend --- Modules/_collectionsmodule.c | 56 ++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 491008ccf14e32..5d82b047d11da1 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -475,25 +475,25 @@ deque_extend_impl(dequeobject *deque, PyObject *iterable) { PyObject *it, *item; PyObject *(*iternext)(PyObject *); - PyObject *list = NULL; - PyObject *result = NULL; + Py_ssize_t maxlen = deque->maxlen; /* Handle case where id(deque) == id(iterable) */ if ((PyObject *)deque == iterable) { - list = PySequence_List(iterable); - if (list == NULL) + PyObject *result; + PyObject *s = PySequence_List(iterable); + if (s == NULL) return NULL; - iterable = list; + result = deque_extend(deque, s); + Py_DECREF(s); + return result; } it = PyObject_GetIter(iterable); if (it == NULL) - goto done; + return NULL; - if (deque->maxlen == 0) { - result = consume_iterator(it); - goto done; - } + if (maxlen == 0) + return consume_iterator(it); /* Space saving heuristic. Start filling from the left */ if (Py_SIZE(deque) == 0) { @@ -508,14 +508,10 @@ deque_extend_impl(dequeobject *deque, PyObject *iterable) if (deque_append_lock_held(deque, item) == -1) { Py_DECREF(item); Py_DECREF(it); - goto done; + return NULL; } } - result = finalize_iterator(it); - -done: - Py_XDECREF(list); - return result; + return finalize_iterator(it); } /*[clinic input] @@ -535,25 +531,25 @@ deque_extendleft_impl(dequeobject *deque, PyObject *iterable) { PyObject *it, *item; PyObject *(*iternext)(PyObject *); - PyObject *list = NULL; - PyObject *result = NULL; + Py_ssize_t maxlen = deque->maxlen; /* Handle case where id(deque) == id(iterable) */ if ((PyObject *)deque == iterable) { - list = PySequence_List(iterable); - if (list == NULL) + PyObject *result; + PyObject *s = PySequence_List(iterable); + if (s == NULL) return NULL; - iterable = list; + result = deque_extendleft_impl(deque, s); + Py_DECREF(s); + return result; } it = PyObject_GetIter(iterable); if (it == NULL) - goto done; + return NULL; - if (deque->maxlen == 0) { - result = consume_iterator(it); - goto done; - } + if (maxlen == 0) + return consume_iterator(it); /* Space saving heuristic. Start filling from the right */ if (Py_SIZE(deque) == 0) { @@ -568,14 +564,10 @@ deque_extendleft_impl(dequeobject *deque, PyObject *iterable) if (deque_appendleft_lock_held(deque, item) == -1) { Py_DECREF(item); Py_DECREF(it); - goto done; + return NULL; } } - result = finalize_iterator(it); - -done: - Py_XDECREF(list); - return result; + return finalize_iterator(it); } static PyObject * From 7d65ac53c68021a3b62917c8ac20c3ae5f86807a Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 21:04:52 -0800 Subject: [PATCH 42/51] Fix critical section --- Modules/_collectionsmodule.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5d82b047d11da1..fe9b645a778f1e 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2065,27 +2065,34 @@ static PyType_Spec dequeiter_spec = { }; /*********************** Deque Reverse Iterator **************************/ - static PyObject * -deque_reviter(dequeobject *deque) +deque_reviter_lock_held(dequeobject *deque) { - dequeiterobject *it; - Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); - - it = PyObject_GC_New(dequeiterobject, state->dequereviter_type); - if (it == NULL) + dequeiterobject *it = + PyObject_GC_New(dequeiterobject, state->dequereviter_type); + if (it == NULL) { return NULL; + } it->b = deque->rightblock; it->index = deque->rightindex; it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); - Py_END_CRITICAL_SECTION(); PyObject_GC_Track(it); return (PyObject *)it; } +static PyObject * +deque_reviter(dequeobject *deque) +{ + PyObject *it = NULL; + Py_BEGIN_CRITICAL_SECTION(deque); + it = deque_reviter_lock_held(deque); + Py_END_CRITICAL_SECTION(); + return it; +} + static PyObject * dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) { From 8072617d92ba836f05bfb4726e5299012f566d0c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 29 Jan 2024 22:09:22 -0800 Subject: [PATCH 43/51] Revert NEEDS_TRIM refactor Further minimize changes --- Modules/_collectionsmodule.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index fe9b645a778f1e..d3b47da6777f3a 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -331,11 +331,10 @@ deque_popleft_impl(dequeobject *deque) * unsigned test that returns true whenever 0 <= maxlen < Py_SIZE(deque). */ -#define NEEDS_TRIM(deque) \ - ((size_t)((deque)->maxlen) < (size_t)(Py_SIZE(deque))) +#define NEEDS_TRIM(deque, maxlen) ((size_t)(maxlen) < (size_t)(Py_SIZE(deque))) static inline int -deque_append_lock_held(dequeobject *deque, PyObject *item) +deque_append_lock_held(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) { if (deque->rightindex == BLOCKLEN - 1) { block *b = newblock(deque); @@ -351,7 +350,7 @@ deque_append_lock_held(dequeobject *deque, PyObject *item) Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->rightindex++; deque->rightblock->data[deque->rightindex] = item; - if (NEEDS_TRIM(deque)) { + if (NEEDS_TRIM(deque, maxlen)) { PyObject *olditem = deque_popleft_impl(deque); Py_DECREF(olditem); } @@ -376,13 +375,14 @@ static PyObject * deque_append_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9c7bcb8b599c6362 input=b0eeeb09b9f5cf18]*/ { - if (deque_append_lock_held(deque, Py_NewRef(item)) < 0) + if (deque_append_lock_held(deque, Py_NewRef(item), deque->maxlen) < 0) return NULL; Py_RETURN_NONE; } static inline int -deque_appendleft_lock_held(dequeobject *deque, PyObject *item) +deque_appendleft_lock_held(dequeobject *deque, PyObject *item, + Py_ssize_t maxlen) { if (deque->leftindex == 0) { block *b = newblock(deque); @@ -398,7 +398,7 @@ deque_appendleft_lock_held(dequeobject *deque, PyObject *item) Py_SET_SIZE(deque, Py_SIZE(deque) + 1); deque->leftindex--; deque->leftblock->data[deque->leftindex] = item; - if (NEEDS_TRIM(deque)) { + if (NEEDS_TRIM(deque, maxlen)) { PyObject *olditem = deque_pop_impl(deque); Py_DECREF(olditem); } @@ -423,7 +423,7 @@ static PyObject * deque_appendleft_impl(dequeobject *deque, PyObject *item) /*[clinic end generated code: output=9a192edbcd0f20db input=236c2fbceaf08e14]*/ { - if (deque_appendleft_lock_held(deque, Py_NewRef(item)) < 0) + if (deque_appendleft_lock_held(deque, Py_NewRef(item), deque->maxlen) < 0) return NULL; Py_RETURN_NONE; } @@ -505,7 +505,7 @@ deque_extend_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - if (deque_append_lock_held(deque, item) == -1) { + if (deque_append_lock_held(deque, item, maxlen) == -1) { Py_DECREF(item); Py_DECREF(it); return NULL; @@ -561,7 +561,7 @@ deque_extendleft_impl(dequeobject *deque, PyObject *iterable) iternext = *Py_TYPE(it)->tp_iternext; while ((item = iternext(it)) != NULL) { - if (deque_appendleft_lock_held(deque, item) == -1) { + if (deque_appendleft_lock_held(deque, item, maxlen) == -1) { Py_DECREF(item); Py_DECREF(it); return NULL; From 672184d974e736ad7f3867aaa823aa337afcc25f Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 30 Jan 2024 09:50:00 -0800 Subject: [PATCH 44/51] Don't need to protect type accesses w/ CS These are protected by stop-the-world pauses when type is re-assigned --- Modules/_collectionsmodule.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index d3b47da6777f3a..2683c59bc82268 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1891,24 +1891,20 @@ typedef struct { static PyObject * deque_iter(dequeobject *deque) { - dequeiterobject *it; - - Py_BEGIN_CRITICAL_SECTION(deque); collections_state *state = find_module_state_by_def(Py_TYPE(deque)); - it = PyObject_GC_New(dequeiterobject, state->dequeiter_type); + dequeiterobject *it = + PyObject_GC_New(dequeiterobject, state->dequeiter_type); if (it == NULL) { - goto done; + return NULL; } + Py_BEGIN_CRITICAL_SECTION(deque); it->b = deque->leftblock; it->index = deque->leftindex; it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); -done: Py_END_CRITICAL_SECTION(); - if (it != NULL) { - PyObject_GC_Track(it); - } + PyObject_GC_Track(it); return (PyObject *)it; } @@ -2065,8 +2061,9 @@ static PyType_Spec dequeiter_spec = { }; /*********************** Deque Reverse Iterator **************************/ + static PyObject * -deque_reviter_lock_held(dequeobject *deque) +deque_reviter(dequeobject *deque) { collections_state *state = find_module_state_by_def(Py_TYPE(deque)); dequeiterobject *it = @@ -2074,25 +2071,17 @@ deque_reviter_lock_held(dequeobject *deque) if (it == NULL) { return NULL; } + Py_BEGIN_CRITICAL_SECTION(deque); it->b = deque->rightblock; it->index = deque->rightindex; it->deque = (dequeobject*)Py_NewRef(deque); it->state = deque->state; it->counter = Py_SIZE(deque); + Py_END_CRITICAL_SECTION(); PyObject_GC_Track(it); return (PyObject *)it; } -static PyObject * -deque_reviter(dequeobject *deque) -{ - PyObject *it = NULL; - Py_BEGIN_CRITICAL_SECTION(deque); - it = deque_reviter_lock_held(deque); - Py_END_CRITICAL_SECTION(); - return it; -} - static PyObject * dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) { From 4461a008a108301f918a154d7d4b7c1ee8e21f46 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 30 Jan 2024 10:19:53 -0800 Subject: [PATCH 45/51] Remove defensive initializations --- Modules/_collectionsmodule.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 2683c59bc82268..0fc094041fb2d0 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -692,7 +692,7 @@ deque_concat_lock_held(dequeobject *deque, PyObject *other) static PyObject * deque_concat(dequeobject *deque, PyObject *other) { - PyObject *result = NULL; + PyObject *result; Py_BEGIN_CRITICAL_SECTION(deque); result = deque_concat_lock_held(deque, other); Py_END_CRITICAL_SECTION(); @@ -880,7 +880,7 @@ deque_inplace_repeat_lock_held(dequeobject *deque, Py_ssize_t n) static PyObject * deque_inplace_repeat(dequeobject *deque, Py_ssize_t n) { - PyObject *result = NULL; + PyObject *result; Py_BEGIN_CRITICAL_SECTION(deque); result = deque_inplace_repeat_lock_held(deque, n); Py_END_CRITICAL_SECTION(); @@ -1205,7 +1205,7 @@ deque_contains_lock_held(dequeobject *deque, PyObject *v) static int deque_contains(dequeobject *deque, PyObject *v) { - int result = -1; + int result; Py_BEGIN_CRITICAL_SECTION(deque); result = deque_contains_lock_held(deque, v); Py_END_CRITICAL_SECTION(); @@ -1396,7 +1396,7 @@ deque_item_lock_held(dequeobject *deque, Py_ssize_t i) static PyObject * deque_item(dequeobject *deque, Py_ssize_t i) { - PyObject *result = NULL; + PyObject *result; Py_BEGIN_CRITICAL_SECTION(deque); result = deque_item_lock_held(deque, i); Py_END_CRITICAL_SECTION(); @@ -1507,7 +1507,7 @@ deque_ass_item_lock_held(dequeobject *deque, Py_ssize_t i, PyObject *v) static int deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) { - int result = -1; + int result; Py_BEGIN_CRITICAL_SECTION(deque); result = deque_ass_item_lock_held(deque, i, v); Py_END_CRITICAL_SECTION(); @@ -1589,7 +1589,7 @@ deque___reduce___impl(dequeobject *deque) } Py_ssize_t maxlen; - PyTypeObject *typ = NULL; + PyTypeObject *typ; Py_BEGIN_CRITICAL_SECTION(deque); maxlen = deque->maxlen; typ = Py_TYPE(deque); @@ -1962,12 +1962,12 @@ dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) static PyObject * dequeiter_next(dequeiterobject *it) { - dequeobject *deque = NULL; + dequeobject *deque; Py_BEGIN_CRITICAL_SECTION(it); deque = it->deque; Py_END_CRITICAL_SECTION(); - PyObject *result = NULL; + PyObject *result; Py_BEGIN_CRITICAL_SECTION2(it, deque); result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); @@ -2022,8 +2022,8 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * dequeiter_reduce(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { - PyTypeObject *ty = NULL; - dequeobject *deque = NULL; + PyTypeObject *ty; + dequeobject *deque; Py_ssize_t size, counter; Py_BEGIN_CRITICAL_SECTION(it); ty = Py_TYPE(it); @@ -2108,7 +2108,7 @@ dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) static PyObject * dequereviter_next(dequeiterobject *it) { - dequeobject *deque = NULL; + dequeobject *deque; Py_ssize_t counter; Py_BEGIN_CRITICAL_SECTION(it); From b6741e3a99a68ec0c078e6d78cd394e8e22a5b3d Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 30 Jan 2024 11:16:15 -0800 Subject: [PATCH 46/51] Relax critsecs accesses that are immutable post construction --- Modules/_collectionsmodule.c | 43 ++++++++++++++---------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 0fc094041fb2d0..fe3c7cd96b63c4 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1588,13 +1588,10 @@ deque___reduce___impl(dequeobject *deque) return NULL; } - Py_ssize_t maxlen; - PyTypeObject *typ; - Py_BEGIN_CRITICAL_SECTION(deque); - maxlen = deque->maxlen; - typ = Py_TYPE(deque); - Py_END_CRITICAL_SECTION(); - + // It's safe to access deque->maxlen here without holding the per object + // lock for deque; deque->maxlen is only assigned during construction. + Py_ssize_t maxlen = deque->maxlen; + PyTypeObject *typ = Py_TYPE(deque); if (maxlen < 0) { return Py_BuildValue("O()NN", typ, state, it); } @@ -1937,7 +1934,7 @@ dequeiter_dealloc(dequeiterobject *dio) static PyObject * dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) { - if (it->deque != deque || it->deque->state != it->state) { + if (it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, "deque mutated during iteration"); @@ -1962,12 +1959,10 @@ dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) static PyObject * dequeiter_next(dequeiterobject *it) { - dequeobject *deque; - Py_BEGIN_CRITICAL_SECTION(it); - deque = it->deque; - Py_END_CRITICAL_SECTION(); - PyObject *result; + // It's safe to access it->deque without holding the per-object lock for it + // here; it->deque is only assigned during construction of it. + dequeobject *deque = it->deque; Py_BEGIN_CRITICAL_SECTION2(it, deque); result = dequeiter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); @@ -2085,7 +2080,10 @@ deque_reviter(dequeobject *deque) static PyObject * dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) { - if (it->deque != deque || it->deque->state != it->state) { + if (it->counter == 0) + return NULL; + + if (it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, "deque mutated during iteration"); @@ -2108,22 +2106,13 @@ dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) static PyObject * dequereviter_next(dequeiterobject *it) { - dequeobject *deque; - Py_ssize_t counter; - - Py_BEGIN_CRITICAL_SECTION(it); - counter = it->counter; - deque = it->deque; - Py_END_CRITICAL_SECTION(); - - if (counter == 0) - return NULL; - - PyObject *item = NULL; + PyObject *item; + // It's safe to access it->deque without holding the per-object lock for it + // here; it->deque is only assigned during construction of it. + dequeobject *deque = it->deque; Py_BEGIN_CRITICAL_SECTION2(it, deque); item = dequereviter_next_lock_held(it, deque); Py_END_CRITICAL_SECTION2(); - return item; } From 0519f722da9e62f9cb9c7890718e777bbdacf16a Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 30 Jan 2024 13:02:19 -0800 Subject: [PATCH 47/51] Fix critsec for dequeiter_reduce Acquire both per-object locks while loading from size and counter --- Modules/_collectionsmodule.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index fe3c7cd96b63c4..40e993aad48e7e 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2017,15 +2017,15 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * dequeiter_reduce(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { - PyTypeObject *ty; - dequeobject *deque; + PyTypeObject *ty = Py_TYPE(it); + // It's safe to access it->deque without holding the per-object lock for it + // here; it->deque is only assigned during construction of it. + dequeobject *deque = it->deque; Py_ssize_t size, counter; - Py_BEGIN_CRITICAL_SECTION(it); - ty = Py_TYPE(it); - deque = it->deque; + Py_BEGIN_CRITICAL_SECTION2(it, deque); size = Py_SIZE(deque); counter = it->counter; - Py_END_CRITICAL_SECTION(); + Py_END_CRITICAL_SECTION2(); return Py_BuildValue("O(On)", ty, deque, size - counter); } From 339dcd9ce9cd3996a836dba66516fbe5245f2467 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 1 Feb 2024 11:19:07 -0800 Subject: [PATCH 48/51] Revert style changes --- Modules/_collectionsmodule.c | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 40e993aad48e7e..dc6a0822efe744 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -353,8 +353,7 @@ deque_append_lock_held(dequeobject *deque, PyObject *item, Py_ssize_t maxlen) if (NEEDS_TRIM(deque, maxlen)) { PyObject *olditem = deque_popleft_impl(deque); Py_DECREF(olditem); - } - else { + } else { deque->state++; } return 0; @@ -401,8 +400,7 @@ deque_appendleft_lock_held(dequeobject *deque, PyObject *item, if (NEEDS_TRIM(deque, maxlen)) { PyObject *olditem = deque_pop_impl(deque); Py_DECREF(olditem); - } - else { + } else { deque->state++; } return 0; @@ -1590,13 +1588,11 @@ deque___reduce___impl(dequeobject *deque) // It's safe to access deque->maxlen here without holding the per object // lock for deque; deque->maxlen is only assigned during construction. - Py_ssize_t maxlen = deque->maxlen; - PyTypeObject *typ = Py_TYPE(deque); - if (maxlen < 0) { - return Py_BuildValue("O()NN", typ, state, it); + if (deque->maxlen < 0) { + return Py_BuildValue("O()NN", Py_TYPE(deque), state, it); } else { - return Py_BuildValue("O(()n)NN", typ, maxlen, state, it); + return Py_BuildValue("O(()n)NN", Py_TYPE(deque), deque->maxlen, state, it); } } @@ -1888,12 +1884,12 @@ typedef struct { static PyObject * deque_iter(dequeobject *deque) { + dequeiterobject *it; + collections_state *state = find_module_state_by_def(Py_TYPE(deque)); - dequeiterobject *it = - PyObject_GC_New(dequeiterobject, state->dequeiter_type); - if (it == NULL) { + it = PyObject_GC_New(dequeiterobject, state->dequeiter_type); + if (it == NULL) return NULL; - } Py_BEGIN_CRITICAL_SECTION(deque); it->b = deque->leftblock; it->index = deque->leftindex; @@ -1934,6 +1930,8 @@ dequeiter_dealloc(dequeiterobject *dio) static PyObject * dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) { + PyObject *item; + if (it->deque->state != it->state) { it->counter = 0; PyErr_SetString(PyExc_RuntimeError, @@ -1945,7 +1943,7 @@ dequeiter_next_lock_held(dequeiterobject *it, dequeobject *deque) assert (!(it->b == it->deque->rightblock && it->index > it->deque->rightindex)); - PyObject *item = it->b->data[it->index]; + item = it->b->data[it->index]; it->index++; it->counter--; if (it->index == BLOCKLEN && it->counter > 0) { @@ -2060,12 +2058,12 @@ static PyType_Spec dequeiter_spec = { static PyObject * deque_reviter(dequeobject *deque) { + dequeiterobject *it; collections_state *state = find_module_state_by_def(Py_TYPE(deque)); - dequeiterobject *it = - PyObject_GC_New(dequeiterobject, state->dequereviter_type); - if (it == NULL) { + + it = PyObject_GC_New(dequeiterobject, state->dequereviter_type); + if (it == NULL) return NULL; - } Py_BEGIN_CRITICAL_SECTION(deque); it->b = deque->rightblock; it->index = deque->rightindex; @@ -2080,6 +2078,7 @@ deque_reviter(dequeobject *deque) static PyObject * dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) { + PyObject *item; if (it->counter == 0) return NULL; @@ -2092,7 +2091,7 @@ dequereviter_next_lock_held(dequeiterobject *it, dequeobject *deque) assert (!(it->b == it->deque->leftblock && it->index < it->deque->leftindex)); - PyObject *item = it->b->data[it->index]; + item = it->b->data[it->index]; it->index--; it->counter--; if (it->index < 0 && it->counter > 0) { From fdadbf9a57bc2066b575ba5dda514f4047b52db9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 1 Feb 2024 23:08:37 +0100 Subject: [PATCH 49/51] Update Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst --- .../2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst index db52c5a8ee913a..3041d6513c6597 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-08-21-57-41.gh-issue-112050.qwgjx1.rst @@ -1 +1 @@ -Make methods on ``collections.deque`` thread-safe when the GIL is disabled. +Make methods on :class:`collections.deque` thread-safe when the GIL is disabled. From 6e64ef471619b68c29cdc838cd545da27037364d Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 14 Feb 2024 12:46:39 -0800 Subject: [PATCH 50/51] Use free-threaded atomic wrapper for loading deque_iter len --- Include/internal/pycore_pyatomic_ft_wrappers.h | 2 ++ Modules/_collectionsmodule.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index d1313976da1cfc..cbbe90e009c8d2 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -20,11 +20,13 @@ extern "C" { #endif #ifdef Py_GIL_DISABLED +#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value) #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \ _Py_atomic_load_ssize_relaxed(&value) #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \ _Py_atomic_store_ssize_relaxed(&value, new_value) #else +#define FT_ATOMIC_LOAD_SSIZE(value) value #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #endif diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 9098de6d2f143a..47898b1ae23257 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -3,6 +3,7 @@ #include "pycore_dict.h" // _PyDict_GetItem_KnownHash() #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_moduleobject.h" // _PyModule_GetState() +#include "pycore_pyatomic_ft_wrappers.h" #include "pycore_typeobject.h" // _PyType_GetModuleState() #include @@ -2007,7 +2008,7 @@ dequeiter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) static PyObject * dequeiter_len(dequeiterobject *it, PyObject *Py_UNUSED(ignored)) { - Py_ssize_t len = _Py_atomic_load_ssize(&it->counter); + Py_ssize_t len = FT_ATOMIC_LOAD_SSIZE(it->counter); return PyLong_FromSsize_t(len); } From fa7e8a828902552dab60eb1dd407ffcabfd73118 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 14 Feb 2024 13:45:17 -0800 Subject: [PATCH 51/51] Use free-threaded atomic wrapper in deque_len, too --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 47898b1ae23257..309d63c9bf7cbe 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1214,7 +1214,7 @@ deque_contains(dequeobject *deque, PyObject *v) static Py_ssize_t deque_len(dequeobject *deque) { - return _Py_atomic_load_ssize(&((PyVarObject *)deque)->ob_size); + return FT_ATOMIC_LOAD_SSIZE(((PyVarObject *)deque)->ob_size); } /*[clinic input]