From 7c8edcd7e1fbd9a13caf9bf8d48665906ab251bf Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 08:49:43 -0500 Subject: [PATCH 01/31] gh-128942: make arraymodule.c free-thread safe --- Modules/arraymodule.c | 487 ++++++++++++++++++++++----------- Modules/clinic/arraymodule.c.h | 27 +- 2 files changed, 328 insertions(+), 186 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index b80c964f20d65e..0320d9b8c103da 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -13,6 +13,7 @@ #include "pycore_ceval.h" // _PyEval_GetBuiltin() #include "pycore_modsupport.h" // _PyArg_NoKeywords() #include "pycore_moduleobject.h" // _PyModule_GetState() +#include "pycore_pyatomic_ft_wrappers.h" #include // offsetof() #include @@ -68,6 +69,9 @@ typedef struct { PyObject *str_iter; } array_state; +/* Forward declaration. */ +static PyObject *array_array_frombytes(arrayobject *self, PyObject *bytes); + static array_state * get_array_state(PyObject *module) { @@ -735,12 +739,12 @@ array_dealloc(arrayobject *op) static PyObject * array_richcompare(PyObject *v, PyObject *w, int op) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(v)); arrayobject *va, *wa; PyObject *vi = NULL; PyObject *wi = NULL; Py_ssize_t i, k; - PyObject *res; if (!array_Check(v, state) || !array_Check(w, state)) Py_RETURN_NOTIMPLEMENTED; @@ -748,13 +752,14 @@ array_richcompare(PyObject *v, PyObject *w, int op) va = (arrayobject *)v; wa = (arrayobject *)w; + Py_BEGIN_CRITICAL_SECTION2(v, w); if (Py_SIZE(va) != Py_SIZE(wa) && (op == Py_EQ || op == Py_NE)) { /* Shortcut: if the lengths differ, the arrays differ */ if (op == Py_EQ) - res = Py_False; + ret = Py_False; else - res = Py_True; - return Py_NewRef(res); + ret = Py_True; + goto done; } if (va->ob_descr == wa->ob_descr && va->ob_descr->compareitems != NULL) { @@ -774,10 +779,10 @@ array_richcompare(PyObject *v, PyObject *w, int op) case Py_NE: cmp = result != 0; break; case Py_GT: cmp = result > 0; break; case Py_GE: cmp = result >= 0; break; - default: return NULL; /* cannot happen */ + default: goto done; /* cannot happen */ } - PyObject *res = cmp ? Py_True : Py_False; - return Py_NewRef(res); + ret = cmp ? Py_True : Py_False; + goto done; } @@ -786,12 +791,12 @@ array_richcompare(PyObject *v, PyObject *w, int op) for (i = 0; i < Py_SIZE(va) && i < Py_SIZE(wa); i++) { vi = getarrayitem(v, i); if (vi == NULL) { - return NULL; + goto done; } wi = getarrayitem(w, i); if (wi == NULL) { Py_DECREF(vi); - return NULL; + goto done; } k = PyObject_RichCompareBool(vi, wi, Py_EQ); if (k == 0) @@ -799,7 +804,7 @@ array_richcompare(PyObject *v, PyObject *w, int op) Py_DECREF(vi); Py_DECREF(wi); if (k < 0) - return NULL; + goto done; } if (k) { @@ -817,53 +822,67 @@ array_richcompare(PyObject *v, PyObject *w, int op) case Py_NE: assert(vs == ws); cmp = 0; break; case Py_GT: cmp = vs > ws; break; case Py_GE: cmp = vs >= ws; break; - default: return NULL; /* cannot happen */ + default: goto done; /* cannot happen */ } if (cmp) - res = Py_True; + ret = Py_True; else - res = Py_False; - return Py_NewRef(res); + ret = Py_False; + goto done; } /* We have an item that differs. First, shortcuts for EQ/NE */ if (op == Py_EQ) { - res = Py_NewRef(Py_False); + ret = Py_False; } else if (op == Py_NE) { - res = Py_NewRef(Py_True); + ret = Py_True; } else { /* Compare the final item again using the proper operator */ - res = PyObject_RichCompare(vi, wi, op); + ret = PyObject_RichCompare(vi, wi, op); } Py_DECREF(vi); Py_DECREF(wi); - return res; +done: + Py_END_CRITICAL_SECTION2(); + return ret; } static Py_ssize_t array_length(arrayobject *a) { - return Py_SIZE(a); + Py_ssize_t ret; + Py_BEGIN_CRITICAL_SECTION(a); // overkill but lets not tempt tsan + ret = Py_SIZE(a); + Py_END_CRITICAL_SECTION(); + return ret; } static PyObject * array_item(arrayobject *a, Py_ssize_t i) { + PyObject *ret = NULL; + + Py_BEGIN_CRITICAL_SECTION(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array index out of range"); - return NULL; + goto done; } - return getarrayitem((PyObject *)a, i); + ret = getarrayitem((PyObject *)a, i); +done: + Py_END_CRITICAL_SECTION(); + return ret; } static PyObject * array_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); arrayobject *np; + Py_BEGIN_CRITICAL_SECTION(a); if (ilow < 0) ilow = 0; else if (ilow > Py_SIZE(a)) @@ -876,12 +895,15 @@ array_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) ihigh = Py_SIZE(a); np = (arrayobject *) newarrayobject(state->ArrayType, ihigh - ilow, a->ob_descr); if (np == NULL) - return NULL; + goto done; if (ihigh > ilow) { memcpy(np->ob_item, a->ob_item + ilow * a->ob_descr->itemsize, (ihigh-ilow) * a->ob_descr->itemsize); } - return (PyObject *)np; + ret = (PyObject *)np; +done: + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -894,10 +916,13 @@ static PyObject * array_array_clear_impl(arrayobject *self) /*[clinic end generated code: output=5efe0417062210a9 input=5dffa30e94e717a4]*/ { + PyObject *ret = Py_None; + Py_BEGIN_CRITICAL_SECTION(self); if (array_resize(self, 0) == -1) { - return NULL; + ret = NULL; } - Py_RETURN_NONE; + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -932,6 +957,7 @@ array_array___deepcopy__(arrayobject *self, PyObject *unused) static PyObject * array_concat(arrayobject *a, PyObject *bb) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); Py_ssize_t size; arrayobject *np; @@ -946,13 +972,16 @@ array_concat(arrayobject *a, PyObject *bb) PyErr_BadArgument(); return NULL; } + + Py_BEGIN_CRITICAL_SECTION2(a, bb); if (Py_SIZE(a) > PY_SSIZE_T_MAX - Py_SIZE(b)) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto done; } size = Py_SIZE(a) + Py_SIZE(b); np = (arrayobject *) newarrayobject(state->ArrayType, size, a->ob_descr); if (np == NULL) { - return NULL; + goto done; } if (Py_SIZE(a) > 0) { memcpy(np->ob_item, a->ob_item, Py_SIZE(a)*a->ob_descr->itemsize); @@ -961,33 +990,42 @@ array_concat(arrayobject *a, PyObject *bb) memcpy(np->ob_item + Py_SIZE(a)*a->ob_descr->itemsize, b->ob_item, Py_SIZE(b)*b->ob_descr->itemsize); } - return (PyObject *)np; + ret = (PyObject *)np; +done: + Py_END_CRITICAL_SECTION2(); + return ret; #undef b } static PyObject * array_repeat(arrayobject *a, Py_ssize_t n) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); + Py_BEGIN_CRITICAL_SECTION(a); if (n < 0) n = 0; const Py_ssize_t array_length = Py_SIZE(a); if ((array_length != 0) && (n > PY_SSIZE_T_MAX / array_length)) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto done; } Py_ssize_t size = array_length * n; arrayobject* np = (arrayobject *) newarrayobject(state->ArrayType, size, a->ob_descr); if (np == NULL) - return NULL; + goto done; + ret = (PyObject *)np; if (size == 0) - return (PyObject *)np; + goto done; const Py_ssize_t oldbytes = array_length * a->ob_descr->itemsize; const Py_ssize_t newbytes = oldbytes * n; _PyBytes_Repeat(np->ob_item, newbytes, a->ob_item, oldbytes); - return (PyObject *)np; +done: + Py_END_CRITICAL_SECTION(); + return ret; } static int @@ -1028,14 +1066,21 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) static int array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) { + int ret = -1; + + Py_BEGIN_CRITICAL_SECTION(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); - return -1; + goto done; } if (v == NULL) - return array_del_slice(a, i, i+1); - return (*a->ob_descr->setitem)(a, i, v); + ret = array_del_slice(a, i, i+1); + else + ret = (*a->ob_descr->setitem)(a, i, v); +done: + Py_END_CRITICAL_SECTION(); + return ret; } static int @@ -1074,33 +1119,40 @@ array_iter_extend(arrayobject *self, PyObject *bb) static int array_do_extend(array_state *state, arrayobject *self, PyObject *bb) { + int ret = -1; Py_ssize_t size, oldsize, bbsize; - if (!array_Check(bb, state)) - return array_iter_extend(self, bb); + Py_BEGIN_CRITICAL_SECTION2(self, bb); + if (!array_Check(bb, state)) { + ret = array_iter_extend(self, bb); + goto done; + } #define b ((arrayobject *)bb) if (self->ob_descr != b->ob_descr) { PyErr_SetString(PyExc_TypeError, "can only extend with array of same kind"); - return -1; + goto done; } if ((Py_SIZE(self) > PY_SSIZE_T_MAX - Py_SIZE(b)) || ((Py_SIZE(self) + Py_SIZE(b)) > PY_SSIZE_T_MAX / self->ob_descr->itemsize)) { PyErr_NoMemory(); - return -1; + goto done; } oldsize = Py_SIZE(self); /* Get the size of bb before resizing the array since bb could be self. */ bbsize = Py_SIZE(bb); size = oldsize + Py_SIZE(b); if (array_resize(self, size) == -1) - return -1; + goto done; if (bbsize > 0) { memcpy(self->ob_item + oldsize * self->ob_descr->itemsize, b->ob_item, bbsize * b->ob_descr->itemsize); } + ret = 0; - return 0; +done: + Py_END_CRITICAL_SECTION2(); + return ret; #undef b } @@ -1123,32 +1175,43 @@ array_inplace_concat(arrayobject *self, PyObject *bb) static PyObject * array_inplace_repeat(arrayobject *self, Py_ssize_t n) { + PyObject *ret = NULL; const Py_ssize_t array_size = Py_SIZE(self); + Py_BEGIN_CRITICAL_SECTION(self); if (array_size > 0 && n != 1 ) { if (n < 0) n = 0; if ((self->ob_descr->itemsize != 0) && (array_size > PY_SSIZE_T_MAX / self->ob_descr->itemsize)) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto done; } Py_ssize_t size = array_size * self->ob_descr->itemsize; if (n > 0 && size > PY_SSIZE_T_MAX / n) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto done; } if (array_resize(self, n * array_size) == -1) - return NULL; + goto done; _PyBytes_Repeat(self->ob_item, n*size, self->ob_item, size); } - return Py_NewRef(self); + ret = Py_NewRef(self); +done: + Py_END_CRITICAL_SECTION(); + return ret; } static PyObject * ins(arrayobject *self, Py_ssize_t where, PyObject *v) { - if (ins1(self, where, v) != 0) + int res; + Py_BEGIN_CRITICAL_SECTION(self); + res = ins1(self, where, v); + Py_END_CRITICAL_SECTION(); + if (res != 0) return NULL; Py_RETURN_NONE; } @@ -1166,24 +1229,29 @@ static PyObject * array_array_count(arrayobject *self, PyObject *v) /*[clinic end generated code: output=3dd3624bf7135a3a input=d9bce9d65e39d1f5]*/ { + PyObject *ret = NULL; Py_ssize_t count = 0; Py_ssize_t i; + Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *selfi; int cmp; selfi = getarrayitem((PyObject *)self, i); if (selfi == NULL) - return NULL; + goto done; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) count++; else if (cmp < 0) - return NULL; + goto done; } - return PyLong_FromSsize_t(count); + ret = PyLong_FromSsize_t(count); +done: + Py_END_CRITICAL_SECTION(); + return ret; } @@ -1205,6 +1273,9 @@ array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, Py_ssize_t stop) /*[clinic end generated code: output=c45e777880c99f52 input=089dff7baa7e5a7e]*/ { + PyObject *ret = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); if (start < 0) { start += Py_SIZE(self); if (start < 0) { @@ -1222,17 +1293,20 @@ array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, selfi = getarrayitem((PyObject *)self, i); if (selfi == NULL) - return NULL; + goto done; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) { - return PyLong_FromSsize_t(i); + ret = PyLong_FromSsize_t(i); + goto done; } else if (cmp < 0) - return NULL; + goto done; } PyErr_SetString(PyExc_ValueError, "array.index(x): x not in array"); - return NULL; +done: + Py_END_CRITICAL_SECTION(); + return ret; } static int @@ -1241,13 +1315,18 @@ array_contains(arrayobject *self, PyObject *v) Py_ssize_t i; int cmp; + Py_BEGIN_CRITICAL_SECTION(self); for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(self); i++) { PyObject *selfi = getarrayitem((PyObject *)self, i); - if (selfi == NULL) - return -1; + if (selfi == NULL) { + cmp = -1; + goto done; + } cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); } +done: + Py_END_CRITICAL_SECTION(); return cmp; } @@ -1264,27 +1343,32 @@ static PyObject * array_array_remove(arrayobject *self, PyObject *v) /*[clinic end generated code: output=bef06be9fdf9dceb input=0b1e5aed25590027]*/ { + PyObject *ret = NULL; Py_ssize_t i; + Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *selfi; int cmp; selfi = getarrayitem((PyObject *)self,i); if (selfi == NULL) - return NULL; + goto done; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) { if (array_del_slice(self, i, i+1) != 0) - return NULL; - Py_RETURN_NONE; + goto done; + ret = Py_None; + goto done; } else if (cmp < 0) - return NULL; + goto done; } PyErr_SetString(PyExc_ValueError, "array.remove(x): x not in array"); - return NULL; +done: + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1302,27 +1386,31 @@ static PyObject * array_array_pop_impl(arrayobject *self, Py_ssize_t i) /*[clinic end generated code: output=bc1f0c54fe5308e4 input=8e5feb4c1a11cd44]*/ { - PyObject *v; + PyObject *ret = NULL; + Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) == 0) { /* Special-case most common failure cause */ PyErr_SetString(PyExc_IndexError, "pop from empty array"); - return NULL; + goto done; } if (i < 0) i += Py_SIZE(self); if (i < 0 || i >= Py_SIZE(self)) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); - return NULL; + goto done; } - v = getarrayitem((PyObject *)self, i); + PyObject *v = getarrayitem((PyObject *)self, i); if (v == NULL) - return NULL; + goto done; if (array_del_slice(self, i, i+1) != 0) { Py_DECREF(v); - return NULL; + goto done; } - return v; + ret = v; +done: + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1376,27 +1464,29 @@ static PyObject * array_array_buffer_info_impl(arrayobject *self) /*[clinic end generated code: output=9b2a4ec3ae7e98e7 input=a58bae5c6e1ac6a6]*/ { - PyObject *retval = NULL, *v; - - retval = PyTuple_New(2); + PyObject *ret = NULL, *v; + PyObject *retval = PyTuple_New(2); if (!retval) return NULL; + Py_BEGIN_CRITICAL_SECTION(self); v = PyLong_FromVoidPtr(self->ob_item); if (v == NULL) { Py_DECREF(retval); - return NULL; + goto done; } PyTuple_SET_ITEM(retval, 0, v); v = PyLong_FromSsize_t(Py_SIZE(self)); if (v == NULL) { Py_DECREF(retval); - return NULL; + goto done; } PyTuple_SET_ITEM(retval, 1, v); - - return retval; + ret = retval; +done: + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1428,9 +1518,11 @@ static PyObject * array_array_byteswap_impl(arrayobject *self) /*[clinic end generated code: output=5f8236cbdf0d90b5 input=6a85591b950a0186]*/ { + PyObject *ret = Py_None; char *p; Py_ssize_t i; + Py_BEGIN_CRITICAL_SECTION(self); switch (self->ob_descr->itemsize) { case 1: break; @@ -1470,9 +1562,10 @@ array_array_byteswap_impl(arrayobject *self) default: PyErr_SetString(PyExc_RuntimeError, "don't know how to byteswap this array type"); - return NULL; + ret = NULL; } - Py_RETURN_NONE; + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1491,6 +1584,7 @@ array_array_reverse_impl(arrayobject *self) char tmp[256]; /* 8 is probably enough -- but why skimp */ assert((size_t)itemsize <= sizeof(tmp)); + Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) > 1) { for (p = self->ob_item, q = self->ob_item + (Py_SIZE(self) - 1)*itemsize; @@ -1504,6 +1598,7 @@ array_array_reverse_impl(arrayobject *self) memcpy(q, tmp, itemsize); } } + Py_END_CRITICAL_SECTION(); Py_RETURN_NONE; } @@ -1586,6 +1681,9 @@ static PyObject * array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) /*[clinic end generated code: output=4560c628d9c18bc2 input=5a24da7a7b407b52]*/ { + PyObject *ret = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); Py_ssize_t nbytes = Py_SIZE(self) * self->ob_descr->itemsize; /* Write 64K blocks at a time */ /* XXX Make the block size settable */ @@ -1593,8 +1691,10 @@ array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) Py_ssize_t nblocks = (nbytes + BLOCKSIZE - 1) / BLOCKSIZE; Py_ssize_t i; - if (Py_SIZE(self) == 0) + if (Py_SIZE(self) == 0) { + ret = Py_None; goto done; + } array_state *state = get_array_state_by_class(cls); @@ -1609,16 +1709,18 @@ array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) size = nbytes - i*BLOCKSIZE; bytes = PyBytes_FromStringAndSize(ptr, size); if (bytes == NULL) - return NULL; + goto done; res = PyObject_CallMethodOneArg(f, state->str_write, bytes); Py_DECREF(bytes); if (res == NULL) - return NULL; + goto done; Py_DECREF(res); /* drop write result */ } + ret = Py_None; done: - Py_RETURN_NONE; + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1634,34 +1736,40 @@ static PyObject * array_array_fromlist(arrayobject *self, PyObject *list) /*[clinic end generated code: output=26411c2d228a3e3f input=be2605a96c49680f]*/ { + PyObject *ret = NULL; Py_ssize_t n; if (!PyList_Check(list)) { PyErr_SetString(PyExc_TypeError, "arg must be list"); return NULL; } + + Py_BEGIN_CRITICAL_SECTION2(self, list); n = PyList_Size(list); if (n > 0) { Py_ssize_t i, old_size; old_size = Py_SIZE(self); if (array_resize(self, old_size + n) == -1) - return NULL; + goto done; for (i = 0; i < n; i++) { PyObject *v = PyList_GET_ITEM(list, i); if ((*self->ob_descr->setitem)(self, Py_SIZE(self) - n + i, v) != 0) { array_resize(self, old_size); - return NULL; + goto done; } if (n != PyList_GET_SIZE(list)) { PyErr_SetString(PyExc_RuntimeError, "list changed size during iteration"); array_resize(self, old_size); - return NULL; + goto done; } } } - Py_RETURN_NONE; + ret = Py_None; +done: + Py_END_CRITICAL_SECTION2(); + return ret; } /*[clinic input] @@ -1679,69 +1787,82 @@ array_array_tolist_impl(arrayobject *self) if (list == NULL) return NULL; + Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *v = getarrayitem((PyObject *)self, i); - if (v == NULL) + if (v == NULL) { + Py_DECREF(list); + list = NULL; goto error; + } PyList_SET_ITEM(list, i, v); } - return list; error: - Py_DECREF(list); - return NULL; + Py_END_CRITICAL_SECTION(); + return list; } static PyObject * -frombytes(arrayobject *self, Py_buffer *buffer) +frombytes(arrayobject *self, PyObject *bytes) { + PyObject *ret = NULL; + Py_buffer buffer = {NULL, NULL}; + + Py_BEGIN_CRITICAL_SECTION2(self, bytes); + if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE) != 0) { + goto done; + } + int itemsize = self->ob_descr->itemsize; Py_ssize_t n; - if (buffer->itemsize != 1) { - PyBuffer_Release(buffer); + if (buffer.itemsize != 1) { PyErr_SetString(PyExc_TypeError, "a bytes-like object is required"); - return NULL; + goto done; } - n = buffer->len; + n = buffer.len; if (n % itemsize != 0) { - PyBuffer_Release(buffer); PyErr_SetString(PyExc_ValueError, "bytes length not a multiple of item size"); - return NULL; + goto done; } n = n / itemsize; if (n > 0) { Py_ssize_t old_size = Py_SIZE(self); if ((n > PY_SSIZE_T_MAX - old_size) || ((old_size + n) > PY_SSIZE_T_MAX / itemsize)) { - PyBuffer_Release(buffer); return PyErr_NoMemory(); } if (array_resize(self, old_size + n) == -1) { - PyBuffer_Release(buffer); - return NULL; + goto done; } memcpy(self->ob_item + old_size * itemsize, - buffer->buf, n * itemsize); + buffer.buf, n * itemsize); } - PyBuffer_Release(buffer); - Py_RETURN_NONE; + ret = Py_None; +done: + // check below depends on Limited API and stable ABI + if (buffer.obj != NULL) { + PyBuffer_Release(&buffer); + } + Py_END_CRITICAL_SECTION2(); + return ret; } /*[clinic input] array.array.frombytes - buffer: Py_buffer + bytes: object / Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method. [clinic start generated code]*/ static PyObject * -array_array_frombytes_impl(arrayobject *self, Py_buffer *buffer) -/*[clinic end generated code: output=d9842c8f7510a516 input=378db226dfac949e]*/ +array_array_frombytes(arrayobject *self, PyObject *bytes) +/*[clinic end generated code: output=3d8413868a91ec1f input=9fcb15a49b73d960]*/ { - return frombytes(self, buffer); + return frombytes(self, bytes); } /*[clinic input] @@ -1754,12 +1875,17 @@ static PyObject * array_array_tobytes_impl(arrayobject *self) /*[clinic end generated code: output=87318e4edcdc2bb6 input=90ee495f96de34f5]*/ { + PyObject *ret; + + Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) <= PY_SSIZE_T_MAX / self->ob_descr->itemsize) { - return PyBytes_FromStringAndSize(self->ob_item, + ret = PyBytes_FromStringAndSize(self->ob_item, Py_SIZE(self) * self->ob_descr->itemsize); } else { - return PyErr_NoMemory(); + ret = PyErr_NoMemory(); } + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1779,6 +1905,8 @@ static PyObject * array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) /*[clinic end generated code: output=24359f5e001a7f2b input=025db1fdade7a4ce]*/ { + PyObject *ret = NULL; + int typecode = self->ob_descr->typecode; if (typecode != 'u' && typecode != 'w') { PyErr_SetString(PyExc_ValueError, @@ -1787,6 +1915,7 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) return NULL; } + Py_BEGIN_CRITICAL_SECTION(self); if (typecode == 'u') { Py_ssize_t ustr_length = PyUnicode_AsWideChar(ustr, NULL, 0); assert(ustr_length > 0); @@ -1794,7 +1923,7 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) ustr_length--; /* trim trailing NUL character */ Py_ssize_t old_size = Py_SIZE(self); if (array_resize(self, old_size + ustr_length) == -1) { - return NULL; + goto done; } // must not fail @@ -1808,10 +1937,11 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) Py_ssize_t new_size = old_size + ustr_length; if (new_size < 0 || (size_t)new_size > PY_SSIZE_T_MAX / sizeof(Py_UCS4)) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto done; } if (array_resize(self, new_size) == -1) { - return NULL; + goto done; } // must not fail @@ -1820,8 +1950,10 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) assert(u != NULL); (void)u; // Suppress unused_variable warning. } - - Py_RETURN_NONE; + ret = Py_None; +done: + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1838,20 +1970,24 @@ static PyObject * array_array_tounicode_impl(arrayobject *self) /*[clinic end generated code: output=08e442378336e1ef input=127242eebe70b66d]*/ { + PyObject *ret; int typecode = self->ob_descr->typecode; if (typecode != 'u' && typecode != 'w') { PyErr_SetString(PyExc_ValueError, "tounicode() may only be called on unicode type arrays ('u' or 'w')"); return NULL; } + Py_BEGIN_CRITICAL_SECTION(self); if (typecode == 'u') { - return PyUnicode_FromWideChar((wchar_t *) self->ob_item, Py_SIZE(self)); + ret = PyUnicode_FromWideChar((wchar_t *) self->ob_item, Py_SIZE(self)); } else { // typecode == 'w' int byteorder = 0; // native byteorder - return PyUnicode_DecodeUTF32((const char *) self->ob_item, Py_SIZE(self) * 4, + ret = PyUnicode_DecodeUTF32((const char *) self->ob_item, Py_SIZE(self) * 4, NULL, &byteorder); } + Py_END_CRITICAL_SECTION(); + return ret; } /*[clinic input] @@ -1865,7 +2001,8 @@ array_array___sizeof___impl(arrayobject *self) /*[clinic end generated code: output=d8e1c61ebbe3eaed input=805586565bf2b3c6]*/ { size_t res = _PyObject_SIZE(Py_TYPE(self)); - res += (size_t)self->allocated * (size_t)self->ob_descr->itemsize; + res += (size_t)FT_ATOMIC_LOAD_SSIZE_RELAXED(self->allocated) + * (size_t)self->ob_descr->itemsize; return PyLong_FromSize_t(res); } @@ -2401,14 +2538,16 @@ static PyObject * array_repr(arrayobject *a) { char typecode; - PyObject *s, *v = NULL; + PyObject *v = NULL, *ret = NULL; Py_ssize_t len; + Py_BEGIN_CRITICAL_SECTION(a); len = Py_SIZE(a); typecode = a->ob_descr->typecode; if (len == 0) { - return PyUnicode_FromFormat("%s('%c')", - _PyType_Name(Py_TYPE(a)), (int)typecode); + ret = PyUnicode_FromFormat("%s('%c')", + _PyType_Name(Py_TYPE(a)), (int)typecode); + goto done; } if (typecode == 'u' || typecode == 'w') { v = array_array_tounicode_impl(a); @@ -2416,27 +2555,32 @@ array_repr(arrayobject *a) v = array_array_tolist_impl(a); } if (v == NULL) - return NULL; + goto done; - s = PyUnicode_FromFormat("%s('%c', %R)", - _PyType_Name(Py_TYPE(a)), (int)typecode, v); + ret = PyUnicode_FromFormat("%s('%c', %R)", + _PyType_Name(Py_TYPE(a)), (int)typecode, v); Py_DECREF(v); - return s; +done: + Py_END_CRITICAL_SECTION(); + return ret; } static PyObject* array_subscr(arrayobject* self, PyObject* item) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(self)); + Py_BEGIN_CRITICAL_SECTION(self); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); if (i==-1 && PyErr_Occurred()) { - return NULL; + goto done; } if (i < 0) i += Py_SIZE(self); - return array_item(self, i); + ret = array_item(self, i); + goto done; } else if (PySlice_Check(item)) { Py_ssize_t start, stop, step, slicelength, i; @@ -2446,27 +2590,30 @@ array_subscr(arrayobject* self, PyObject* item) int itemsize = self->ob_descr->itemsize; if (PySlice_Unpack(item, &start, &stop, &step) < 0) { - return NULL; + goto done; } slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop, step); if (slicelength <= 0) { - return newarrayobject(state->ArrayType, 0, self->ob_descr); + ret = newarrayobject(state->ArrayType, 0, self->ob_descr); + goto done; } else if (step == 1) { PyObject *result = newarrayobject(state->ArrayType, slicelength, self->ob_descr); if (result == NULL) - return NULL; + goto done; memcpy(((arrayobject *)result)->ob_item, self->ob_item + start * itemsize, slicelength * itemsize); - return result; + ret = result; + goto done; } else { result = newarrayobject(state->ArrayType, slicelength, self->ob_descr); - if (!result) return NULL; + if (!result) + goto done; ar = (arrayobject*)result; @@ -2477,35 +2624,40 @@ array_subscr(arrayobject* self, PyObject* item) itemsize); } - return result; + ret = result; + goto done; } } else { PyErr_SetString(PyExc_TypeError, "array indices must be integers"); - return NULL; } +done: + Py_END_CRITICAL_SECTION(); + return ret; } static int array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) { + int ret = -1; Py_ssize_t start, stop, step, slicelength, needed; array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; int itemsize; + Py_BEGIN_CRITICAL_SECTION2(self, value); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); if (i == -1 && PyErr_Occurred()) - return -1; + goto done; if (i < 0) i += Py_SIZE(self); if (i < 0 || i >= Py_SIZE(self)) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); - return -1; + goto done; } if (value == NULL) { /* Fall through to slice assignment */ @@ -2514,12 +2666,14 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) step = 1; slicelength = 1; } - else - return (*self->ob_descr->setitem)(self, i, value); + else { + ret = (*self->ob_descr->setitem)(self, i, value); + goto done; + } } else if (PySlice_Check(item)) { if (PySlice_Unpack(item, &start, &stop, &step) < 0) { - return -1; + goto done; } slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop, step); @@ -2527,7 +2681,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) else { PyErr_SetString(PyExc_TypeError, "array indices must be integers"); - return -1; + goto done; } if (value == NULL) { other = NULL; @@ -2538,24 +2692,23 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) needed = Py_SIZE(other); if (self == other) { /* Special case "self[i:j] = self" -- copy self first */ - int ret; value = array_slice(other, 0, needed); if (value == NULL) - return -1; + goto done; ret = array_ass_subscr(self, item, value); Py_DECREF(value); - return ret; + goto done; } if (other->ob_descr != self->ob_descr) { PyErr_BadArgument(); - return -1; + goto done; } } else { PyErr_Format(PyExc_TypeError, "can only assign array (not \"%.200s\") to array slice", Py_TYPE(value)->tp_name); - return -1; + goto done; } itemsize = self->ob_descr->itemsize; /* for 'a[2:1] = ...', the insertion point is 'start', not 'stop' */ @@ -2569,7 +2722,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) if ((needed == 0 || slicelength != needed) && self->ob_exports > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); - return -1; + goto done; } if (step == 1) { @@ -2579,12 +2732,12 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) (Py_SIZE(self) - stop) * itemsize); if (array_resize(self, Py_SIZE(self) + needed - slicelength) < 0) - return -1; + goto done; } else if (slicelength < needed) { if (array_resize(self, Py_SIZE(self) + needed - slicelength) < 0) - return -1; + goto done; memmove(self->ob_item + (start + needed) * itemsize, self->ob_item + stop * itemsize, (Py_SIZE(self) - start - needed) * itemsize); @@ -2592,7 +2745,6 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) if (needed > 0) memcpy(self->ob_item + start * itemsize, other->ob_item, needed * itemsize); - return 0; } else if (needed == 0) { /* Delete slice */ @@ -2621,8 +2773,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) (Py_SIZE(self) - cur) * itemsize); } if (array_resize(self, Py_SIZE(self) - slicelength) < 0) - return -1; - return 0; + goto done; } else { size_t cur; @@ -2633,7 +2784,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) "attempt to assign array of size %zd " "to extended slice of size %zd", needed, slicelength); - return -1; + goto done; } for (cur = start, i = 0; i < slicelength; cur += step, i++) { @@ -2641,8 +2792,11 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) other->ob_item + i * itemsize, itemsize); } - return 0; } + ret = 0; +done: + Py_END_CRITICAL_SECTION2(); + return ret; } static const void *emptybuf = ""; @@ -2657,6 +2811,7 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) return -1; } + Py_BEGIN_CRITICAL_SECTION(self); view->buf = (void *)self->ob_item; view->obj = Py_NewRef(self); if (view->buf == NULL) @@ -2685,18 +2840,20 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) } self->ob_exports++; + Py_END_CRITICAL_SECTION(); return 0; } static void array_buffer_relbuf(arrayobject *self, Py_buffer *view) { - self->ob_exports--; + _Py_atomic_add_ssize(&self->ob_exports, -1); } static PyObject * array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { + PyObject *ret = NULL; array_state *state = find_array_state_by_type(type); int c; PyObject *initial = NULL, *it = NULL; @@ -2742,6 +2899,11 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } } + /* If the initial object is NULL then we use args as a stand-in object + for the critical section. */ + PyObject *lock_obj = initial != NULL ? initial : args; + + Py_BEGIN_CRITICAL_SECTION(lock_obj); if (!(initial == NULL || PyList_Check(initial) || PyByteArray_Check(initial) || PyBytes_Check(initial) @@ -2751,7 +2913,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) && c == ((arrayobject*)initial)->ob_descr->typecode))) { it = PyObject_GetIter(initial); if (it == NULL) - return NULL; + goto done; /* We set initial to NULL so that the subsequent code will create an empty array of the appropriate type and afterwards we can use array_iter_extend to populate @@ -2775,7 +2937,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) a = newarrayobject(type, len, descr); if (a == NULL) - return NULL; + goto done; if (len > 0 && !array_Check(initial, state)) { Py_ssize_t i; @@ -2784,12 +2946,12 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PySequence_GetItem(initial, i); if (v == NULL) { Py_DECREF(a); - return NULL; + goto done; } if (setarrayitem(a, i, v) != 0) { Py_DECREF(v); Py_DECREF(a); - return NULL; + goto done; } Py_DECREF(v); } @@ -2801,7 +2963,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) initial); if (v == NULL) { Py_DECREF(a); - return NULL; + goto done; } Py_DECREF(v); } @@ -2811,7 +2973,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) wchar_t *ustr = PyUnicode_AsWideCharString(initial, &n); if (ustr == NULL) { Py_DECREF(a); - return NULL; + goto done; } if (n > 0) { @@ -2828,7 +2990,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) Py_UCS4 *ustr = PyUnicode_AsUCS4Copy(initial); if (ustr == NULL) { Py_DECREF(a); - return NULL; + goto done; } arrayobject *self = (arrayobject *)a; @@ -2848,16 +3010,19 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (array_iter_extend((arrayobject *)a, it) == -1) { Py_DECREF(it); Py_DECREF(a); - return NULL; + goto done; } Py_DECREF(it); } - return a; + ret = a; + goto done; } } PyErr_SetString(PyExc_ValueError, "bad typecode (must be b, B, u, h, H, i, I, l, L, q, Q, f or d)"); - return NULL; +done: + Py_END_CRITICAL_SECTION(); + return ret; } diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 4a7266ecb8b84f..1087da12606b9f 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -446,7 +446,7 @@ array_array_tolist(arrayobject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(array_array_frombytes__doc__, -"frombytes($self, buffer, /)\n" +"frombytes($self, bytes, /)\n" "--\n" "\n" "Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method."); @@ -454,29 +454,6 @@ PyDoc_STRVAR(array_array_frombytes__doc__, #define ARRAY_ARRAY_FROMBYTES_METHODDEF \ {"frombytes", (PyCFunction)array_array_frombytes, METH_O, array_array_frombytes__doc__}, -static PyObject * -array_array_frombytes_impl(arrayobject *self, Py_buffer *buffer); - -static PyObject * -array_array_frombytes(arrayobject *self, PyObject *arg) -{ - PyObject *return_value = NULL; - Py_buffer buffer = {NULL, NULL}; - - if (PyObject_GetBuffer(arg, &buffer, PyBUF_SIMPLE) != 0) { - goto exit; - } - return_value = array_array_frombytes_impl(self, &buffer); - -exit: - /* Cleanup for buffer */ - if (buffer.obj) { - PyBuffer_Release(&buffer); - } - - return return_value; -} - PyDoc_STRVAR(array_array_tobytes__doc__, "tobytes($self, /)\n" "--\n" @@ -695,4 +672,4 @@ PyDoc_STRVAR(array_arrayiterator___setstate____doc__, #define ARRAY_ARRAYITERATOR___SETSTATE___METHODDEF \ {"__setstate__", (PyCFunction)array_arrayiterator___setstate__, METH_O, array_arrayiterator___setstate____doc__}, -/*[clinic end generated code: output=22dbe12826bfa86f input=a9049054013a1b77]*/ +/*[clinic end generated code: output=02996fe706cfd51b input=a9049054013a1b77]*/ From b4c38d65177ab21a164c5df26e50cb143833851f Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:53:44 +0000 Subject: [PATCH 02/31] =?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 --- .../next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst diff --git a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst new file mode 100644 index 00000000000000..ba2e4eb9a2000d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst @@ -0,0 +1 @@ +Make :module:`array` safe under :term:`free threading`. From 523c049454c7f992e92c75c23292554693bca5b9 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 08:57:53 -0500 Subject: [PATCH 03/31] Update 2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst --- .../next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst index ba2e4eb9a2000d..d9dc92fa167139 100644 --- a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst +++ b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst @@ -1 +1 @@ -Make :module:`array` safe under :term:`free threading`. +Make `array` module safe under :term:`free threading`. From 714da01f330cce05589a442d06e312f58d5d0be2 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 08:59:43 -0500 Subject: [PATCH 04/31] Update 2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst --- .../next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst index d9dc92fa167139..c50e6796f328d4 100644 --- a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst +++ b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst @@ -1 +1 @@ -Make `array` module safe under :term:`free threading`. +Make array module safe under :term:`free threading`. From 68934682f881ea47221253702cd2139e2de74508 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 12:34:41 -0500 Subject: [PATCH 05/31] refactor array methods to established patterns --- Modules/arraymodule.c | 726 +++++++++++++++++---------------- Modules/clinic/arraymodule.c.h | 137 ++++++- 2 files changed, 501 insertions(+), 362 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 0320d9b8c103da..482a37b2616a41 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -737,14 +737,14 @@ array_dealloc(arrayobject *op) } static PyObject * -array_richcompare(PyObject *v, PyObject *w, int op) +array_richcompare_lock_held(PyObject *v, PyObject *w, int op) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(v)); arrayobject *va, *wa; PyObject *vi = NULL; PyObject *wi = NULL; Py_ssize_t i, k; + PyObject *res; if (!array_Check(v, state) || !array_Check(w, state)) Py_RETURN_NOTIMPLEMENTED; @@ -752,14 +752,13 @@ array_richcompare(PyObject *v, PyObject *w, int op) va = (arrayobject *)v; wa = (arrayobject *)w; - Py_BEGIN_CRITICAL_SECTION2(v, w); if (Py_SIZE(va) != Py_SIZE(wa) && (op == Py_EQ || op == Py_NE)) { /* Shortcut: if the lengths differ, the arrays differ */ if (op == Py_EQ) - ret = Py_False; + res = Py_False; else - ret = Py_True; - goto done; + res = Py_True; + return res; } if (va->ob_descr == wa->ob_descr && va->ob_descr->compareitems != NULL) { @@ -779,10 +778,10 @@ array_richcompare(PyObject *v, PyObject *w, int op) case Py_NE: cmp = result != 0; break; case Py_GT: cmp = result > 0; break; case Py_GE: cmp = result >= 0; break; - default: goto done; /* cannot happen */ + default: return NULL; /* cannot happen */ } - ret = cmp ? Py_True : Py_False; - goto done; + PyObject *res = cmp ? Py_True : Py_False; + return res; } @@ -791,12 +790,12 @@ array_richcompare(PyObject *v, PyObject *w, int op) for (i = 0; i < Py_SIZE(va) && i < Py_SIZE(wa); i++) { vi = getarrayitem(v, i); if (vi == NULL) { - goto done; + return NULL; } wi = getarrayitem(w, i); if (wi == NULL) { Py_DECREF(vi); - goto done; + return NULL; } k = PyObject_RichCompareBool(vi, wi, Py_EQ); if (k == 0) @@ -804,7 +803,7 @@ array_richcompare(PyObject *v, PyObject *w, int op) Py_DECREF(vi); Py_DECREF(wi); if (k < 0) - goto done; + return NULL; } if (k) { @@ -822,29 +821,37 @@ array_richcompare(PyObject *v, PyObject *w, int op) case Py_NE: assert(vs == ws); cmp = 0; break; case Py_GT: cmp = vs > ws; break; case Py_GE: cmp = vs >= ws; break; - default: goto done; /* cannot happen */ + default: return NULL; /* cannot happen */ } if (cmp) - ret = Py_True; + res = Py_True; else - ret = Py_False; - goto done; + res = Py_False; + return res; } /* We have an item that differs. First, shortcuts for EQ/NE */ if (op == Py_EQ) { - ret = Py_False; + res = Py_False; } else if (op == Py_NE) { - ret = Py_True; + res = Py_True; } else { /* Compare the final item again using the proper operator */ - ret = PyObject_RichCompare(vi, wi, op); + res = PyObject_RichCompare(vi, wi, op); } Py_DECREF(vi); Py_DECREF(wi); -done: + return res; +} + +static PyObject * +array_richcompare(PyObject *v, PyObject *w, int op) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION2(v, w); + ret = array_richcompare_lock_held(v, w, op); Py_END_CRITICAL_SECTION2(); return ret; } @@ -860,17 +867,21 @@ array_length(arrayobject *a) } static PyObject * -array_item(arrayobject *a, Py_ssize_t i) +array_item_lock_held(arrayobject *a, Py_ssize_t i) { - PyObject *ret = NULL; - - Py_BEGIN_CRITICAL_SECTION(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array index out of range"); - goto done; + return NULL; } - ret = getarrayitem((PyObject *)a, i); -done: + return getarrayitem((PyObject *)a, i); +} + +static PyObject * +array_item(arrayobject *a, Py_ssize_t i) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(a); + ret = array_item_lock_held(a, i); Py_END_CRITICAL_SECTION(); return ret; } @@ -878,11 +889,9 @@ array_item(arrayobject *a, Py_ssize_t i) static PyObject * array_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); arrayobject *np; - Py_BEGIN_CRITICAL_SECTION(a); if (ilow < 0) ilow = 0; else if (ilow > Py_SIZE(a)) @@ -895,18 +904,16 @@ array_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) ihigh = Py_SIZE(a); np = (arrayobject *) newarrayobject(state->ArrayType, ihigh - ilow, a->ob_descr); if (np == NULL) - goto done; + return NULL; if (ihigh > ilow) { memcpy(np->ob_item, a->ob_item + ilow * a->ob_descr->itemsize, (ihigh-ilow) * a->ob_descr->itemsize); } - ret = (PyObject *)np; -done: - Py_END_CRITICAL_SECTION(); - return ret; + return (PyObject *)np; } /*[clinic input] +@critical_section array.array.clear Remove all items from the array. @@ -914,18 +921,16 @@ Remove all items from the array. static PyObject * array_array_clear_impl(arrayobject *self) -/*[clinic end generated code: output=5efe0417062210a9 input=5dffa30e94e717a4]*/ +/*[clinic end generated code: output=5efe0417062210a9 input=1c9dfcc80f5b6731]*/ { - PyObject *ret = Py_None; - Py_BEGIN_CRITICAL_SECTION(self); if (array_resize(self, 0) == -1) { - ret = NULL; + return NULL; } - Py_END_CRITICAL_SECTION(); - return ret; + Py_RETURN_NONE; } /*[clinic input] +@critical_section array.array.__copy__ Return a copy of the array. @@ -933,12 +938,13 @@ Return a copy of the array. static PyObject * array_array___copy___impl(arrayobject *self) -/*[clinic end generated code: output=dec7c3f925d9619e input=ad1ee5b086965f09]*/ +/*[clinic end generated code: output=dec7c3f925d9619e input=7622f8f9489472d5]*/ { return array_slice(self, 0, Py_SIZE(self)); } /*[clinic input] +@critical_section array.array.__deepcopy__ unused: object @@ -948,16 +954,15 @@ Return a copy of the array. [clinic start generated code]*/ static PyObject * -array_array___deepcopy__(arrayobject *self, PyObject *unused) -/*[clinic end generated code: output=1ec748d8e14a9faa input=2405ecb4933748c4]*/ +array_array___deepcopy___impl(arrayobject *self, PyObject *unused) +/*[clinic end generated code: output=703b4c412feaaf31 input=1a29f718f5b8a1dc]*/ { return array_array___copy___impl(self); } static PyObject * -array_concat(arrayobject *a, PyObject *bb) +array_concat_lock_held(arrayobject *a, PyObject *bb) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); Py_ssize_t size; arrayobject *np; @@ -972,16 +977,13 @@ array_concat(arrayobject *a, PyObject *bb) PyErr_BadArgument(); return NULL; } - - Py_BEGIN_CRITICAL_SECTION2(a, bb); if (Py_SIZE(a) > PY_SSIZE_T_MAX - Py_SIZE(b)) { - PyErr_NoMemory(); - goto done; + return PyErr_NoMemory(); } size = Py_SIZE(a) + Py_SIZE(b); np = (arrayobject *) newarrayobject(state->ArrayType, size, a->ob_descr); if (np == NULL) { - goto done; + return NULL; } if (Py_SIZE(a) > 0) { memcpy(np->ob_item, a->ob_item, Py_SIZE(a)*a->ob_descr->itemsize); @@ -990,40 +992,51 @@ array_concat(arrayobject *a, PyObject *bb) memcpy(np->ob_item + Py_SIZE(a)*a->ob_descr->itemsize, b->ob_item, Py_SIZE(b)*b->ob_descr->itemsize); } - ret = (PyObject *)np; -done: + return (PyObject *)np; +#undef b +} + +static PyObject * +array_concat(arrayobject *a, PyObject *bb) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION2(a, bb); + ret = array_concat_lock_held(a, bb); Py_END_CRITICAL_SECTION2(); return ret; -#undef b } static PyObject * -array_repeat(arrayobject *a, Py_ssize_t n) +array_repeat_lock_held(arrayobject *a, Py_ssize_t n) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(a)); - Py_BEGIN_CRITICAL_SECTION(a); if (n < 0) n = 0; const Py_ssize_t array_length = Py_SIZE(a); if ((array_length != 0) && (n > PY_SSIZE_T_MAX / array_length)) { - PyErr_NoMemory(); - goto done; + return PyErr_NoMemory(); } Py_ssize_t size = array_length * n; arrayobject* np = (arrayobject *) newarrayobject(state->ArrayType, size, a->ob_descr); if (np == NULL) - goto done; - ret = (PyObject *)np; + return NULL; if (size == 0) - goto done; + return (PyObject *)np; const Py_ssize_t oldbytes = array_length * a->ob_descr->itemsize; const Py_ssize_t newbytes = oldbytes * n; _PyBytes_Repeat(np->ob_item, newbytes, a->ob_item, oldbytes); -done: + return (PyObject *)np; +} + +static PyObject * +array_repeat(arrayobject *a, Py_ssize_t n) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(a); + ret = array_repeat_lock_held(a, n); Py_END_CRITICAL_SECTION(); return ret; } @@ -1064,21 +1077,24 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) } static int -array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) +array_ass_item_lock_held(arrayobject *a, Py_ssize_t i, PyObject *v) { - int ret = -1; - - Py_BEGIN_CRITICAL_SECTION(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); - goto done; + return -1; } if (v == NULL) - ret = array_del_slice(a, i, i+1); - else - ret = (*a->ob_descr->setitem)(a, i, v); -done: + return array_del_slice(a, i, i+1); + return (*a->ob_descr->setitem)(a, i, v); +} + +static int +array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) +{ + int ret; + Py_BEGIN_CRITICAL_SECTION(a); + ret = array_ass_item_lock_held(a, i, v); Py_END_CRITICAL_SECTION(); return ret; } @@ -1117,43 +1133,46 @@ array_iter_extend(arrayobject *self, PyObject *bb) } static int -array_do_extend(array_state *state, arrayobject *self, PyObject *bb) +array_do_extend_lock_held(array_state *state, arrayobject *self, PyObject *bb) { - int ret = -1; Py_ssize_t size, oldsize, bbsize; - Py_BEGIN_CRITICAL_SECTION2(self, bb); - if (!array_Check(bb, state)) { - ret = array_iter_extend(self, bb); - goto done; - } + if (!array_Check(bb, state)) + return array_iter_extend(self, bb); #define b ((arrayobject *)bb) if (self->ob_descr != b->ob_descr) { PyErr_SetString(PyExc_TypeError, "can only extend with array of same kind"); - goto done; + return -1; } if ((Py_SIZE(self) > PY_SSIZE_T_MAX - Py_SIZE(b)) || ((Py_SIZE(self) + Py_SIZE(b)) > PY_SSIZE_T_MAX / self->ob_descr->itemsize)) { PyErr_NoMemory(); - goto done; + return -1; } oldsize = Py_SIZE(self); /* Get the size of bb before resizing the array since bb could be self. */ bbsize = Py_SIZE(bb); size = oldsize + Py_SIZE(b); if (array_resize(self, size) == -1) - goto done; + return -1; if (bbsize > 0) { memcpy(self->ob_item + oldsize * self->ob_descr->itemsize, b->ob_item, bbsize * b->ob_descr->itemsize); } - ret = 0; -done: + return 0; +#undef b +} + +static int +array_do_extend(array_state *state, arrayobject *self, PyObject *bb) +{ + int ret; + Py_BEGIN_CRITICAL_SECTION2(self, bb); + ret = array_do_extend_lock_held(state, self, bb); Py_END_CRITICAL_SECTION2(); return ret; -#undef b } static PyObject * @@ -1173,32 +1192,35 @@ array_inplace_concat(arrayobject *self, PyObject *bb) } static PyObject * -array_inplace_repeat(arrayobject *self, Py_ssize_t n) +array_inplace_repeat_lock_held(arrayobject *self, Py_ssize_t n) { - PyObject *ret = NULL; const Py_ssize_t array_size = Py_SIZE(self); - Py_BEGIN_CRITICAL_SECTION(self); if (array_size > 0 && n != 1 ) { if (n < 0) n = 0; if ((self->ob_descr->itemsize != 0) && (array_size > PY_SSIZE_T_MAX / self->ob_descr->itemsize)) { - PyErr_NoMemory(); - goto done; + return PyErr_NoMemory(); } Py_ssize_t size = array_size * self->ob_descr->itemsize; if (n > 0 && size > PY_SSIZE_T_MAX / n) { - PyErr_NoMemory(); - goto done; + return PyErr_NoMemory(); } if (array_resize(self, n * array_size) == -1) - goto done; + return NULL; _PyBytes_Repeat(self->ob_item, n*size, self->ob_item, size); } - ret = Py_NewRef(self); -done: + return Py_NewRef(self); +} + +static PyObject * +array_inplace_repeat(arrayobject *self, Py_ssize_t n) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(self); + ret = array_inplace_repeat_lock_held(self, n); Py_END_CRITICAL_SECTION(); return ret; } @@ -1217,6 +1239,7 @@ ins(arrayobject *self, Py_ssize_t where, PyObject *v) } /*[clinic input] +@critical_section array.array.count v: object @@ -1226,36 +1249,32 @@ Return number of occurrences of v in the array. [clinic start generated code]*/ static PyObject * -array_array_count(arrayobject *self, PyObject *v) -/*[clinic end generated code: output=3dd3624bf7135a3a input=d9bce9d65e39d1f5]*/ +array_array_count_impl(arrayobject *self, PyObject *v) +/*[clinic end generated code: output=93ead26a2affb739 input=c12c0042c1d0e27e]*/ { - PyObject *ret = NULL; Py_ssize_t count = 0; Py_ssize_t i; - Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *selfi; int cmp; selfi = getarrayitem((PyObject *)self, i); if (selfi == NULL) - goto done; + return NULL; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) count++; else if (cmp < 0) - goto done; + return NULL; } - ret = PyLong_FromSsize_t(count); -done: - Py_END_CRITICAL_SECTION(); - return ret; + return PyLong_FromSsize_t(count); } /*[clinic input] +@critical_section array.array.index v: object @@ -1271,11 +1290,8 @@ Raise ValueError if the value is not present. static PyObject * array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, Py_ssize_t stop) -/*[clinic end generated code: output=c45e777880c99f52 input=089dff7baa7e5a7e]*/ +/*[clinic end generated code: output=c45e777880c99f52 input=fa32ac8ec22175d6]*/ { - PyObject *ret = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); if (start < 0) { start += Py_SIZE(self); if (start < 0) { @@ -1293,44 +1309,47 @@ array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, selfi = getarrayitem((PyObject *)self, i); if (selfi == NULL) - goto done; + return NULL; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) { - ret = PyLong_FromSsize_t(i); - goto done; + return PyLong_FromSsize_t(i); } else if (cmp < 0) - goto done; + return NULL; } PyErr_SetString(PyExc_ValueError, "array.index(x): x not in array"); -done: - Py_END_CRITICAL_SECTION(); - return ret; + return NULL; } static int -array_contains(arrayobject *self, PyObject *v) +array_contains_lock_held(arrayobject *self, PyObject *v) { Py_ssize_t i; int cmp; - Py_BEGIN_CRITICAL_SECTION(self); for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(self); i++) { PyObject *selfi = getarrayitem((PyObject *)self, i); - if (selfi == NULL) { - cmp = -1; - goto done; - } + if (selfi == NULL) + return -1; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); } -done: - Py_END_CRITICAL_SECTION(); return cmp; } +static int +array_contains(arrayobject *self, PyObject *v) +{ + int ret; + Py_BEGIN_CRITICAL_SECTION(self); + ret = array_contains_lock_held(self, v); + Py_END_CRITICAL_SECTION(); + return ret; +} + /*[clinic input] +@critical_section array.array.remove v: object @@ -1340,38 +1359,34 @@ Remove the first occurrence of v in the array. [clinic start generated code]*/ static PyObject * -array_array_remove(arrayobject *self, PyObject *v) -/*[clinic end generated code: output=bef06be9fdf9dceb input=0b1e5aed25590027]*/ +array_array_remove_impl(arrayobject *self, PyObject *v) +/*[clinic end generated code: output=f2a24e288ecb2a35 input=78bef3fd40e62f7a]*/ { - PyObject *ret = NULL; Py_ssize_t i; - Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *selfi; int cmp; selfi = getarrayitem((PyObject *)self,i); if (selfi == NULL) - goto done; + return NULL; cmp = PyObject_RichCompareBool(selfi, v, Py_EQ); Py_DECREF(selfi); if (cmp > 0) { if (array_del_slice(self, i, i+1) != 0) - goto done; - ret = Py_None; - goto done; + return NULL; + Py_RETURN_NONE; } else if (cmp < 0) - goto done; + return NULL; } PyErr_SetString(PyExc_ValueError, "array.remove(x): x not in array"); -done: - Py_END_CRITICAL_SECTION(); - return ret; + return NULL; } /*[clinic input] +@critical_section array.array.pop i: Py_ssize_t = -1 @@ -1384,33 +1399,29 @@ i defaults to -1. static PyObject * array_array_pop_impl(arrayobject *self, Py_ssize_t i) -/*[clinic end generated code: output=bc1f0c54fe5308e4 input=8e5feb4c1a11cd44]*/ +/*[clinic end generated code: output=bc1f0c54fe5308e4 input=c69a7f1f8c570e2f]*/ { - PyObject *ret = NULL; + PyObject *v; - Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) == 0) { /* Special-case most common failure cause */ PyErr_SetString(PyExc_IndexError, "pop from empty array"); - goto done; + return NULL; } if (i < 0) i += Py_SIZE(self); if (i < 0 || i >= Py_SIZE(self)) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); - goto done; + return NULL; } - PyObject *v = getarrayitem((PyObject *)self, i); + v = getarrayitem((PyObject *)self, i); if (v == NULL) - goto done; + return NULL; if (array_del_slice(self, i, i+1) != 0) { Py_DECREF(v); - goto done; + return NULL; } - ret = v; -done: - Py_END_CRITICAL_SECTION(); - return ret; + return v; } /*[clinic input] @@ -1452,6 +1463,7 @@ array_array_insert_impl(arrayobject *self, Py_ssize_t i, PyObject *v) } /*[clinic input] +@critical_section array.array.buffer_info Return a tuple (address, length) giving the current memory address and the length in items of the buffer used to hold array's contents. @@ -1462,31 +1474,29 @@ the buffer length in bytes. static PyObject * array_array_buffer_info_impl(arrayobject *self) -/*[clinic end generated code: output=9b2a4ec3ae7e98e7 input=a58bae5c6e1ac6a6]*/ +/*[clinic end generated code: output=9b2a4ec3ae7e98e7 input=9d0dc1ff0e6542e8]*/ { - PyObject *ret = NULL, *v; - PyObject *retval = PyTuple_New(2); + PyObject *retval = NULL, *v; + + retval = PyTuple_New(2); if (!retval) return NULL; - Py_BEGIN_CRITICAL_SECTION(self); v = PyLong_FromVoidPtr(self->ob_item); if (v == NULL) { Py_DECREF(retval); - goto done; + return NULL; } PyTuple_SET_ITEM(retval, 0, v); v = PyLong_FromSsize_t(Py_SIZE(self)); if (v == NULL) { Py_DECREF(retval); - goto done; + return NULL; } PyTuple_SET_ITEM(retval, 1, v); - ret = retval; -done: - Py_END_CRITICAL_SECTION(); - return ret; + + return retval; } /*[clinic input] @@ -1506,6 +1516,7 @@ array_array_append(arrayobject *self, PyObject *v) } /*[clinic input] +@critical_section array.array.byteswap Byteswap all items of the array. @@ -1516,13 +1527,11 @@ raised. static PyObject * array_array_byteswap_impl(arrayobject *self) -/*[clinic end generated code: output=5f8236cbdf0d90b5 input=6a85591b950a0186]*/ +/*[clinic end generated code: output=5f8236cbdf0d90b5 input=e691b6eff94d8b2e]*/ { - PyObject *ret = Py_None; char *p; Py_ssize_t i; - Py_BEGIN_CRITICAL_SECTION(self); switch (self->ob_descr->itemsize) { case 1: break; @@ -1562,13 +1571,13 @@ array_array_byteswap_impl(arrayobject *self) default: PyErr_SetString(PyExc_RuntimeError, "don't know how to byteswap this array type"); - ret = NULL; + return NULL; } - Py_END_CRITICAL_SECTION(); - return ret; + Py_RETURN_NONE; } /*[clinic input] +@critical_section array.array.reverse Reverse the order of the items in the array. @@ -1576,7 +1585,7 @@ Reverse the order of the items in the array. static PyObject * array_array_reverse_impl(arrayobject *self) -/*[clinic end generated code: output=c04868b36f6f4089 input=cd904f01b27d966a]*/ +/*[clinic end generated code: output=c04868b36f6f4089 input=e3947e98aed068ed]*/ { Py_ssize_t itemsize = self->ob_descr->itemsize; char *p, *q; @@ -1584,7 +1593,6 @@ array_array_reverse_impl(arrayobject *self) char tmp[256]; /* 8 is probably enough -- but why skimp */ assert((size_t)itemsize <= sizeof(tmp)); - Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) > 1) { for (p = self->ob_item, q = self->ob_item + (Py_SIZE(self) - 1)*itemsize; @@ -1598,7 +1606,6 @@ array_array_reverse_impl(arrayobject *self) memcpy(q, tmp, itemsize); } } - Py_END_CRITICAL_SECTION(); Py_RETURN_NONE; } @@ -1668,6 +1675,7 @@ array_array_fromfile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f, } /*[clinic input] +@critical_section array.array.tofile cls: defining_class @@ -1679,11 +1687,8 @@ Write all items (as machine values) to the file object f. static PyObject * array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) -/*[clinic end generated code: output=4560c628d9c18bc2 input=5a24da7a7b407b52]*/ +/*[clinic end generated code: output=4560c628d9c18bc2 input=a26bc66df57864dd]*/ { - PyObject *ret = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); Py_ssize_t nbytes = Py_SIZE(self) * self->ob_descr->itemsize; /* Write 64K blocks at a time */ /* XXX Make the block size settable */ @@ -1691,10 +1696,8 @@ array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) Py_ssize_t nblocks = (nbytes + BLOCKSIZE - 1) / BLOCKSIZE; Py_ssize_t i; - if (Py_SIZE(self) == 0) { - ret = Py_None; + if (Py_SIZE(self) == 0) goto done; - } array_state *state = get_array_state_by_class(cls); @@ -1709,21 +1712,20 @@ array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) size = nbytes - i*BLOCKSIZE; bytes = PyBytes_FromStringAndSize(ptr, size); if (bytes == NULL) - goto done; + return NULL; res = PyObject_CallMethodOneArg(f, state->str_write, bytes); Py_DECREF(bytes); if (res == NULL) - goto done; + return NULL; Py_DECREF(res); /* drop write result */ } - ret = Py_None; - done: - Py_END_CRITICAL_SECTION(); - return ret; +done: + Py_RETURN_NONE; } /*[clinic input] +@critical_section self list array.array.fromlist list: object @@ -1733,46 +1735,41 @@ Append items to array from list. [clinic start generated code]*/ static PyObject * -array_array_fromlist(arrayobject *self, PyObject *list) -/*[clinic end generated code: output=26411c2d228a3e3f input=be2605a96c49680f]*/ +array_array_fromlist_impl(arrayobject *self, PyObject *list) +/*[clinic end generated code: output=6c23733a68dd68df input=c7c056aaf85d997a]*/ { - PyObject *ret = NULL; Py_ssize_t n; if (!PyList_Check(list)) { PyErr_SetString(PyExc_TypeError, "arg must be list"); return NULL; } - - Py_BEGIN_CRITICAL_SECTION2(self, list); n = PyList_Size(list); if (n > 0) { Py_ssize_t i, old_size; old_size = Py_SIZE(self); if (array_resize(self, old_size + n) == -1) - goto done; + return NULL; for (i = 0; i < n; i++) { PyObject *v = PyList_GET_ITEM(list, i); if ((*self->ob_descr->setitem)(self, Py_SIZE(self) - n + i, v) != 0) { array_resize(self, old_size); - goto done; + return NULL; } if (n != PyList_GET_SIZE(list)) { PyErr_SetString(PyExc_RuntimeError, "list changed size during iteration"); array_resize(self, old_size); - goto done; + return NULL; } } } - ret = Py_None; -done: - Py_END_CRITICAL_SECTION2(); - return ret; + Py_RETURN_NONE; } /*[clinic input] +@critical_section array.array.tolist Convert array to an ordinary list with the same items. @@ -1780,71 +1777,73 @@ Convert array to an ordinary list with the same items. static PyObject * array_array_tolist_impl(arrayobject *self) -/*[clinic end generated code: output=00b60cc9eab8ef89 input=a8d7784a94f86b53]*/ +/*[clinic end generated code: output=00b60cc9eab8ef89 input=4543fdbac475c52c]*/ { PyObject *list = PyList_New(Py_SIZE(self)); Py_ssize_t i; if (list == NULL) return NULL; - Py_BEGIN_CRITICAL_SECTION(self); for (i = 0; i < Py_SIZE(self); i++) { PyObject *v = getarrayitem((PyObject *)self, i); - if (v == NULL) { - Py_DECREF(list); - list = NULL; + if (v == NULL) goto error; - } PyList_SET_ITEM(list, i, v); } + return list; error: - Py_END_CRITICAL_SECTION(); - return list; + Py_DECREF(list); + return NULL; } + static PyObject * -frombytes(arrayobject *self, PyObject *bytes) +frombytes_lock_held(arrayobject *self, PyObject *bytes) { - PyObject *ret = NULL; Py_buffer buffer = {NULL, NULL}; - - Py_BEGIN_CRITICAL_SECTION2(self, bytes); if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE) != 0) { - goto done; + return NULL; } - int itemsize = self->ob_descr->itemsize; Py_ssize_t n; if (buffer.itemsize != 1) { + PyBuffer_Release(&buffer); PyErr_SetString(PyExc_TypeError, "a bytes-like object is required"); - goto done; + return NULL; } n = buffer.len; if (n % itemsize != 0) { + PyBuffer_Release(&buffer); PyErr_SetString(PyExc_ValueError, "bytes length not a multiple of item size"); - goto done; + return NULL; } n = n / itemsize; if (n > 0) { Py_ssize_t old_size = Py_SIZE(self); if ((n > PY_SSIZE_T_MAX - old_size) || ((old_size + n) > PY_SSIZE_T_MAX / itemsize)) { + PyBuffer_Release(&buffer); return PyErr_NoMemory(); } if (array_resize(self, old_size + n) == -1) { - goto done; + PyBuffer_Release(&buffer); + return NULL; } memcpy(self->ob_item + old_size * itemsize, buffer.buf, n * itemsize); } - ret = Py_None; -done: - // check below depends on Limited API and stable ABI - if (buffer.obj != NULL) { - PyBuffer_Release(&buffer); - } + PyBuffer_Release(&buffer); + Py_RETURN_NONE; +} + +static PyObject * +frombytes(arrayobject *self, PyObject *bytes) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION2(self, bytes); + ret = frombytes_lock_held(self, bytes); Py_END_CRITICAL_SECTION2(); return ret; } @@ -1866,6 +1865,7 @@ array_array_frombytes(arrayobject *self, PyObject *bytes) } /*[clinic input] +@critical_section array.array.tobytes Convert the array to an array of machine values and return the bytes representation. @@ -1873,22 +1873,18 @@ Convert the array to an array of machine values and return the bytes representat static PyObject * array_array_tobytes_impl(arrayobject *self) -/*[clinic end generated code: output=87318e4edcdc2bb6 input=90ee495f96de34f5]*/ +/*[clinic end generated code: output=87318e4edcdc2bb6 input=c4d44d5499d2320f]*/ { - PyObject *ret; - - Py_BEGIN_CRITICAL_SECTION(self); if (Py_SIZE(self) <= PY_SSIZE_T_MAX / self->ob_descr->itemsize) { - ret = PyBytes_FromStringAndSize(self->ob_item, + return PyBytes_FromStringAndSize(self->ob_item, Py_SIZE(self) * self->ob_descr->itemsize); } else { - ret = PyErr_NoMemory(); + return PyErr_NoMemory(); } - Py_END_CRITICAL_SECTION(); - return ret; } /*[clinic input] +@critical_section array.array.fromunicode ustr: unicode @@ -1903,10 +1899,8 @@ some other type. static PyObject * array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) -/*[clinic end generated code: output=24359f5e001a7f2b input=025db1fdade7a4ce]*/ +/*[clinic end generated code: output=24359f5e001a7f2b input=01e2a776cee82011]*/ { - PyObject *ret = NULL; - int typecode = self->ob_descr->typecode; if (typecode != 'u' && typecode != 'w') { PyErr_SetString(PyExc_ValueError, @@ -1915,7 +1909,6 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) return NULL; } - Py_BEGIN_CRITICAL_SECTION(self); if (typecode == 'u') { Py_ssize_t ustr_length = PyUnicode_AsWideChar(ustr, NULL, 0); assert(ustr_length > 0); @@ -1923,7 +1916,7 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) ustr_length--; /* trim trailing NUL character */ Py_ssize_t old_size = Py_SIZE(self); if (array_resize(self, old_size + ustr_length) == -1) { - goto done; + return NULL; } // must not fail @@ -1937,11 +1930,10 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) Py_ssize_t new_size = old_size + ustr_length; if (new_size < 0 || (size_t)new_size > PY_SSIZE_T_MAX / sizeof(Py_UCS4)) { - PyErr_NoMemory(); - goto done; + return PyErr_NoMemory(); } if (array_resize(self, new_size) == -1) { - goto done; + return NULL; } // must not fail @@ -1950,13 +1942,12 @@ array_array_fromunicode_impl(arrayobject *self, PyObject *ustr) assert(u != NULL); (void)u; // Suppress unused_variable warning. } - ret = Py_None; -done: - Py_END_CRITICAL_SECTION(); - return ret; + + Py_RETURN_NONE; } /*[clinic input] +@critical_section array.array.tounicode Extends this array with data from the unicode string ustr. @@ -1968,26 +1959,22 @@ unicode string from an array of some other type. static PyObject * array_array_tounicode_impl(arrayobject *self) -/*[clinic end generated code: output=08e442378336e1ef input=127242eebe70b66d]*/ +/*[clinic end generated code: output=08e442378336e1ef input=6c69dfe81a279b91]*/ { - PyObject *ret; int typecode = self->ob_descr->typecode; if (typecode != 'u' && typecode != 'w') { PyErr_SetString(PyExc_ValueError, "tounicode() may only be called on unicode type arrays ('u' or 'w')"); return NULL; } - Py_BEGIN_CRITICAL_SECTION(self); if (typecode == 'u') { - ret = PyUnicode_FromWideChar((wchar_t *) self->ob_item, Py_SIZE(self)); + return PyUnicode_FromWideChar((wchar_t *) self->ob_item, Py_SIZE(self)); } else { // typecode == 'w' int byteorder = 0; // native byteorder - ret = PyUnicode_DecodeUTF32((const char *) self->ob_item, Py_SIZE(self) * 4, + return PyUnicode_DecodeUTF32((const char *) self->ob_item, Py_SIZE(self) * 4, NULL, &byteorder); } - Py_END_CRITICAL_SECTION(); - return ret; } /*[clinic input] @@ -2397,6 +2384,7 @@ array__array_reconstructor_impl(PyObject *module, PyTypeObject *arraytype, } /*[clinic input] +@critical_section array.array.__reduce_ex__ cls: defining_class @@ -2409,7 +2397,7 @@ Return state information for pickling. static PyObject * array_array___reduce_ex___impl(arrayobject *self, PyTypeObject *cls, PyObject *value) -/*[clinic end generated code: output=4958ee5d79452ad5 input=19968cf0f91d3eea]*/ +/*[clinic end generated code: output=4958ee5d79452ad5 input=18c90a4cad7ac527]*/ { PyObject *dict; PyObject *result; @@ -2535,19 +2523,17 @@ static PyMethodDef array_methods[] = { }; static PyObject * -array_repr(arrayobject *a) +array_repr_lock_held(arrayobject *a) { char typecode; - PyObject *v = NULL, *ret = NULL; + PyObject *s, *v = NULL; Py_ssize_t len; - Py_BEGIN_CRITICAL_SECTION(a); len = Py_SIZE(a); typecode = a->ob_descr->typecode; if (len == 0) { - ret = PyUnicode_FromFormat("%s('%c')", - _PyType_Name(Py_TYPE(a)), (int)typecode); - goto done; + return PyUnicode_FromFormat("%s('%c')", + _PyType_Name(Py_TYPE(a)), (int)typecode); } if (typecode == 'u' || typecode == 'w') { v = array_array_tounicode_impl(a); @@ -2555,32 +2541,37 @@ array_repr(arrayobject *a) v = array_array_tolist_impl(a); } if (v == NULL) - goto done; + return NULL; - ret = PyUnicode_FromFormat("%s('%c', %R)", - _PyType_Name(Py_TYPE(a)), (int)typecode, v); + s = PyUnicode_FromFormat("%s('%c', %R)", + _PyType_Name(Py_TYPE(a)), (int)typecode, v); Py_DECREF(v); -done: + return s; +} + +static PyObject * +array_repr(arrayobject *a) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(a); + ret = array_repr_lock_held(a); Py_END_CRITICAL_SECTION(); return ret; } static PyObject* -array_subscr(arrayobject* self, PyObject* item) +array_subscr_lock_held(arrayobject* self, PyObject* item) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(Py_TYPE(self)); - Py_BEGIN_CRITICAL_SECTION(self); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); if (i==-1 && PyErr_Occurred()) { - goto done; + return NULL; } if (i < 0) i += Py_SIZE(self); - ret = array_item(self, i); - goto done; + return array_item(self, i); } else if (PySlice_Check(item)) { Py_ssize_t start, stop, step, slicelength, i; @@ -2590,30 +2581,27 @@ array_subscr(arrayobject* self, PyObject* item) int itemsize = self->ob_descr->itemsize; if (PySlice_Unpack(item, &start, &stop, &step) < 0) { - goto done; + return NULL; } slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop, step); if (slicelength <= 0) { - ret = newarrayobject(state->ArrayType, 0, self->ob_descr); - goto done; + return newarrayobject(state->ArrayType, 0, self->ob_descr); } else if (step == 1) { PyObject *result = newarrayobject(state->ArrayType, slicelength, self->ob_descr); if (result == NULL) - goto done; + return NULL; memcpy(((arrayobject *)result)->ob_item, self->ob_item + start * itemsize, slicelength * itemsize); - ret = result; - goto done; + return result; } else { result = newarrayobject(state->ArrayType, slicelength, self->ob_descr); - if (!result) - goto done; + if (!result) return NULL; ar = (arrayobject*)result; @@ -2624,40 +2612,45 @@ array_subscr(arrayobject* self, PyObject* item) itemsize); } - ret = result; - goto done; + return result; } } else { PyErr_SetString(PyExc_TypeError, "array indices must be integers"); + return NULL; } -done: +} + +static PyObject* +array_subscr(arrayobject* self, PyObject* item) +{ + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(self); + ret = array_subscr_lock_held(self, item); Py_END_CRITICAL_SECTION(); return ret; } static int -array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) +array_ass_subscr_lock_held(arrayobject* self, PyObject* item, PyObject* value) { - int ret = -1; Py_ssize_t start, stop, step, slicelength, needed; array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; int itemsize; - Py_BEGIN_CRITICAL_SECTION2(self, value); if (PyIndex_Check(item)) { Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError); if (i == -1 && PyErr_Occurred()) - goto done; + return -1; if (i < 0) i += Py_SIZE(self); if (i < 0 || i >= Py_SIZE(self)) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); - goto done; + return -1; } if (value == NULL) { /* Fall through to slice assignment */ @@ -2666,14 +2659,12 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) step = 1; slicelength = 1; } - else { - ret = (*self->ob_descr->setitem)(self, i, value); - goto done; - } + else + return (*self->ob_descr->setitem)(self, i, value); } else if (PySlice_Check(item)) { if (PySlice_Unpack(item, &start, &stop, &step) < 0) { - goto done; + return -1; } slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop, step); @@ -2681,7 +2672,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) else { PyErr_SetString(PyExc_TypeError, "array indices must be integers"); - goto done; + return -1; } if (value == NULL) { other = NULL; @@ -2692,23 +2683,24 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) needed = Py_SIZE(other); if (self == other) { /* Special case "self[i:j] = self" -- copy self first */ + int ret; value = array_slice(other, 0, needed); if (value == NULL) - goto done; - ret = array_ass_subscr(self, item, value); + return -1; + ret = array_ass_subscr_lock_held(self, item, value); Py_DECREF(value); - goto done; + return ret; } if (other->ob_descr != self->ob_descr) { PyErr_BadArgument(); - goto done; + return -1; } } else { PyErr_Format(PyExc_TypeError, "can only assign array (not \"%.200s\") to array slice", Py_TYPE(value)->tp_name); - goto done; + return -1; } itemsize = self->ob_descr->itemsize; /* for 'a[2:1] = ...', the insertion point is 'start', not 'stop' */ @@ -2722,7 +2714,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) if ((needed == 0 || slicelength != needed) && self->ob_exports > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); - goto done; + return -1; } if (step == 1) { @@ -2732,12 +2724,12 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) (Py_SIZE(self) - stop) * itemsize); if (array_resize(self, Py_SIZE(self) + needed - slicelength) < 0) - goto done; + return -1; } else if (slicelength < needed) { if (array_resize(self, Py_SIZE(self) + needed - slicelength) < 0) - goto done; + return -1; memmove(self->ob_item + (start + needed) * itemsize, self->ob_item + stop * itemsize, (Py_SIZE(self) - start - needed) * itemsize); @@ -2745,6 +2737,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) if (needed > 0) memcpy(self->ob_item + start * itemsize, other->ob_item, needed * itemsize); + return 0; } else if (needed == 0) { /* Delete slice */ @@ -2773,7 +2766,8 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) (Py_SIZE(self) - cur) * itemsize); } if (array_resize(self, Py_SIZE(self) - slicelength) < 0) - goto done; + return -1; + return 0; } else { size_t cur; @@ -2784,7 +2778,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) "attempt to assign array of size %zd " "to extended slice of size %zd", needed, slicelength); - goto done; + return -1; } for (cur = start, i = 0; i < slicelength; cur += step, i++) { @@ -2792,10 +2786,24 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) other->ob_item + i * itemsize, itemsize); } + return 0; + } +} + +static int +array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) +{ + int ret; + if (value == NULL) { + Py_BEGIN_CRITICAL_SECTION(self); + ret = array_ass_subscr_lock_held(self, item, value); + Py_END_CRITICAL_SECTION(); + } + else { + Py_BEGIN_CRITICAL_SECTION2(self, value); + ret = array_ass_subscr_lock_held(self, item, value); + Py_END_CRITICAL_SECTION2(); } - ret = 0; -done: - Py_END_CRITICAL_SECTION2(); return ret; } @@ -2851,59 +2859,13 @@ array_buffer_relbuf(arrayobject *self, Py_buffer *view) } static PyObject * -array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +array_new_internal_lock_held(PyTypeObject *type, PyObject *initial, int c) { - PyObject *ret = NULL; array_state *state = find_array_state_by_type(type); - int c; - PyObject *initial = NULL, *it = NULL; + PyObject *it = NULL; const struct arraydescr *descr; - - if ((type == state->ArrayType || - type->tp_init == state->ArrayType->tp_init) && - !_PyArg_NoKeywords("array.array", kwds)) - return NULL; - - if (!PyArg_ParseTuple(args, "C|O:array", &c, &initial)) - return NULL; - - if (PySys_Audit("array.__new__", "CO", - c, initial ? initial : Py_None) < 0) { - return NULL; - } - - if (c == 'u') { - if (PyErr_WarnEx(PyExc_DeprecationWarning, - "The 'u' type code is deprecated and " - "will be removed in Python 3.16", - 1)) { - return NULL; - } - } - bool is_unicode = c == 'u' || c == 'w'; - if (initial && !is_unicode) { - if (PyUnicode_Check(initial)) { - PyErr_Format(PyExc_TypeError, "cannot use a str to initialize " - "an array with typecode '%c'", c); - return NULL; - } - else if (array_Check(initial, state)) { - int ic = ((arrayobject*)initial)->ob_descr->typecode; - if (ic == 'u' || ic == 'w') { - PyErr_Format(PyExc_TypeError, "cannot use a unicode array to " - "initialize an array with typecode '%c'", c); - return NULL; - } - } - } - - /* If the initial object is NULL then we use args as a stand-in object - for the critical section. */ - PyObject *lock_obj = initial != NULL ? initial : args; - - Py_BEGIN_CRITICAL_SECTION(lock_obj); if (!(initial == NULL || PyList_Check(initial) || PyByteArray_Check(initial) || PyBytes_Check(initial) @@ -2913,7 +2875,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) && c == ((arrayobject*)initial)->ob_descr->typecode))) { it = PyObject_GetIter(initial); if (it == NULL) - goto done; + return NULL; /* We set initial to NULL so that the subsequent code will create an empty array of the appropriate type and afterwards we can use array_iter_extend to populate @@ -2937,7 +2899,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) a = newarrayobject(type, len, descr); if (a == NULL) - goto done; + return NULL; if (len > 0 && !array_Check(initial, state)) { Py_ssize_t i; @@ -2946,12 +2908,12 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PySequence_GetItem(initial, i); if (v == NULL) { Py_DECREF(a); - goto done; + return NULL; } if (setarrayitem(a, i, v) != 0) { Py_DECREF(v); Py_DECREF(a); - goto done; + return NULL; } Py_DECREF(v); } @@ -2963,7 +2925,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) initial); if (v == NULL) { Py_DECREF(a); - goto done; + return NULL; } Py_DECREF(v); } @@ -2973,7 +2935,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) wchar_t *ustr = PyUnicode_AsWideCharString(initial, &n); if (ustr == NULL) { Py_DECREF(a); - goto done; + return NULL; } if (n > 0) { @@ -2990,7 +2952,7 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) Py_UCS4 *ustr = PyUnicode_AsUCS4Copy(initial); if (ustr == NULL) { Py_DECREF(a); - goto done; + return NULL; } arrayobject *self = (arrayobject *)a; @@ -3010,18 +2972,76 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (array_iter_extend((arrayobject *)a, it) == -1) { Py_DECREF(it); Py_DECREF(a); - goto done; + return NULL; } Py_DECREF(it); } - ret = a; - goto done; + return a; } } PyErr_SetString(PyExc_ValueError, "bad typecode (must be b, B, u, h, H, i, I, l, L, q, Q, f or d)"); -done: - Py_END_CRITICAL_SECTION(); + return NULL; +} + +static PyObject * +array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) +{ + array_state *state = find_array_state_by_type(type); + int c; + PyObject *initial = NULL; + + if ((type == state->ArrayType || + type->tp_init == state->ArrayType->tp_init) && + !_PyArg_NoKeywords("array.array", kwds)) + return NULL; + + if (!PyArg_ParseTuple(args, "C|O:array", &c, &initial)) + return NULL; + + if (PySys_Audit("array.__new__", "CO", + c, initial ? initial : Py_None) < 0) { + return NULL; + } + + if (c == 'u') { + if (PyErr_WarnEx(PyExc_DeprecationWarning, + "The 'u' type code is deprecated and " + "will be removed in Python 3.16", + 1)) { + return NULL; + } + } + + bool is_unicode = c == 'u' || c == 'w'; + + if (initial && !is_unicode) { + if (PyUnicode_Check(initial)) { + PyErr_Format(PyExc_TypeError, "cannot use a str to initialize " + "an array with typecode '%c'", c); + return NULL; + } + else if (array_Check(initial, state)) { + int ic = ((arrayobject*)initial)->ob_descr->typecode; + if (ic == 'u' || ic == 'w') { + PyErr_Format(PyExc_TypeError, "cannot use a unicode array to " + "initialize an array with typecode '%c'", c); + return NULL; + } + } + } + + PyObject *ret; + + if (initial == NULL) { + ret = array_new_internal_lock_held(type, initial, c); + } + else { + Py_BEGIN_CRITICAL_SECTION(initial); + ret = array_new_internal_lock_held(type, initial, c); + Py_END_CRITICAL_SECTION(); + } + return ret; } diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 1087da12606b9f..830a76248fc9c7 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -6,6 +6,7 @@ preserve # include "pycore_runtime.h" // _Py_SINGLETON() #endif #include "pycore_abstract.h" // _PyNumber_Index() +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_CheckPositional() PyDoc_STRVAR(array_array_clear__doc__, @@ -23,7 +24,13 @@ array_array_clear_impl(arrayobject *self); static PyObject * array_array_clear(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_clear_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_clear_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array___copy____doc__, @@ -41,7 +48,13 @@ array_array___copy___impl(arrayobject *self); static PyObject * array_array___copy__(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array___copy___impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array___copy___impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array___deepcopy____doc__, @@ -53,6 +66,21 @@ PyDoc_STRVAR(array_array___deepcopy____doc__, #define ARRAY_ARRAY___DEEPCOPY___METHODDEF \ {"__deepcopy__", (PyCFunction)array_array___deepcopy__, METH_O, array_array___deepcopy____doc__}, +static PyObject * +array_array___deepcopy___impl(arrayobject *self, PyObject *unused); + +static PyObject * +array_array___deepcopy__(arrayobject *self, PyObject *unused) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array___deepcopy___impl(self, unused); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(array_array_count__doc__, "count($self, v, /)\n" "--\n" @@ -62,6 +90,21 @@ PyDoc_STRVAR(array_array_count__doc__, #define ARRAY_ARRAY_COUNT_METHODDEF \ {"count", (PyCFunction)array_array_count, METH_O, array_array_count__doc__}, +static PyObject * +array_array_count_impl(arrayobject *self, PyObject *v); + +static PyObject * +array_array_count(arrayobject *self, PyObject *v) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_count_impl(self, v); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(array_array_index__doc__, "index($self, v, start=0, stop=sys.maxsize, /)\n" "--\n" @@ -102,7 +145,9 @@ array_array_index(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) goto exit; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_index_impl(self, v, start, stop); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -117,6 +162,21 @@ PyDoc_STRVAR(array_array_remove__doc__, #define ARRAY_ARRAY_REMOVE_METHODDEF \ {"remove", (PyCFunction)array_array_remove, METH_O, array_array_remove__doc__}, +static PyObject * +array_array_remove_impl(arrayobject *self, PyObject *v); + +static PyObject * +array_array_remove(arrayobject *self, PyObject *v) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_remove_impl(self, v); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(array_array_pop__doc__, "pop($self, i=-1, /)\n" "--\n" @@ -156,7 +216,9 @@ array_array_pop(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) i = ival; } skip_optional: + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_pop_impl(self, i); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -265,7 +327,13 @@ array_array_buffer_info_impl(arrayobject *self); static PyObject * array_array_buffer_info(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_buffer_info_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_buffer_info_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array_append__doc__, @@ -295,7 +363,13 @@ array_array_byteswap_impl(arrayobject *self); static PyObject * array_array_byteswap(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_byteswap_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_byteswap_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array_reverse__doc__, @@ -313,7 +387,13 @@ array_array_reverse_impl(arrayobject *self); static PyObject * array_array_reverse(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_reverse_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_reverse_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array_fromfile__doc__, @@ -412,7 +492,9 @@ array_array_tofile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, goto exit; } f = args[0]; + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_tofile_impl(self, cls, f); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -427,6 +509,21 @@ PyDoc_STRVAR(array_array_fromlist__doc__, #define ARRAY_ARRAY_FROMLIST_METHODDEF \ {"fromlist", (PyCFunction)array_array_fromlist, METH_O, array_array_fromlist__doc__}, +static PyObject * +array_array_fromlist_impl(arrayobject *self, PyObject *list); + +static PyObject * +array_array_fromlist(arrayobject *self, PyObject *list) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(self, list); + return_value = array_array_fromlist_impl(self, list); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(array_array_tolist__doc__, "tolist($self, /)\n" "--\n" @@ -442,7 +539,13 @@ array_array_tolist_impl(arrayobject *self); static PyObject * array_array_tolist(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_tolist_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_tolist_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array_frombytes__doc__, @@ -469,7 +572,13 @@ array_array_tobytes_impl(arrayobject *self); static PyObject * array_array_tobytes(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_tobytes_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_tobytes_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array_fromunicode__doc__, @@ -499,7 +608,9 @@ array_array_fromunicode(arrayobject *self, PyObject *arg) goto exit; } ustr = arg; + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_fromunicode_impl(self, ustr); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -524,7 +635,13 @@ array_array_tounicode_impl(arrayobject *self); static PyObject * array_array_tounicode(arrayobject *self, PyObject *Py_UNUSED(ignored)) { - return array_array_tounicode_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_tounicode_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(array_array___sizeof____doc__, @@ -636,7 +753,9 @@ array_array___reduce_ex__(arrayobject *self, PyTypeObject *cls, PyObject *const goto exit; } value = args[0]; + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array___reduce_ex___impl(self, cls, value); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -672,4 +791,4 @@ PyDoc_STRVAR(array_arrayiterator___setstate____doc__, #define ARRAY_ARRAYITERATOR___SETSTATE___METHODDEF \ {"__setstate__", (PyCFunction)array_arrayiterator___setstate__, METH_O, array_arrayiterator___setstate____doc__}, -/*[clinic end generated code: output=02996fe706cfd51b input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b00b42399d49b14b input=a9049054013a1b77]*/ From 65b4a63280133b8536de442420e2a1784d17327d Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 13:15:14 -0500 Subject: [PATCH 06/31] made arrayiter freethread safe --- Modules/arraymodule.c | 51 ++++++++++++++++++++++++++-------- Modules/clinic/arraymodule.c.h | 28 +++++++++++++++++-- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 482a37b2616a41..cae5eec7b7ce00 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3193,11 +3193,10 @@ array_iter(arrayobject *ao) } static PyObject * -arrayiter_next(arrayiterobject *it) +arrayiter_next_lock_held(arrayiterobject *it) { arrayobject *ao; - assert(it != NULL); #ifndef NDEBUG array_state *state = find_array_state_by_type(Py_TYPE(it)); assert(PyObject_TypeCheck(it, state->ArrayIterType)); @@ -3206,17 +3205,35 @@ arrayiter_next(arrayiterobject *it) if (ao == NULL) { return NULL; } + PyObject *ret = NULL; + Py_BEGIN_CRITICAL_SECTION(ao); #ifndef NDEBUG assert(array_Check(ao, state)); #endif if (it->index < Py_SIZE(ao)) { - return (*it->getitem)(ao, it->index++); + ret = (*it->getitem)(ao, it->index++); } + Py_END_CRITICAL_SECTION(); + if (ret != NULL) + return ret; it->ao = NULL; Py_DECREF(ao); return NULL; } +static PyObject * +arrayiter_next(arrayiterobject *it) +{ + PyObject *ret; + assert(it != NULL); + + Py_BEGIN_CRITICAL_SECTION(it); + ret = arrayiter_next_lock_held(it); + Py_END_CRITICAL_SECTION(); + + return ret; +} + static void arrayiter_dealloc(arrayiterobject *it) { @@ -3237,6 +3254,7 @@ arrayiter_traverse(arrayiterobject *it, visitproc visit, void *arg) } /*[clinic input] +@critical_section array.arrayiterator.__reduce__ cls: defining_class @@ -3247,9 +3265,8 @@ Return state information for pickling. static PyObject * array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) -/*[clinic end generated code: output=4b032417a2c8f5e6 input=ac64e65a87ad452e]*/ +/*[clinic end generated code: output=4b032417a2c8f5e6 input=61ad213fe49ae0f7]*/ { - array_state *state = get_array_state_by_class(cls); assert(state != NULL); PyObject *func = _PyEval_GetBuiltin(state->str_iter); @@ -3260,6 +3277,7 @@ array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) } /*[clinic input] +@critical_section array.arrayiterator.__setstate__ state: object @@ -3269,17 +3287,26 @@ Set state information for unpickling. [clinic start generated code]*/ static PyObject * -array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) -/*[clinic end generated code: output=397da9904e443cbe input=f47d5ceda19e787b]*/ +array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state) +/*[clinic end generated code: output=d7837ae4ac1fd8b9 input=8d8dc7ce40b9c1f7]*/ { Py_ssize_t index = PyLong_AsSsize_t(state); + if (index == -1 && PyErr_Occurred()) return NULL; - if (index < 0) - index = 0; - else if (index > Py_SIZE(self->ao)) - index = Py_SIZE(self->ao); /* iterator exhausted */ - self->index = index; + + arrayobject *ao = self->ao; + + if (ao != NULL) { + Py_BEGIN_CRITICAL_SECTION(ao); + if (index < 0) + index = 0; + else if (index > Py_SIZE(self->ao)) + index = Py_SIZE(self->ao); /* iterator exhausted */ + Py_END_CRITICAL_SECTION(); + self->index = index; + } + Py_RETURN_NONE; } diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 830a76248fc9c7..4f50775804c1dc 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -776,11 +776,18 @@ array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls); static PyObject * array_arrayiterator___reduce__(arrayiterobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { + PyObject *return_value = NULL; + if (nargs || (kwnames && PyTuple_GET_SIZE(kwnames))) { PyErr_SetString(PyExc_TypeError, "__reduce__() takes no arguments"); - return NULL; + goto exit; } - return array_arrayiterator___reduce___impl(self, cls); + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_arrayiterator___reduce___impl(self, cls); + Py_END_CRITICAL_SECTION(); + +exit: + return return_value; } PyDoc_STRVAR(array_arrayiterator___setstate____doc__, @@ -791,4 +798,19 @@ PyDoc_STRVAR(array_arrayiterator___setstate____doc__, #define ARRAY_ARRAYITERATOR___SETSTATE___METHODDEF \ {"__setstate__", (PyCFunction)array_arrayiterator___setstate__, METH_O, array_arrayiterator___setstate____doc__}, -/*[clinic end generated code: output=b00b42399d49b14b input=a9049054013a1b77]*/ + +static PyObject * +array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state); + +static PyObject * +array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_arrayiterator___setstate___impl(self, state); + Py_END_CRITICAL_SECTION(); + + return return_value; +} +/*[clinic end generated code: output=c576d61ae3881a07 input=a9049054013a1b77]*/ From 761d27a3efad1a4220e65d292cca761880b09f24 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 16:38:54 -0500 Subject: [PATCH 07/31] cleanup to match other segfault fix PR --- Modules/arraymodule.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index cae5eec7b7ce00..6d2ed278ff8396 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3291,22 +3291,18 @@ array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state) /*[clinic end generated code: output=d7837ae4ac1fd8b9 input=8d8dc7ce40b9c1f7]*/ { Py_ssize_t index = PyLong_AsSsize_t(state); - if (index == -1 && PyErr_Occurred()) return NULL; - arrayobject *ao = self->ao; - if (ao != NULL) { Py_BEGIN_CRITICAL_SECTION(ao); if (index < 0) index = 0; - else if (index > Py_SIZE(self->ao)) - index = Py_SIZE(self->ao); /* iterator exhausted */ + else if (index > Py_SIZE(ao)) + index = Py_SIZE(ao); /* iterator exhausted */ Py_END_CRITICAL_SECTION(); self->index = index; } - Py_RETURN_NONE; } From 0bcd3c7896ff7de65b711334269e707423ef8683 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 16:59:20 -0500 Subject: [PATCH 08/31] style nit from other PR --- Modules/arraymodule.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 6d2ed278ff8396..338088ad03b116 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3296,10 +3296,11 @@ array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state) arrayobject *ao = self->ao; if (ao != NULL) { Py_BEGIN_CRITICAL_SECTION(ao); - if (index < 0) + if (index < 0) { index = 0; - else if (index > Py_SIZE(ao)) + } else if (index > Py_SIZE(ao)) { index = Py_SIZE(ao); /* iterator exhausted */ + } Py_END_CRITICAL_SECTION(); self->index = index; } From 5676c9219a236dfe4d03ed525a032b96b547b8c4 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 18:38:57 -0500 Subject: [PATCH 09/31] add _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED() --- Modules/arraymodule.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 338088ad03b116..b3ab97ea5edbdf 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -739,6 +739,8 @@ array_dealloc(arrayobject *op) static PyObject * array_richcompare_lock_held(PyObject *v, PyObject *w, int op) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(v); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(w); array_state *state = find_array_state_by_type(Py_TYPE(v)); arrayobject *va, *wa; PyObject *vi = NULL; @@ -869,6 +871,7 @@ array_length(arrayobject *a) static PyObject * array_item_lock_held(arrayobject *a, Py_ssize_t i) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array index out of range"); return NULL; @@ -963,6 +966,8 @@ array_array___deepcopy___impl(arrayobject *self, PyObject *unused) static PyObject * array_concat_lock_held(arrayobject *a, PyObject *bb) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bb); array_state *state = find_array_state_by_type(Py_TYPE(a)); Py_ssize_t size; arrayobject *np; @@ -1009,6 +1014,7 @@ array_concat(arrayobject *a, PyObject *bb) static PyObject * array_repeat_lock_held(arrayobject *a, Py_ssize_t n) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); array_state *state = find_array_state_by_type(Py_TYPE(a)); if (n < 0) @@ -1079,6 +1085,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) static int array_ass_item_lock_held(arrayobject *a, Py_ssize_t i, PyObject *v) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); if (i < 0 || i >= Py_SIZE(a)) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); @@ -1135,6 +1142,8 @@ array_iter_extend(arrayobject *self, PyObject *bb) static int array_do_extend_lock_held(array_state *state, arrayobject *self, PyObject *bb) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bb); Py_ssize_t size, oldsize, bbsize; if (!array_Check(bb, state)) @@ -1194,6 +1203,7 @@ array_inplace_concat(arrayobject *self, PyObject *bb) static PyObject * array_inplace_repeat_lock_held(arrayobject *self, Py_ssize_t n) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); const Py_ssize_t array_size = Py_SIZE(self); if (array_size > 0 && n != 1 ) { @@ -1325,6 +1335,7 @@ array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, static int array_contains_lock_held(arrayobject *self, PyObject *v) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); Py_ssize_t i; int cmp; @@ -1801,6 +1812,8 @@ array_array_tolist_impl(arrayobject *self) static PyObject * frombytes_lock_held(arrayobject *self, PyObject *bytes) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bytes); Py_buffer buffer = {NULL, NULL}; if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE) != 0) { return NULL; @@ -2525,6 +2538,7 @@ static PyMethodDef array_methods[] = { static PyObject * array_repr_lock_held(arrayobject *a) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); char typecode; PyObject *s, *v = NULL; Py_ssize_t len; @@ -2562,6 +2576,7 @@ array_repr(arrayobject *a) static PyObject* array_subscr_lock_held(arrayobject* self, PyObject* item) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); array_state *state = find_array_state_by_type(Py_TYPE(self)); if (PyIndex_Check(item)) { @@ -2635,6 +2650,10 @@ array_subscr(arrayobject* self, PyObject* item) static int array_ass_subscr_lock_held(arrayobject* self, PyObject* item, PyObject* value) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); + if (value != NULL) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); + } Py_ssize_t start, stop, step, slicelength, needed; array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; @@ -2861,6 +2880,9 @@ array_buffer_relbuf(arrayobject *self, Py_buffer *view) static PyObject * array_new_internal_lock_held(PyTypeObject *type, PyObject *initial, int c) { + if (initial != NULL) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(initial); + } array_state *state = find_array_state_by_type(type); PyObject *it = NULL; const struct arraydescr *descr; @@ -3195,6 +3217,7 @@ array_iter(arrayobject *ao) static PyObject * arrayiter_next_lock_held(arrayiterobject *it) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(it); arrayobject *ao; #ifndef NDEBUG From c18290cbd87f13a3f8a2283ba48420123157d38a Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 17 Jan 2025 18:45:41 -0500 Subject: [PATCH 10/31] misc style --- Modules/arraymodule.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index b3ab97ea5edbdf..6d3536e3abbe5b 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3015,11 +3015,13 @@ array_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if ((type == state->ArrayType || type->tp_init == state->ArrayType->tp_init) && - !_PyArg_NoKeywords("array.array", kwds)) + !_PyArg_NoKeywords("array.array", kwds)) { return NULL; + } - if (!PyArg_ParseTuple(args, "C|O:array", &c, &initial)) + if (!PyArg_ParseTuple(args, "C|O:array", &c, &initial)) { return NULL; + } if (PySys_Audit("array.__new__", "CO", c, initial ? initial : Py_None) < 0) { @@ -3237,8 +3239,9 @@ arrayiter_next_lock_held(arrayiterobject *it) ret = (*it->getitem)(ao, it->index++); } Py_END_CRITICAL_SECTION(); - if (ret != NULL) + if (ret != NULL) { return ret; + } it->ao = NULL; Py_DECREF(ao); return NULL; From 6ee23038929b8fcdd0881adcc077144b6fcea1bd Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 18 Jan 2025 07:40:21 -0500 Subject: [PATCH 11/31] misc cleanups --- Modules/arraymodule.c | 51 +++++++++++++--------------------- Modules/clinic/arraymodule.c.h | 34 ++++++++++++++++++++++- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 6d3536e3abbe5b..414149873c5493 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -1236,18 +1236,6 @@ array_inplace_repeat(arrayobject *self, Py_ssize_t n) } -static PyObject * -ins(arrayobject *self, Py_ssize_t where, PyObject *v) -{ - int res; - Py_BEGIN_CRITICAL_SECTION(self); - res = ins1(self, where, v); - Py_END_CRITICAL_SECTION(); - if (res != 0) - return NULL; - Py_RETURN_NONE; -} - /*[clinic input] @critical_section array.array.count @@ -1457,6 +1445,7 @@ array_array_extend_impl(arrayobject *self, PyTypeObject *cls, PyObject *bb) } /*[clinic input] +@critical_section array.array.insert i: Py_ssize_t @@ -1468,9 +1457,11 @@ Insert a new item v into the array before position i. static PyObject * array_array_insert_impl(arrayobject *self, Py_ssize_t i, PyObject *v) -/*[clinic end generated code: output=5a3648e278348564 input=5577d1b4383e9313]*/ +/*[clinic end generated code: output=5a3648e278348564 input=3c922bbd81462978]*/ { - return ins(self, i, v); + if (ins1(self, i, v) != 0) + return NULL; + Py_RETURN_NONE; } /*[clinic input] @@ -1511,6 +1502,7 @@ array_array_buffer_info_impl(arrayobject *self) } /*[clinic input] +@critical_section array.array.append v: object @@ -1520,10 +1512,12 @@ Append new value v to the end of the array. [clinic start generated code]*/ static PyObject * -array_array_append(arrayobject *self, PyObject *v) -/*[clinic end generated code: output=745a0669bf8db0e2 input=0b98d9d78e78f0fa]*/ +array_array_append_impl(arrayobject *self, PyObject *v) +/*[clinic end generated code: output=2f1e8cbad70c2a8b input=9cdd897c66a40c3f]*/ { - return ins(self, Py_SIZE(self), v); + if (ins1(self, Py_SIZE(self), v) != 0) + return NULL; + Py_RETURN_NONE; } /*[clinic input] @@ -1810,7 +1804,7 @@ array_array_tolist_impl(arrayobject *self) static PyObject * -frombytes_lock_held(arrayobject *self, PyObject *bytes) +array_array_frombytes_lock_held(arrayobject *self, PyObject *bytes) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bytes); @@ -1851,17 +1845,8 @@ frombytes_lock_held(arrayobject *self, PyObject *bytes) Py_RETURN_NONE; } -static PyObject * -frombytes(arrayobject *self, PyObject *bytes) -{ - PyObject *ret; - Py_BEGIN_CRITICAL_SECTION2(self, bytes); - ret = frombytes_lock_held(self, bytes); - Py_END_CRITICAL_SECTION2(); - return ret; -} - /*[clinic input] +@critical_section self bytes array.array.frombytes bytes: object @@ -1871,10 +1856,10 @@ Appends items from the string, interpreting it as an array of machine values, as [clinic start generated code]*/ static PyObject * -array_array_frombytes(arrayobject *self, PyObject *bytes) -/*[clinic end generated code: output=3d8413868a91ec1f input=9fcb15a49b73d960]*/ +array_array_frombytes_impl(arrayobject *self, PyObject *bytes) +/*[clinic end generated code: output=8a48da6fa2f9dcde input=f8d97acc9f269f7b]*/ { - return frombytes(self, bytes); + return array_array_frombytes_lock_held(self, bytes); } /*[clinic input] @@ -2651,9 +2636,11 @@ static int array_ass_subscr_lock_held(arrayobject* self, PyObject* item, PyObject* value) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); +#ifdef Py_DEBUG if (value != NULL) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); } +#endif Py_ssize_t start, stop, step, slicelength, needed; array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; @@ -2880,9 +2867,11 @@ array_buffer_relbuf(arrayobject *self, Py_buffer *view) static PyObject * array_new_internal_lock_held(PyTypeObject *type, PyObject *initial, int c) { +#ifdef Py_DEBUG if (initial != NULL) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(initial); } +#endif array_state *state = find_array_state_by_type(type); PyObject *it = NULL; const struct arraydescr *descr; diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 4f50775804c1dc..723b9d7826ea7c 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -303,7 +303,9 @@ array_array_insert(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) i = ival; } v = args[1]; + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_insert_impl(self, i, v); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -345,6 +347,21 @@ PyDoc_STRVAR(array_array_append__doc__, #define ARRAY_ARRAY_APPEND_METHODDEF \ {"append", (PyCFunction)array_array_append, METH_O, array_array_append__doc__}, +static PyObject * +array_array_append_impl(arrayobject *self, PyObject *v); + +static PyObject * +array_array_append(arrayobject *self, PyObject *v) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = array_array_append_impl(self, v); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(array_array_byteswap__doc__, "byteswap($self, /)\n" "--\n" @@ -557,6 +574,21 @@ PyDoc_STRVAR(array_array_frombytes__doc__, #define ARRAY_ARRAY_FROMBYTES_METHODDEF \ {"frombytes", (PyCFunction)array_array_frombytes, METH_O, array_array_frombytes__doc__}, +static PyObject * +array_array_frombytes_impl(arrayobject *self, PyObject *bytes); + +static PyObject * +array_array_frombytes(arrayobject *self, PyObject *bytes) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(self, bytes); + return_value = array_array_frombytes_impl(self, bytes); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(array_array_tobytes__doc__, "tobytes($self, /)\n" "--\n" @@ -813,4 +845,4 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) return return_value; } -/*[clinic end generated code: output=c576d61ae3881a07 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6ec51e036151acac input=a9049054013a1b77]*/ From 7d5de1f50509b6bd2cb5942d5d8f35e59bdad76f Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 18 Jan 2025 07:47:32 -0500 Subject: [PATCH 12/31] misc --- .../Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst | 2 +- Modules/arraymodule.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst index c50e6796f328d4..2b5eb581f1ffa1 100644 --- a/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst +++ b/Misc/NEWS.d/next/Library/2025-01-17-13-53-32.gh-issue-128942.DxzaIg.rst @@ -1 +1 @@ -Make array module safe under :term:`free threading`. +Make the :mod:`array` module safe under :term:`free threading`. diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 414149873c5493..1ce6e9c1a8e779 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3306,14 +3306,16 @@ array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state) /*[clinic end generated code: output=d7837ae4ac1fd8b9 input=8d8dc7ce40b9c1f7]*/ { Py_ssize_t index = PyLong_AsSsize_t(state); - if (index == -1 && PyErr_Occurred()) + if (index == -1 && PyErr_Occurred()) { return NULL; + } arrayobject *ao = self->ao; if (ao != NULL) { Py_BEGIN_CRITICAL_SECTION(ao); if (index < 0) { index = 0; - } else if (index > Py_SIZE(ao)) { + } + else if (index > Py_SIZE(ao)) { index = Py_SIZE(ao); /* iterator exhausted */ } Py_END_CRITICAL_SECTION(); From 0ad980040ba11f225740582ce2b303737f168177 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 18 Jan 2025 16:06:34 -0500 Subject: [PATCH 13/31] 3 og 4 requested changes --- Modules/arraymodule.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 1ce6e9c1a8e779..e0bc00b2fed50b 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -760,7 +760,7 @@ array_richcompare_lock_held(PyObject *v, PyObject *w, int op) res = Py_False; else res = Py_True; - return res; + return Py_NewRef(res); } if (va->ob_descr == wa->ob_descr && va->ob_descr->compareitems != NULL) { @@ -783,7 +783,7 @@ array_richcompare_lock_held(PyObject *v, PyObject *w, int op) default: return NULL; /* cannot happen */ } PyObject *res = cmp ? Py_True : Py_False; - return res; + return Py_NewRef(res); } @@ -829,15 +829,15 @@ array_richcompare_lock_held(PyObject *v, PyObject *w, int op) res = Py_True; else res = Py_False; - return res; + return Py_NewRef(res); } /* We have an item that differs. First, shortcuts for EQ/NE */ if (op == Py_EQ) { - res = Py_False; + res = Py_NewRef(Py_False); } else if (op == Py_NE) { - res = Py_True; + res = Py_NewRef(Py_True); } else { /* Compare the final item again using the proper operator */ @@ -2817,7 +2817,7 @@ static const void *emptybuf = ""; static int -array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) +array_buffer_getbuf_lock_held(arrayobject *self, Py_buffer *view, int flags) { if (view == NULL) { PyErr_SetString(PyExc_BufferError, @@ -2825,7 +2825,6 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) return -1; } - Py_BEGIN_CRITICAL_SECTION(self); view->buf = (void *)self->ob_item; view->obj = Py_NewRef(self); if (view->buf == NULL) @@ -2854,14 +2853,27 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) } self->ob_exports++; - Py_END_CRITICAL_SECTION(); return 0; } +static int +array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) +{ + int ret; + Py_BEGIN_CRITICAL_SECTION(self); + ret = array_buffer_getbuf_lock_held(self, view, flags); + Py_END_CRITICAL_SECTION(); + return ret; +} + static void array_buffer_relbuf(arrayobject *self, Py_buffer *view) { +#ifdef Py_GIL_DISABLED _Py_atomic_add_ssize(&self->ob_exports, -1); +#else + self->ob_exports--; +#endif } static PyObject * From 325c241f93167035420dcb1f8e070ae40bc0cedc Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 18 Jan 2025 16:26:38 -0500 Subject: [PATCH 14/31] not locking bytes --- Modules/arraymodule.c | 32 ++++++++++++-------------------- Modules/clinic/arraymodule.c.h | 6 +++--- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index e0bc00b2fed50b..445da7dd73e1a1 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -1803,11 +1803,20 @@ array_array_tolist_impl(arrayobject *self) } +/*[clinic input] +@critical_section +array.array.frombytes + + bytes: object + / + +Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method. +[clinic start generated code]*/ + static PyObject * -array_array_frombytes_lock_held(arrayobject *self, PyObject *bytes) +array_array_frombytes_impl(arrayobject *self, PyObject *bytes) +/*[clinic end generated code: output=8a48da6fa2f9dcde input=1758523e88df5d98]*/ { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bytes); Py_buffer buffer = {NULL, NULL}; if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE) != 0) { return NULL; @@ -1845,23 +1854,6 @@ array_array_frombytes_lock_held(arrayobject *self, PyObject *bytes) Py_RETURN_NONE; } -/*[clinic input] -@critical_section self bytes -array.array.frombytes - - bytes: object - / - -Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method. -[clinic start generated code]*/ - -static PyObject * -array_array_frombytes_impl(arrayobject *self, PyObject *bytes) -/*[clinic end generated code: output=8a48da6fa2f9dcde input=f8d97acc9f269f7b]*/ -{ - return array_array_frombytes_lock_held(self, bytes); -} - /*[clinic input] @critical_section array.array.tobytes diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 723b9d7826ea7c..6d043d3b5484c3 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -582,9 +582,9 @@ array_array_frombytes(arrayobject *self, PyObject *bytes) { PyObject *return_value = NULL; - Py_BEGIN_CRITICAL_SECTION2(self, bytes); + Py_BEGIN_CRITICAL_SECTION(self); return_value = array_array_frombytes_impl(self, bytes); - Py_END_CRITICAL_SECTION2(); + Py_END_CRITICAL_SECTION(); return return_value; } @@ -845,4 +845,4 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) return return_value; } -/*[clinic end generated code: output=6ec51e036151acac input=a9049054013a1b77]*/ +/*[clinic end generated code: output=a9e86c5a2f55a222 input=a9049054013a1b77]*/ From 7958c552e0a62fc026290b491846089ec7f72a91 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 18 Jan 2025 17:06:02 -0500 Subject: [PATCH 15/31] array_array_frombytes get buffer from clinic --- Modules/arraymodule.c | 21 ++++++--------------- Modules/clinic/arraymodule.c.h | 20 +++++++++++++++----- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 445da7dd73e1a1..947e835e2c94c9 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -1807,30 +1807,24 @@ array_array_tolist_impl(arrayobject *self) @critical_section array.array.frombytes - bytes: object + buffer: Py_buffer / Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method. [clinic start generated code]*/ static PyObject * -array_array_frombytes_impl(arrayobject *self, PyObject *bytes) -/*[clinic end generated code: output=8a48da6fa2f9dcde input=1758523e88df5d98]*/ +array_array_frombytes_impl(arrayobject *self, Py_buffer *buffer) +/*[clinic end generated code: output=d9842c8f7510a516 input=2245f9ea58579960]*/ { - Py_buffer buffer = {NULL, NULL}; - if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE) != 0) { - return NULL; - } int itemsize = self->ob_descr->itemsize; Py_ssize_t n; - if (buffer.itemsize != 1) { - PyBuffer_Release(&buffer); + if (buffer->itemsize != 1) { PyErr_SetString(PyExc_TypeError, "a bytes-like object is required"); return NULL; } - n = buffer.len; + n = buffer->len; if (n % itemsize != 0) { - PyBuffer_Release(&buffer); PyErr_SetString(PyExc_ValueError, "bytes length not a multiple of item size"); return NULL; @@ -1840,17 +1834,14 @@ array_array_frombytes_impl(arrayobject *self, PyObject *bytes) Py_ssize_t old_size = Py_SIZE(self); if ((n > PY_SSIZE_T_MAX - old_size) || ((old_size + n) > PY_SSIZE_T_MAX / itemsize)) { - PyBuffer_Release(&buffer); return PyErr_NoMemory(); } if (array_resize(self, old_size + n) == -1) { - PyBuffer_Release(&buffer); return NULL; } memcpy(self->ob_item + old_size * itemsize, - buffer.buf, n * itemsize); + buffer->buf, n * itemsize); } - PyBuffer_Release(&buffer); Py_RETURN_NONE; } diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 6d043d3b5484c3..123dce8eebc30b 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -566,7 +566,7 @@ array_array_tolist(arrayobject *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(array_array_frombytes__doc__, -"frombytes($self, bytes, /)\n" +"frombytes($self, buffer, /)\n" "--\n" "\n" "Appends items from the string, interpreting it as an array of machine values, as if it had been read from a file using the fromfile() method."); @@ -575,17 +575,27 @@ PyDoc_STRVAR(array_array_frombytes__doc__, {"frombytes", (PyCFunction)array_array_frombytes, METH_O, array_array_frombytes__doc__}, static PyObject * -array_array_frombytes_impl(arrayobject *self, PyObject *bytes); +array_array_frombytes_impl(arrayobject *self, Py_buffer *buffer); static PyObject * -array_array_frombytes(arrayobject *self, PyObject *bytes) +array_array_frombytes(arrayobject *self, PyObject *arg) { PyObject *return_value = NULL; + Py_buffer buffer = {NULL, NULL}; + if (PyObject_GetBuffer(arg, &buffer, PyBUF_SIMPLE) != 0) { + goto exit; + } Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_frombytes_impl(self, bytes); + return_value = array_array_frombytes_impl(self, &buffer); Py_END_CRITICAL_SECTION(); +exit: + /* Cleanup for buffer */ + if (buffer.obj) { + PyBuffer_Release(&buffer); + } + return return_value; } @@ -845,4 +855,4 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) return return_value; } -/*[clinic end generated code: output=a9e86c5a2f55a222 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=2417e21ccf1eb8e6 input=a9049054013a1b77]*/ From 0532d9353d0bda12b997717e6d316ce7c4fc42a1 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sun, 19 Jan 2025 15:46:45 -0500 Subject: [PATCH 16/31] more safe --- Modules/arraymodule.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 947e835e2c94c9..96507b6f838084 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -728,10 +728,17 @@ array_dealloc(arrayobject *op) PyTypeObject *tp = Py_TYPE(op); PyObject_GC_UnTrack(op); - if (op->weakreflist != NULL) + if (op->ob_exports > 0) { + PyErr_SetString(PyExc_SystemError, + "deallocated array object has exported buffers"); + PyErr_Print(); + } + if (op->weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject *) op); - if (op->ob_item != NULL) + } + if (op->ob_item != NULL) { PyMem_Free(op->ob_item); + } tp->tp_free(op); Py_DECREF(tp); } @@ -2835,7 +2842,11 @@ array_buffer_getbuf_lock_held(arrayobject *self, Py_buffer *view, int flags) #endif } +#ifdef Py_GIL_DISABLED + _Py_atomic_add_ssize(&self->ob_exports, 1); +#else self->ob_exports++; +#endif return 0; } @@ -2853,9 +2864,10 @@ static void array_buffer_relbuf(arrayobject *self, Py_buffer *view) { #ifdef Py_GIL_DISABLED - _Py_atomic_add_ssize(&self->ob_exports, -1); + assert(_Py_atomic_add_ssize(&self->ob_exports, -1) >= 1); #else self->ob_exports--; + assert(self->ob_exports >= 0); #endif } From 6afec34d98657e484dadaac1ec04936a74c5d8e2 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Mon, 20 Jan 2025 08:51:19 -0500 Subject: [PATCH 17/31] protect ob_exports exclusively with critical section --- Modules/arraymodule.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 96507b6f838084..09adaeeab1639f 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -2842,11 +2842,7 @@ array_buffer_getbuf_lock_held(arrayobject *self, Py_buffer *view, int flags) #endif } -#ifdef Py_GIL_DISABLED - _Py_atomic_add_ssize(&self->ob_exports, 1); -#else self->ob_exports++; -#endif return 0; } @@ -2863,12 +2859,10 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) static void array_buffer_relbuf(arrayobject *self, Py_buffer *view) { -#ifdef Py_GIL_DISABLED - assert(_Py_atomic_add_ssize(&self->ob_exports, -1) >= 1); -#else + Py_BEGIN_CRITICAL_SECTION(self); self->ob_exports--; assert(self->ob_exports >= 0); -#endif + Py_END_CRITICAL_SECTION(); } static PyObject * From 1bf45fc4cdd5320b4fec4040e7acf42dc6ed848f Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Mon, 20 Jan 2025 10:12:52 -0500 Subject: [PATCH 18/31] fix clinic stuff that seems to have changed --- Modules/arraymodule.c | 2 +- Modules/clinic/arraymodule.c.h | 90 +++++++++++++++++----------------- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index c6f73337a6a808..e836a42aa984c2 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -70,7 +70,7 @@ typedef struct { } array_state; /* Forward declaration. */ -static PyObject *array_array_frombytes(arrayobject *self, PyObject *bytes); +static PyObject *array_array_frombytes(PyObject *self, PyObject *bytes); static array_state * get_array_state(PyObject *module) diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index 123dce8eebc30b..e1d5277daeb98b 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -22,12 +22,12 @@ static PyObject * array_array_clear_impl(arrayobject *self); static PyObject * -array_array_clear(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_clear(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_clear_impl(self); + return_value = array_array_clear_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -46,12 +46,12 @@ static PyObject * array_array___copy___impl(arrayobject *self); static PyObject * -array_array___copy__(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array___copy__(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array___copy___impl(self); + return_value = array_array___copy___impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -75,7 +75,7 @@ array_array___deepcopy__(arrayobject *self, PyObject *unused) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array___deepcopy___impl(self, unused); + return_value = array_array___deepcopy___impl((arrayobject *)self, unused); Py_END_CRITICAL_SECTION(); return return_value; @@ -99,7 +99,7 @@ array_array_count(arrayobject *self, PyObject *v) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_count_impl(self, v); + return_value = array_array_count_impl((arrayobject *)self, v); Py_END_CRITICAL_SECTION(); return return_value; @@ -121,7 +121,7 @@ array_array_index_impl(arrayobject *self, PyObject *v, Py_ssize_t start, Py_ssize_t stop); static PyObject * -array_array_index(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) +array_array_index(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; PyObject *v; @@ -146,7 +146,7 @@ array_array_index(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) } skip_optional: Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_index_impl(self, v, start, stop); + return_value = array_array_index_impl((arrayobject *)self, v, start, stop); Py_END_CRITICAL_SECTION(); exit: @@ -171,7 +171,7 @@ array_array_remove(arrayobject *self, PyObject *v) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_remove_impl(self, v); + return_value = array_array_remove_impl((arrayobject *)self, v); Py_END_CRITICAL_SECTION(); return return_value; @@ -192,7 +192,7 @@ static PyObject * array_array_pop_impl(arrayobject *self, Py_ssize_t i); static PyObject * -array_array_pop(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) +array_array_pop(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; Py_ssize_t i = -1; @@ -217,7 +217,7 @@ array_array_pop(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) } skip_optional: Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_pop_impl(self, i); + return_value = array_array_pop_impl((arrayobject *)self, i); Py_END_CRITICAL_SECTION(); exit: @@ -237,7 +237,7 @@ static PyObject * array_array_extend_impl(arrayobject *self, PyTypeObject *cls, PyObject *bb); static PyObject * -array_array_extend(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +array_array_extend(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) @@ -262,7 +262,7 @@ array_array_extend(arrayobject *self, PyTypeObject *cls, PyObject *const *args, goto exit; } bb = args[0]; - return_value = array_array_extend_impl(self, cls, bb); + return_value = array_array_extend_impl((arrayobject *)self, cls, bb); exit: return return_value; @@ -281,7 +281,7 @@ static PyObject * array_array_insert_impl(arrayobject *self, Py_ssize_t i, PyObject *v); static PyObject * -array_array_insert(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) +array_array_insert(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; Py_ssize_t i; @@ -304,7 +304,7 @@ array_array_insert(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) } v = args[1]; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_insert_impl(self, i, v); + return_value = array_array_insert_impl((arrayobject *)self, i, v); Py_END_CRITICAL_SECTION(); exit: @@ -327,12 +327,12 @@ static PyObject * array_array_buffer_info_impl(arrayobject *self); static PyObject * -array_array_buffer_info(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_buffer_info(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_buffer_info_impl(self); + return_value = array_array_buffer_info_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -356,7 +356,7 @@ array_array_append(arrayobject *self, PyObject *v) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_append_impl(self, v); + return_value = array_array_append_impl((arrayobject *)self, v); Py_END_CRITICAL_SECTION(); return return_value; @@ -378,12 +378,12 @@ static PyObject * array_array_byteswap_impl(arrayobject *self); static PyObject * -array_array_byteswap(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_byteswap(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_byteswap_impl(self); + return_value = array_array_byteswap_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -402,12 +402,12 @@ static PyObject * array_array_reverse_impl(arrayobject *self); static PyObject * -array_array_reverse(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_reverse(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_reverse_impl(self); + return_value = array_array_reverse_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -427,7 +427,7 @@ array_array_fromfile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f, Py_ssize_t n); static PyObject * -array_array_fromfile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +array_array_fromfile(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) @@ -465,7 +465,7 @@ array_array_fromfile(arrayobject *self, PyTypeObject *cls, PyObject *const *args } n = ival; } - return_value = array_array_fromfile_impl(self, cls, f, n); + return_value = array_array_fromfile_impl((arrayobject *)self, cls, f, n); exit: return return_value; @@ -484,7 +484,7 @@ static PyObject * array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f); static PyObject * -array_array_tofile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +array_array_tofile(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) @@ -510,7 +510,7 @@ array_array_tofile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, } f = args[0]; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_tofile_impl(self, cls, f); + return_value = array_array_tofile_impl((arrayobject *)self, cls, f); Py_END_CRITICAL_SECTION(); exit: @@ -535,7 +535,7 @@ array_array_fromlist(arrayobject *self, PyObject *list) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION2(self, list); - return_value = array_array_fromlist_impl(self, list); + return_value = array_array_fromlist_impl((arrayobject *)self, list); Py_END_CRITICAL_SECTION2(); return return_value; @@ -554,12 +554,12 @@ static PyObject * array_array_tolist_impl(arrayobject *self); static PyObject * -array_array_tolist(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_tolist(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_tolist_impl(self); + return_value = array_array_tolist_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -578,7 +578,7 @@ static PyObject * array_array_frombytes_impl(arrayobject *self, Py_buffer *buffer); static PyObject * -array_array_frombytes(arrayobject *self, PyObject *arg) +array_array_frombytes(PyObject *self, PyObject *arg) { PyObject *return_value = NULL; Py_buffer buffer = {NULL, NULL}; @@ -587,7 +587,7 @@ array_array_frombytes(arrayobject *self, PyObject *arg) goto exit; } Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_frombytes_impl(self, &buffer); + return_value = array_array_frombytes_impl((arrayobject *)self, &buffer); Py_END_CRITICAL_SECTION(); exit: @@ -612,12 +612,12 @@ static PyObject * array_array_tobytes_impl(arrayobject *self); static PyObject * -array_array_tobytes(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_tobytes(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_tobytes_impl(self); + return_value = array_array_tobytes_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -640,7 +640,7 @@ static PyObject * array_array_fromunicode_impl(arrayobject *self, PyObject *ustr); static PyObject * -array_array_fromunicode(arrayobject *self, PyObject *arg) +array_array_fromunicode(PyObject *self, PyObject *arg) { PyObject *return_value = NULL; PyObject *ustr; @@ -651,7 +651,7 @@ array_array_fromunicode(arrayobject *self, PyObject *arg) } ustr = arg; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_fromunicode_impl(self, ustr); + return_value = array_array_fromunicode_impl((arrayobject *)self, ustr); Py_END_CRITICAL_SECTION(); exit: @@ -675,12 +675,12 @@ static PyObject * array_array_tounicode_impl(arrayobject *self); static PyObject * -array_array_tounicode(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array_tounicode(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array_tounicode_impl(self); + return_value = array_array_tounicode_impl((arrayobject *)self); Py_END_CRITICAL_SECTION(); return return_value; @@ -699,9 +699,9 @@ static PyObject * array_array___sizeof___impl(arrayobject *self); static PyObject * -array_array___sizeof__(arrayobject *self, PyObject *Py_UNUSED(ignored)) +array_array___sizeof__(PyObject *self, PyObject *Py_UNUSED(ignored)) { - return array_array___sizeof___impl(self); + return array_array___sizeof___impl((arrayobject *)self); } PyDoc_STRVAR(array__array_reconstructor__doc__, @@ -770,7 +770,7 @@ array_array___reduce_ex___impl(arrayobject *self, PyTypeObject *cls, PyObject *value); static PyObject * -array_array___reduce_ex__(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +array_array___reduce_ex__(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) @@ -796,7 +796,7 @@ array_array___reduce_ex__(arrayobject *self, PyTypeObject *cls, PyObject *const } value = args[0]; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_array___reduce_ex___impl(self, cls, value); + return_value = array_array___reduce_ex___impl((arrayobject *)self, cls, value); Py_END_CRITICAL_SECTION(); exit: @@ -816,7 +816,7 @@ static PyObject * array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls); static PyObject * -array_arrayiterator___reduce__(arrayiterobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +array_arrayiterator___reduce__(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; @@ -825,7 +825,7 @@ array_arrayiterator___reduce__(arrayiterobject *self, PyTypeObject *cls, PyObjec goto exit; } Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_arrayiterator___reduce___impl(self, cls); + return_value = array_arrayiterator___reduce___impl((arrayiterobject *)self, cls); Py_END_CRITICAL_SECTION(); exit: @@ -850,9 +850,9 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_arrayiterator___setstate___impl(self, state); + return_value = array_arrayiterator___setstate___impl((arrayiterobject *)self, state); Py_END_CRITICAL_SECTION(); return return_value; } -/*[clinic end generated code: output=2417e21ccf1eb8e6 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c57dd72b41be600b input=a9049054013a1b77]*/ From bb4bf90825e5f67581ca0d56a9706076dc87756a Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Mon, 20 Jan 2025 15:15:08 -0500 Subject: [PATCH 19/31] iterator safe from multi-crit deadlocks --- Modules/arraymodule.c | 96 +++++++++++++++++----------------- Modules/clinic/arraymodule.c.h | 28 ++-------- 2 files changed, 52 insertions(+), 72 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index e836a42aa984c2..321339f859b7bb 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3199,53 +3199,46 @@ array_iter(arrayobject *ao) return NULL; it->ao = (arrayobject*)Py_NewRef(ao); - it->index = 0; + it->index = 0; // -1 indicates exhausted it->getitem = ao->ob_descr->getitem; PyObject_GC_Track(it); return (PyObject *)it; } static PyObject * -arrayiter_next_lock_held(arrayiterobject *it) +arrayiter_next(arrayiterobject *it) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(it); - arrayobject *ao; - + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->index); + if (index < 0) { + return NULL; + } + PyObject *ret; #ifndef NDEBUG array_state *state = find_array_state_by_type(Py_TYPE(it)); assert(PyObject_TypeCheck(it, state->ArrayIterType)); + assert(array_Check(it->ao, state)); #endif - ao = it->ao; - if (ao == NULL) { - return NULL; + + Py_BEGIN_CRITICAL_SECTION(it->ao); + if (index < Py_SIZE(it->ao)) { + ret = (*it->getitem)(it->ao, index); } - PyObject *ret = NULL; - Py_BEGIN_CRITICAL_SECTION(ao); -#ifndef NDEBUG - assert(array_Check(ao, state)); -#endif - if (it->index < Py_SIZE(ao)) { - ret = (*it->getitem)(ao, it->index++); + else { + ret = NULL; } Py_END_CRITICAL_SECTION(); + if (ret != NULL) { - return ret; + FT_ATOMIC_STORE_SSIZE_RELAXED(it->index, index + 1); + } + else { + FT_ATOMIC_STORE_SSIZE_RELAXED(it->index, -1); +#ifndef Py_GIL_DISABLED + arrayobject *ao = it->ao; + it->ao = NULL; + Py_DECREF(ao); +#endif } - it->ao = NULL; - Py_DECREF(ao); - return NULL; -} - -static PyObject * -arrayiter_next(arrayiterobject *it) -{ - PyObject *ret; - assert(it != NULL); - - Py_BEGIN_CRITICAL_SECTION(it); - ret = arrayiter_next_lock_held(it); - Py_END_CRITICAL_SECTION(); - return ret; } @@ -3269,7 +3262,6 @@ arrayiter_traverse(arrayiterobject *it, visitproc visit, void *arg) } /*[clinic input] -@critical_section array.arrayiterator.__reduce__ cls: defining_class @@ -3280,19 +3272,27 @@ Return state information for pickling. static PyObject * array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) -/*[clinic end generated code: output=4b032417a2c8f5e6 input=61ad213fe49ae0f7]*/ +/*[clinic end generated code: output=4b032417a2c8f5e6 input=ac64e65a87ad452e]*/ { array_state *state = get_array_state_by_class(cls); assert(state != NULL); PyObject *func = _PyEval_GetBuiltin(state->str_iter); - if (self->ao == NULL) { - return Py_BuildValue("N(())", func); + PyObject *ret = NULL; + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->index); + if (index >= 0) { + Py_BEGIN_CRITICAL_SECTION(self->ao); + if (index <= Py_SIZE(self->ao)) { + ret = Py_BuildValue("N(O)n", func, self->ao, index); + } + Py_END_CRITICAL_SECTION(); + } + if (ret == NULL) { + ret = Py_BuildValue("N(())", func); } - return Py_BuildValue("N(O)n", func, self->ao, self->index); + return ret; } /*[clinic input] -@critical_section array.arrayiterator.__setstate__ state: object @@ -3302,24 +3302,26 @@ Set state information for unpickling. [clinic start generated code]*/ static PyObject * -array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state) -/*[clinic end generated code: output=d7837ae4ac1fd8b9 input=8d8dc7ce40b9c1f7]*/ +array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) +/*[clinic end generated code: output=397da9904e443cbe input=f47d5ceda19e787b]*/ { Py_ssize_t index = PyLong_AsSsize_t(state); if (index == -1 && PyErr_Occurred()) { return NULL; } - arrayobject *ao = self->ao; - if (ao != NULL) { - Py_BEGIN_CRITICAL_SECTION(ao); - if (index < 0) { - index = 0; + if (FT_ATOMIC_LOAD_SSIZE_RELAXED(self->index) >= 0) { + Py_BEGIN_CRITICAL_SECTION(self->ao); + if (index < -1) { + index = -1; } - else if (index > Py_SIZE(ao)) { - index = Py_SIZE(ao); /* iterator exhausted */ + else { + Py_ssize_t size = Py_SIZE(self->ao); + if (index > size) { + index = size; /* iterator at end */ + } } + FT_ATOMIC_STORE_SSIZE_RELAXED(self->index, index); Py_END_CRITICAL_SECTION(); - self->index = index; } Py_RETURN_NONE; } diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index e1d5277daeb98b..3816bb7709658e 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -818,18 +818,11 @@ array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls); static PyObject * array_arrayiterator___reduce__(PyObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - PyObject *return_value = NULL; - if (nargs || (kwnames && PyTuple_GET_SIZE(kwnames))) { PyErr_SetString(PyExc_TypeError, "__reduce__() takes no arguments"); - goto exit; + return NULL; } - Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_arrayiterator___reduce___impl((arrayiterobject *)self, cls); - Py_END_CRITICAL_SECTION(); - -exit: - return return_value; + return array_arrayiterator___reduce___impl((arrayiterobject *)self, cls); } PyDoc_STRVAR(array_arrayiterator___setstate____doc__, @@ -840,19 +833,4 @@ PyDoc_STRVAR(array_arrayiterator___setstate____doc__, #define ARRAY_ARRAYITERATOR___SETSTATE___METHODDEF \ {"__setstate__", (PyCFunction)array_arrayiterator___setstate__, METH_O, array_arrayiterator___setstate____doc__}, - -static PyObject * -array_arrayiterator___setstate___impl(arrayiterobject *self, PyObject *state); - -static PyObject * -array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - return_value = array_arrayiterator___setstate___impl((arrayiterobject *)self, state); - Py_END_CRITICAL_SECTION(); - - return return_value; -} -/*[clinic end generated code: output=c57dd72b41be600b input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c9219e074c62e0c8 input=a9049054013a1b77]*/ From 4d5bb3aba8b1d05f6bdcc20c49649545e00ef3eb Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Tue, 21 Jan 2025 13:31:37 -0500 Subject: [PATCH 20/31] remove unnecessary crit sect in setstate --- Modules/arraymodule.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 321339f859b7bb..56ec45d0515d34 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3208,6 +3208,7 @@ array_iter(arrayobject *ao) static PyObject * arrayiter_next(arrayiterobject *it) { + assert(it != NULL); Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->index); if (index < 0) { return NULL; @@ -3310,18 +3311,22 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) return NULL; } if (FT_ATOMIC_LOAD_SSIZE_RELAXED(self->index) >= 0) { - Py_BEGIN_CRITICAL_SECTION(self->ao); if (index < -1) { index = -1; } else { - Py_ssize_t size = Py_SIZE(self->ao); + Py_ssize_t size; +#ifdef Py_GIL_DISABLED + size = _Py_atomic_load_ssize_relaxed( + &(_PyVarObject_CAST(self->ao)->ob_size)); +#else + size = Py_SIZE(self->ao); +#endif if (index > size) { index = size; /* iterator at end */ } } FT_ATOMIC_STORE_SSIZE_RELAXED(self->index, index); - Py_END_CRITICAL_SECTION(); } Py_RETURN_NONE; } From e50e6537a66d58e00736513c83eab931af6a265b Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Wed, 22 Jan 2025 09:01:15 -0500 Subject: [PATCH 21/31] protect ob_exports atomically --- Modules/arraymodule.c | 45 ++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 56ec45d0515d34..5fd3a8429bfea6 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -69,6 +69,16 @@ typedef struct { PyObject *str_iter; } array_state; +static inline Py_ssize_t Pyarrayobject_GET_SIZE(PyObject *op) { + arrayobject *ao = (arrayobject *)op; +#ifdef Py_GIL_DISABLED + return _Py_atomic_load_ssize_relaxed(&(_PyVarObject_CAST(ao)->ob_size)); +#else + return Py_SIZE(ao); +#endif +} +#define Pyarrayobject_GET_SIZE(op) Pyarrayobject_GET_SIZE(_PyObject_CAST(op)) + /* Forward declaration. */ static PyObject *array_array_frombytes(PyObject *self, PyObject *bytes); @@ -137,7 +147,8 @@ array_resize(arrayobject *self, Py_ssize_t newsize) char *items; size_t _new_size; - if (self->ob_exports > 0 && newsize != Py_SIZE(self)) { + if (FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ob_exports) > 0 && + newsize != Py_SIZE(self)) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -728,7 +739,7 @@ array_dealloc(arrayobject *op) PyTypeObject *tp = Py_TYPE(op); PyObject_GC_UnTrack(op); - if (op->ob_exports > 0) { + if (FT_ATOMIC_LOAD_SSIZE_RELAXED(op->ob_exports) > 0) { PyErr_SetString(PyExc_SystemError, "deallocated array object has exported buffers"); PyErr_Print(); @@ -868,11 +879,7 @@ array_richcompare(PyObject *v, PyObject *w, int op) static Py_ssize_t array_length(arrayobject *a) { - Py_ssize_t ret; - Py_BEGIN_CRITICAL_SECTION(a); // overkill but lets not tempt tsan - ret = Py_SIZE(a); - Py_END_CRITICAL_SECTION(); - return ret; + return Pyarrayobject_GET_SIZE(a); } static PyObject * @@ -1074,7 +1081,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) /* Issue #4509: If the array has exported buffers and the slice assignment would change the size of the array, fail early to make sure we don't modify it. */ - if (d != 0 && a->ob_exports > 0) { + if (d != 0 && FT_ATOMIC_LOAD_SSIZE_RELAXED(a->ob_exports) > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -2707,7 +2714,8 @@ array_ass_subscr_lock_held(arrayobject* self, PyObject* item, PyObject* value) /* Issue #4509: If the array has exported buffers and the slice assignment would change the size of the array, fail early to make sure we don't modify it. */ - if ((needed == 0 || slicelength != needed) && self->ob_exports > 0) { + if ((needed == 0 || slicelength != needed) && + FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ob_exports) > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -2809,6 +2817,7 @@ static const void *emptybuf = ""; static int array_buffer_getbuf_lock_held(arrayobject *self, Py_buffer *view, int flags) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); if (view == NULL) { PyErr_SetString(PyExc_BufferError, "array_buffer_getbuf: view==NULL argument is obsolete"); @@ -2842,7 +2851,11 @@ array_buffer_getbuf_lock_held(arrayobject *self, Py_buffer *view, int flags) #endif } +#ifdef Py_GIL_DISABLED + _Py_atomic_add_ssize(&self->ob_exports, 1); +#else self->ob_exports++; +#endif return 0; } @@ -2859,10 +2872,12 @@ array_buffer_getbuf(arrayobject *self, Py_buffer *view, int flags) static void array_buffer_relbuf(arrayobject *self, Py_buffer *view) { - Py_BEGIN_CRITICAL_SECTION(self); +#ifdef Py_GIL_DISABLED + assert(_Py_atomic_add_ssize(&self->ob_exports, -1) >= 1); +#else self->ob_exports--; assert(self->ob_exports >= 0); - Py_END_CRITICAL_SECTION(); +#endif } static PyObject * @@ -3315,13 +3330,7 @@ array_arrayiterator___setstate__(arrayiterobject *self, PyObject *state) index = -1; } else { - Py_ssize_t size; -#ifdef Py_GIL_DISABLED - size = _Py_atomic_load_ssize_relaxed( - &(_PyVarObject_CAST(self->ao)->ob_size)); -#else - size = Py_SIZE(self->ao); -#endif + Py_ssize_t size = Pyarrayobject_GET_SIZE(self->ao); if (index > size) { index = size; /* iterator at end */ } From 841a870488ebd9f5c45cad94e3c1e69c97606bdb Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Thu, 23 Jan 2025 09:03:49 -0500 Subject: [PATCH 22/31] 2 missed misc atomic writes --- Modules/arraymodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 5fd3a8429bfea6..409f9aa051bf6f 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -170,7 +170,7 @@ array_resize(arrayobject *self, Py_ssize_t newsize) PyMem_Free(self->ob_item); self->ob_item = NULL; Py_SET_SIZE(self, 0); - self->allocated = 0; + FT_ATOMIC_STORE_SSIZE_RELAXED(self->allocated, 0); return 0; } @@ -200,7 +200,7 @@ array_resize(arrayobject *self, Py_ssize_t newsize) } self->ob_item = items; Py_SET_SIZE(self, newsize); - self->allocated = _new_size; + FT_ATOMIC_STORE_SSIZE_RELAXED(self->allocated, _new_size); return 0; } From d80c1a50db46fddd6f53451ee6c95b728059ad6c Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Wed, 12 Feb 2025 09:28:47 -0500 Subject: [PATCH 23/31] misc tweaks --- Modules/arraymodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 7150015bcceb81..c9f14d87a6f2f5 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -2571,8 +2571,8 @@ array_repr(PyObject *op) static PyObject* array_subscr_lock_held(PyObject *op, PyObject *item) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); arrayobject *self = arrayobject_CAST(op); - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); array_state *state = find_array_state_by_type(Py_TYPE(self)); if (PyIndex_Check(item)) { From dffc9cd5f88b393e849e3e3ea69c15870b4e2c25 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Wed, 12 Feb 2025 16:08:14 -0500 Subject: [PATCH 24/31] add test --- Lib/test/test_array.py | 270 +++++++++++++++++++++++++++++++++++++++++ Modules/arraymodule.c | 4 +- 2 files changed, 272 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py index 58ea89c4fac833..3350541a5e8a97 100755 --- a/Lib/test/test_array.py +++ b/Lib/test/test_array.py @@ -3,16 +3,21 @@ """ import collections.abc +import io import unittest from test import support from test.support import import_helper from test.support import os_helper +from test.support import threading_helper from test.support import _2G import weakref import pickle import operator +import random import struct import sys +import sysconfig +import threading import warnings import array @@ -1673,5 +1678,270 @@ def test_gh_128961(self): self.assertRaises(StopIteration, next, it) +class FreeThreadingTest(unittest.TestCase): + # Test pretty much everything that can break under free-threading. + # Non-deterministic, but at least one of these things will fail if + # array module is not free-thread safe. + + @unittest.skipUnless(sysconfig.get_config_var('Py_GIL_DISABLED'), + 'this test can only fail with GIL disabled') + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_free_threading(self): + def pop1(b, a): # MODIFIES! + b.wait() + try: a.pop() + except IndexError: pass + + def append1(b, a): # MODIFIES! + b.wait() + a.append(2) + + def insert1(b, a): # MODIFIES! + b.wait() + a.insert(0, 2) + + def extend(b, a): # MODIFIES! + c = array.array('i', [2]) + b.wait() + a.extend(c) + + def extend2(b, a, c): # MODIFIES! + b.wait() + a.extend(c) + + def inplace_concat(b, a): # MODIFIES! + c = array.array('i', [2]) + b.wait() + a += c + + def inplace_concat2(b, a, c): # MODIFIES! + b.wait() + a += c + + def inplace_repeat2(b, a): # MODIFIES! + b.wait() + a *= 2 + + def clear(b, a, *args): # MODIFIES! + b.wait() + a.clear() + + def clear2(b, a, c): # MODIFIES c! + b.wait() + try: c.clear() + except BufferError: pass + + def remove1(b, a): # MODIFIES! + b.wait() + try: a.remove(1) + except ValueError: pass + + def fromunicode(b, a): # MODIFIES! + b.wait() + a.fromunicode('test') + + def frombytes(b, a): # MODIFIES! + b.wait() + a.frombytes(b'0000') + + def frombytes2(b, a, c): # MODIFIES! + b.wait() + a.frombytes(c) + + def fromlist(b, a): # MODIFIES! + n = random.randint(0, 100) + b.wait() + a.fromlist([2] * n) + + def ass_subscr2(b, a, c): # MODIFIES! + b.wait() + a[:] = c + + def ass0(b, a): # modifies inplace + b.wait() + try: a[0] = 0 + except IndexError: pass + + def byteswap(b, a): # modifies inplace + b.wait() + a.byteswap() + + def tounicode(b, a): + b.wait() + a.tounicode() + + def tobytes(b, a): + b.wait() + a.tobytes() + + def tolist(b, a): + b.wait() + a.tolist() + + def tofile(b, a): + f = io.BytesIO() + b.wait() + a.tofile(f) + + def reduce_ex2(b, a): + b.wait() + a.__reduce_ex__(2) + + def reduce_ex3(b, a): + b.wait() + c = a.__reduce_ex__(3) + assert not c[1] or b'\xdd' not in c[1][3] + + def copy(b, a): + b.wait() + c = a.__copy__() + assert not c or 0xdd not in c + + def repr1(b, a): + b.wait() + repr(a) + + def repeat2(b, a): + b.wait() + a * 2 + + def count1(b, a): + b.wait() + a.count(1) + + def index1(b, a): + b.wait() + try: a.index(1) + except ValueError: pass + + def contains1(b, a): + b.wait() + try: 1 in a + except ValueError: pass + + def subscr0(b, a): + b.wait() + try: a[0] + except IndexError: pass + + def concat(b, a): + b.wait() + a + a + + def concat2(b, a, c): + b.wait() + a + c + + def richcmplhs(b, a): + c = a[:] + b.wait() + a == c + + def richcmprhs(b, a): + c = a[:] + b.wait() + c == a + + def new(b, a): + tc = a.typecode + b.wait() + array.array(tc, a) + + def repr_(b, a): + b.wait() + repr(a) + + def irepeat(b, a): # MODIFIES! + b.wait() + a *= 2 + + def newi(b, l): + b.wait() + array.array('i', l) + + def fromlistl(b, a, l): # MODIFIES! + b.wait() + a.fromlist(l) + + def fromlistlclear(b, a, l): # MODIFIES LIST! + b.wait() + l.clear() + + def iter_next(b, a, it): # MODIFIES ITERATOR! + b.wait() + list(it) + + def iter_reduce(b, a, it): + b.wait() + c = it.__reduce__() + assert not c[1] or b'\xdd' not in c[1][0] + + def check(funcs, a=None, *args): + if a is None: + a = array.array('i', [1]) + + barrier = threading.Barrier(len(funcs)) + threads = [] + + for func in funcs: + thread = threading.Thread(target=func, args=(barrier, a, *args)) + + threads.append(thread) + + with threading_helper.start_threads(threads): + pass + + for thread in threads: + threading_helper.join_thread(thread) + + check([pop1] * 10) + check([pop1] + [subscr0] * 10) + check([append1] * 10) + check([insert1] * 10) + check([pop1] + [index1] * 10) + check([pop1] + [contains1] * 10) + check([insert1] + [repeat2] * 10) + check([pop1] + [repr1] * 10) + check([inplace_repeat2] * 10) + check([byteswap] * 10) + check([insert1] + [clear] * 10) + check([pop1] + [count1] * 10) + check([remove1] * 10) + check([clear] + [copy] * 10, array.array('B', b'0' * 0x400000)) + check([pop1] + [reduce_ex2] * 10) + check([clear] + [reduce_ex3] * 10, array.array('B', b'0' * 0x400000)) + check([pop1] + [tobytes] * 10) + check([pop1] + [tolist] * 10) + check([clear, tounicode] * 10, array.array('w', 'a'*10000)) + check([clear, tofile] * 10, array.array('w', 'a'*10000)) + check([clear] + [extend] * 10) + check([clear] + [inplace_concat] * 10) + check([clear] + [concat] * 10, array.array('w', 'a'*10000)) + check([fromunicode] * 10, array.array('w', 'a')) + check([frombytes] * 10) + check([fromlist] * 10) + check([clear] + [richcmplhs] * 10, array.array('i', [1]*10000)) + check([clear] + [richcmprhs] * 10, array.array('i', [1]*10000)) + check([clear, ass0] * 10, array.array('i', [1]*10000)) # to test array_ass_item must disable Py_mp_ass_subscript + check([clear] + [new] * 10, array.array('w', 'a'*10000)) + check([clear] + [repr_] * 10, array.array('B', b'0' * 0x40000)) + check([clear] + [repr_] * 10, array.array('B', b'0' * 0x40000)) + check([clear] + [irepeat] * 10, array.array('B', b'0' * 0x40000)) + check([clear] + [iter_reduce] * 10, a := array.array('B', b'0' * 0x400), iter(a)) + + # make sure we handle non-self objects correctly + check([clear] + [newi] * 10, [2] * random.randint(0, 100)) + check([fromlistlclear] + [fromlistl] * 10, array.array('i', [1]), [2] * random.randint(0, 100)) + check([clear2] + [concat2] * 10, array.array('w', 'a'*10000), array.array('w', 'a'*10000)) + check([clear2] + [inplace_concat2] * 10, array.array('w', 'a'*10000), array.array('w', 'a'*10000)) + check([clear2] + [extend2] * 10, array.array('w', 'a'*10000), array.array('w', 'a'*10000)) + check([clear2] + [ass_subscr2] * 10, array.array('w', 'a'*10000), array.array('w', 'a'*10000)) + check([clear2] + [frombytes2] * 10, array.array('w', 'a'*10000), array.array('B', b'a'*10000)) + + # iterator stuff + check([clear] + [iter_next] * 10, a := array.array('i', [1] * 10), iter(a)) + + if __name__ == "__main__": unittest.main() diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index c9f14d87a6f2f5..64565e931665b9 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3248,13 +3248,13 @@ arrayiter_next(PyObject *op) return NULL; } PyObject *ret; + arrayobject *ao = it->ao; #ifndef NDEBUG array_state *state = find_array_state_by_type(Py_TYPE(it)); assert(PyObject_TypeCheck(it, state->ArrayIterType)); - assert(array_Check(it->ao, state)); + assert(array_Check(ao, state)); #endif - arrayobject *ao = it->ao; Py_BEGIN_CRITICAL_SECTION(ao); if (index < Py_SIZE(ao)) { ret = (*it->getitem)(ao, index); From b3665fdddd042e5138ca7a3fff171733b4db6659 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Wed, 12 Feb 2025 16:35:19 -0500 Subject: [PATCH 25/31] misc fix, b'\xdd' -> 0xdd --- Lib/test/test_array.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py index 3350541a5e8a97..9954afadb53836 100755 --- a/Lib/test/test_array.py +++ b/Lib/test/test_array.py @@ -1791,7 +1791,7 @@ def reduce_ex2(b, a): def reduce_ex3(b, a): b.wait() c = a.__reduce_ex__(3) - assert not c[1] or b'\xdd' not in c[1][3] + assert not c[1] or 0xdd not in c[1][3] def copy(b, a): b.wait() @@ -1875,7 +1875,7 @@ def iter_next(b, a, it): # MODIFIES ITERATOR! def iter_reduce(b, a, it): b.wait() c = it.__reduce__() - assert not c[1] or b'\xdd' not in c[1][0] + assert not c[1] or 0xdd not in c[1][0] def check(funcs, a=None, *args): if a is None: From 3814e53f4749d55920d6dcb0779e3231ed167359 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 14 Feb 2025 18:37:56 -0500 Subject: [PATCH 26/31] use support.Py_GIL_DISABLED --- Lib/test/test_array.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py index 9954afadb53836..faf05ea560912c 100755 --- a/Lib/test/test_array.py +++ b/Lib/test/test_array.py @@ -1683,8 +1683,7 @@ class FreeThreadingTest(unittest.TestCase): # Non-deterministic, but at least one of these things will fail if # array module is not free-thread safe. - @unittest.skipUnless(sysconfig.get_config_var('Py_GIL_DISABLED'), - 'this test can only fail with GIL disabled') + @unittest.skipUnless(support.Py_GIL_DISABLED, 'this test can only possibly fail with GIL disabled') @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_free_threading(self): From 4a5c5682a3939644323dd0c429d9cf5ac8bcd118 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 15 Feb 2025 08:29:07 -0500 Subject: [PATCH 27/31] add critical section held assertions --- Lib/test/test_array.py | 3 --- Modules/arraymodule.c | 33 ++++++++++++++------------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_array.py b/Lib/test/test_array.py index faf05ea560912c..bc3eeef8000190 100755 --- a/Lib/test/test_array.py +++ b/Lib/test/test_array.py @@ -1891,9 +1891,6 @@ def check(funcs, a=None, *args): with threading_helper.start_threads(threads): pass - for thread in threads: - threading_helper.join_thread(thread) - check([pop1] * 10) check([pop1] + [subscr0] * 10) check([append1] * 10) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 64565e931665b9..1021ec0b6a5c21 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -147,6 +147,7 @@ enum machine_format_code { static int array_resize(arrayobject *self, Py_ssize_t newsize) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); char *items; size_t _new_size; @@ -687,6 +688,7 @@ newarrayobject(PyTypeObject *type, Py_ssize_t size, const struct arraydescr *des static PyObject * getarrayitem(PyObject *op, Py_ssize_t i) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); #ifndef NDEBUG array_state *state = find_array_state_by_type(Py_TYPE(op)); assert(array_Check(op, state)); @@ -700,6 +702,7 @@ getarrayitem(PyObject *op, Py_ssize_t i) static int ins1(arrayobject *self, Py_ssize_t where, PyObject *v) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); char *items; Py_ssize_t n = Py_SIZE(self); if (v == NULL) { @@ -911,6 +914,7 @@ array_item(PyObject *op, Py_ssize_t i) static PyObject * array_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); array_state *state = find_array_state_by_type(Py_TYPE(a)); arrayobject *np; @@ -1071,6 +1075,7 @@ array_repeat(PyObject *op, Py_ssize_t n) static int array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); char *item; Py_ssize_t d; /* Change in size */ if (ilow < 0) @@ -1104,7 +1109,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) } static int -array_ass_item_lock_held(PyObject *op, Py_ssize_t i, PyObject *v) +setarrayitem(PyObject *op, Py_ssize_t i, PyObject *v) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); arrayobject *a = arrayobject_CAST(op); @@ -1123,24 +1128,16 @@ array_ass_item(PyObject *op, Py_ssize_t i, PyObject *v) { int ret; Py_BEGIN_CRITICAL_SECTION(op); - ret = array_ass_item_lock_held(op, i, v); + ret = setarrayitem(op, i, v); Py_END_CRITICAL_SECTION(); return ret; } -static int -setarrayitem(PyObject *a, Py_ssize_t i, PyObject *v) -{ -#ifndef NDEBUG - array_state *state = find_array_state_by_type(Py_TYPE(a)); - assert(array_Check(a, state)); -#endif - return array_ass_item(a, i, v); -} - static int array_iter_extend(arrayobject *self, PyObject *bb) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(bb); PyObject *it, *v; it = PyObject_GetIter(bb); @@ -2647,12 +2644,12 @@ static int array_ass_subscr_lock_held(PyObject *op, PyObject* item, PyObject* value) { _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); + #ifdef Py_DEBUG + if (value != NULL) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); + } + #endif arrayobject *self = arrayobject_CAST(op); -#ifdef Py_DEBUG - if (value != NULL) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); - } -#endif Py_ssize_t start, stop, step, slicelength, needed; array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; @@ -3316,11 +3313,9 @@ array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) PyObject *ret = NULL; Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->index); if (index >= 0) { - Py_BEGIN_CRITICAL_SECTION(self->ao); if (index <= Py_SIZE(self->ao)) { ret = Py_BuildValue("N(O)n", func, self->ao, index); } - Py_END_CRITICAL_SECTION(); } if (ret == NULL) { ret = Py_BuildValue("N(())", func); From cc8d71505b1ff07a306adf800dd6d9c8dfd20e60 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 15 Feb 2025 08:37:28 -0500 Subject: [PATCH 28/31] Py_CLEAR(it->ao) --- Modules/arraymodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 1021ec0b6a5c21..8c5d52bbbcbba2 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3267,8 +3267,7 @@ arrayiter_next(PyObject *op) else { FT_ATOMIC_STORE_SSIZE_RELAXED(it->index, -1); #ifndef Py_GIL_DISABLED - it->ao = NULL; - Py_DECREF(ao); + Py_CLEAR(it->ao); #endif } return ret; From 5f352a3997b0bbedcada105b6fb05185d3515963 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sat, 15 Feb 2025 08:52:51 -0500 Subject: [PATCH 29/31] make ob_exports non-atomic everywhere --- Modules/arraymodule.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 8c5d52bbbcbba2..d7359a6d61b74a 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -151,8 +151,7 @@ array_resize(arrayobject *self, Py_ssize_t newsize) char *items; size_t _new_size; - if (FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ob_exports) > 0 && - newsize != Py_SIZE(self)) { + if (self->ob_exports > 0 && newsize != Py_SIZE(self)) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -746,7 +745,7 @@ array_dealloc(PyObject *op) PyObject_GC_UnTrack(op); arrayobject *self = arrayobject_CAST(op); - if (FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ob_exports) > 0) { + if (self->ob_exports > 0) { PyErr_SetString(PyExc_SystemError, "deallocated array object has exported buffers"); PyErr_Print(); @@ -1093,7 +1092,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) /* Issue #4509: If the array has exported buffers and the slice assignment would change the size of the array, fail early to make sure we don't modify it. */ - if (d != 0 && FT_ATOMIC_LOAD_SSIZE_RELAXED(a->ob_exports) > 0) { + if (d != 0 && a->ob_exports > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -2726,8 +2725,7 @@ array_ass_subscr_lock_held(PyObject *op, PyObject* item, PyObject* value) /* Issue #4509: If the array has exported buffers and the slice assignment would change the size of the array, fail early to make sure we don't modify it. */ - if ((needed == 0 || slicelength != needed) && - FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ob_exports) > 0) { + if ((needed == 0 || slicelength != needed) && self->ob_exports > 0) { PyErr_SetString(PyExc_BufferError, "cannot resize an array that is exporting buffers"); return -1; @@ -2864,11 +2862,7 @@ array_buffer_getbuf_lock_held(PyObject *op, Py_buffer *view, int flags) #endif } -#ifdef Py_GIL_DISABLED - _Py_atomic_add_ssize(&self->ob_exports, 1); -#else self->ob_exports++; -#endif return 0; } @@ -2885,13 +2879,11 @@ array_buffer_getbuf(PyObject *op, Py_buffer *view, int flags) static void array_buffer_relbuf(PyObject *op, Py_buffer *Py_UNUSED(view)) { + Py_BEGIN_CRITICAL_SECTION(op); arrayobject *self = arrayobject_CAST(op); -#ifdef Py_GIL_DISABLED - assert(_Py_atomic_add_ssize(&self->ob_exports, -1) >= 1); -#else self->ob_exports--; assert(self->ob_exports >= 0); -#endif + Py_END_CRITICAL_SECTION(); } static PyObject * From 98f14334c83f1c295c8986878d5e94b857e102c6 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Sun, 16 Feb 2025 09:54:52 -0500 Subject: [PATCH 30/31] check type before lock in array_ass_subscr() --- Modules/arraymodule.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index d7359a6d61b74a..e72001127b7a26 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -2642,15 +2642,15 @@ array_subscr(PyObject *op, PyObject *item) static int array_ass_subscr_lock_held(PyObject *op, PyObject* item, PyObject* value) { + array_state* state = find_array_state_by_type(Py_TYPE(op)); _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); - #ifdef Py_DEBUG - if (value != NULL) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); - } - #endif +#ifdef Py_DEBUG + if (value != NULL && array_Check(value, state)) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(value); + } +#endif arrayobject *self = arrayobject_CAST(op); Py_ssize_t start, stop, step, slicelength, needed; - array_state* state = find_array_state_by_type(Py_TYPE(self)); arrayobject* other; int itemsize; @@ -2808,15 +2808,16 @@ static int array_ass_subscr(PyObject *op, PyObject* item, PyObject* value) { int ret; - if (value == NULL) { - Py_BEGIN_CRITICAL_SECTION(op); + array_state* state = find_array_state_by_type(Py_TYPE(op)); + if (value != NULL && array_Check(value, state)) { + Py_BEGIN_CRITICAL_SECTION2(op, value); ret = array_ass_subscr_lock_held(op, item, value); - Py_END_CRITICAL_SECTION(); + Py_END_CRITICAL_SECTION2(); } else { - Py_BEGIN_CRITICAL_SECTION2(op, value); + Py_BEGIN_CRITICAL_SECTION(op); ret = array_ass_subscr_lock_held(op, item, value); - Py_END_CRITICAL_SECTION2(); + Py_END_CRITICAL_SECTION(); } return ret; } From 0bcde5c6a697bdbdc4af4b755fae35915060dbef Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Thu, 27 Feb 2025 13:36:49 +0000 Subject: [PATCH 31/31] simplify array_arrayiterator___reduce___impl --- Modules/arraymodule.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index e72001127b7a26..0775b26e1d68ed 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -3302,17 +3302,11 @@ array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) array_state *state = get_array_state_by_class(cls); assert(state != NULL); PyObject *func = _PyEval_GetBuiltin(state->str_iter); - PyObject *ret = NULL; Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->index); if (index >= 0) { - if (index <= Py_SIZE(self->ao)) { - ret = Py_BuildValue("N(O)n", func, self->ao, index); - } - } - if (ret == NULL) { - ret = Py_BuildValue("N(())", func); + return Py_BuildValue("N(O)n", func, self->ao, index); } - return ret; + return Py_BuildValue("N(())", func); } /*[clinic input]