diff --git a/Misc/NEWS.d/next/Library/2026-05-17-22-42-54.gh-issue-149965.OFW3ZD.rst b/Misc/NEWS.d/next/Library/2026-05-17-22-42-54.gh-issue-149965.OFW3ZD.rst new file mode 100644 index 00000000000000..99d9a06988bb31 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-05-17-22-42-54.gh-issue-149965.OFW3ZD.rst @@ -0,0 +1 @@ +Improve thread-safety of ElementObjects in ``element_ass_subscr`` diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 9e794be5c109ba..0e60e131acc991 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -1863,6 +1863,35 @@ element_subscr(PyObject *op, PyObject *item) } } +// Pointer-by-pointer memmove for PyObject** arrays that is safe +// for shared ElementObjects in Py_GIL_DISABLED builds. +static void +ptr_wise_atomic_memmove(ElementObject *a, PyObject **dest, PyObject **src, Py_ssize_t n) +{ +#ifndef Py_GIL_DISABLED + memmove(dest, src, n * sizeof(PyObject *)); +#else + // XXX: maybe a critical section isn't needed for ElementObject? + // _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a); + if (_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) { + // No other threads can read this list concurrently + memmove(dest, src, n * sizeof(PyObject *)); + return; + } + if (dest < src) { + for (Py_ssize_t i = 0; i != n; i++) { + _Py_atomic_store_ptr_release(&dest[i], src[i]); + } + } + else { + // copy backwards to avoid overwriting src before it's read + for (Py_ssize_t i = n; i != 0; i--) { + _Py_atomic_store_ptr_release(&dest[i - 1], src[i - 1]); + } + } +#endif +} + static int element_ass_subscr(PyObject *op, PyObject *item, PyObject *value) { @@ -1938,19 +1967,21 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value) PyList_SET_ITEM(recycle, i, self->extra->children[cur]); - memmove( + ptr_wise_atomic_memmove( + self, self->extra->children + cur - i, self->extra->children + cur + 1, - num_moved * sizeof(PyObject *)); + num_moved); } /* Leftover "tail" after the last removed child */ cur = start + (size_t)slicelen * step; if (cur < (size_t)self->extra->length) { - memmove( + ptr_wise_atomic_memmove( + self, self->extra->children + cur - slicelen, self->extra->children + cur, - (self->extra->length - cur) * sizeof(PyObject *)); + self->extra->length - cur); } self->extra->length -= slicelen;