gh-149965: rename ptr_wise_atomic_memmove to _PyObject_ptr_wise_atomic_memmove

Include/internal/pycore_object.h

@@ -8,6 +8,7 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
+#include "pycore_critical_section.h" // _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED
 #include "pycore_wasm_trampoline.h" // _PyCFunction_TrampolineCall()
 #include "pycore_gc.h"            // _PyObject_GC_TRACK()
 #include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_ACQUIRE()
@@ -1017,6 +1018,38 @@ static inline Py_ALWAYS_INLINE void _Py_INCREF_MORTAL(PyObject *op)
  * references. */
 PyAPI_FUNC(int) _PyObject_VisitType(PyObject *op, visitproc visit, void *arg);
 
+// Pointer-by-pointer memmove for PyObject** arrays that is safe for shared
+// objects in Py_GIL_DISABLED builds.  Locking is the caller's responsibility.
+static inline void
+_PyObject_ptr_wise_atomic_memmove(PyObject *a, PyObject **dest, PyObject **src,
+                             Py_ssize_t n)
+{
+#ifndef Py_GIL_DISABLED
+    (void)a;
+    memmove(dest, src, n * sizeof(PyObject *));
+#else
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(a);
+    if (_Py_IsOwnedByCurrentThread(a) && !_PyObject_GC_IS_SHARED(a)) {
+        // No other threads can read this object's array 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
+}
+
+#define _PyObject_ptr_wise_atomic_memmove(a, dest, src, n) _PyObject_ptr_wise_atomic_memmove(_PyObject_CAST(a), dest, src, n)
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_xml_etree_c.py

@@ -2,6 +2,7 @@
 import io
 import struct
 from test import support
+from test.support import threading_helper
 from test.support.import_helper import import_fresh_module
 import types
 import unittest
@@ -193,6 +194,48 @@ def test_disallow_instantiation(self):
         support.check_disallow_instantiation(self, iter_type)
 
 
+@unittest.skipUnless(cET, 'requires _elementtree')
+@threading_helper.requires_working_threading()
+class ThreadingTests(unittest.TestCase):
+    def test_concurrent_slice_deletion(self):
+        self.check_concurrent_slice_deletion(slice(1, 65))
+
+    def test_concurrent_extended_slice_deletion(self):
+        for index in (slice(1, 65, 3), slice(64, 0, -3), slice(1, None, 3)):
+            with self.subTest(index=index):
+                self.check_concurrent_slice_deletion(index)
+
+    def check_concurrent_slice_deletion(self, index):
+        # gh-149965: Read children while slice deletion shifts the child
+        # pointers. Create the elements here so the writer does not own them.
+        roots = []
+        expected = []
+        for _ in range(100):
+            root = cET.Element('root')
+            children = [cET.SubElement(root, str(i)) for i in range(100)]
+            del children[index]
+            roots.append(root)
+            expected.append(children)
+
+        def delete():
+            for root in roots:
+                del root[index]
+
+        def read():
+            for _ in range(10):
+                for root in roots:
+                    # Each slice leaves at least two children. Their identity
+                    # can change during deletion, but they must remain valid.
+                    child = root[1]
+                    self.assertIsInstance(child, cET.Element)
+                    self.assertIn(child.tag, tags)
+
+        tags = {str(i) for i in range(100)}
+        threading_helper.run_concurrently([delete, read, read])
+        for root, children in zip(roots, expected):
+            self.assertEqual(root[:], children)
+
+
 @unittest.skipUnless(cET, 'requires _elementtree')
 class TestAliasWorking(unittest.TestCase):
     # Test that the cET alias module is alive

Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-19-38-28.gh-issue-149965.w5eiS4.rst

@@ -0,0 +1 @@
+Fix thread-safety issues in ``xml.etree``

Modules/_elementtree.c

@@ -18,6 +18,7 @@
 #include "Python.h"
 #include "pycore_ceval.h"         // _Py_EnterRecursiveCall()
 #include "pycore_dict.h"          // _PyDict_CopyAsDict()
+#include "pycore_object.h"        // _PyObject_ptr_wise_atomic_memmove()
 #include "pycore_pyhash.h"        // _Py_HashSecret
 #include "pycore_tuple.h"         // _PyTuple_FromPair
 #include "pycore_weakref.h"       // FT_CLEAR_WEAKREFS()
@@ -1947,7 +1948,7 @@ element_subscr(PyObject *op, PyObject *item)
 }
 
 static int
-element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
+element_ass_subscr_lock_held(PyObject *op, PyObject *item, PyObject *value)
 {
     ElementObject *self = _Element_CAST(op);
 
@@ -2021,19 +2022,21 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
 
                 PyList_SET_ITEM(recycle, i, self->extra->children[cur]);
 
-                memmove(
+                _PyObject_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(
+                _PyObject_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;
@@ -2135,6 +2138,16 @@ element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
     }
 }
 
+static int
+element_ass_subscr(PyObject *op, PyObject *item, PyObject *value)
+{
+    int res = 0;
+    Py_BEGIN_CRITICAL_SECTION(op);
+    res = element_ass_subscr_lock_held(op, item, value);
+    Py_END_CRITICAL_SECTION();
+    return res;
+}
+
 static PyObject*
 element_tag_getter(PyObject *op, void *closure)
 {

Objects/listobject.c

@@ -10,7 +10,7 @@
 #include "pycore_list.h"          // struct _Py_list_freelist, _PyListIterObject
 #include "pycore_long.h"          // _PyLong_DigitCount
 #include "pycore_modsupport.h"    // _PyArg_NoKwnames()
-#include "pycore_object.h"        // _PyObject_GC_TRACK(), _PyDebugAllocatorStats()
+#include "pycore_object.h"        // _PyObject_GC_TRACK(), _PyObject_ptr_wise_atomic_memmove()
 #include "pycore_pyatomic_ft_wrappers.h"
 #include "pycore_setobject.h"     // _PySet_NextEntry()
 #include "pycore_stackref.h"      // _Py_TryIncrefCompareStackRef()
@@ -499,15 +499,12 @@ end:;
     return ret;
 }
 
-static void ptr_wise_atomic_memmove(PyListObject *a, PyObject **dest,
-                                    PyObject **src, Py_ssize_t n);
-
 static inline void
 list_shift_items_right_lock_held(PyListObject *self, Py_ssize_t first,
                                  Py_ssize_t last)
 {
 #ifdef Py_GIL_DISABLED
-    ptr_wise_atomic_memmove(self, &self->ob_item[first + 1],
+    _PyObject_ptr_wise_atomic_memmove(self, &self->ob_item[first + 1],
                             &self->ob_item[first], last - first);
 #else
     PyObject **items = self->ob_item;
@@ -522,7 +519,7 @@ list_shift_items_left_lock_held(PyListObject *self, Py_ssize_t first,
                                 Py_ssize_t last)
 {
 #ifdef Py_GIL_DISABLED
-    ptr_wise_atomic_memmove(self, &self->ob_item[first],
+    _PyObject_ptr_wise_atomic_memmove(self, &self->ob_item[first],
                             &self->ob_item[first + 1], last - first);
 #else
     PyObject **items = self->ob_item;
@@ -967,33 +964,6 @@ list_clear_slot(PyObject *self)
     return 0;
 }
 
-// Pointer-by-pointer memmove for PyObject** arrays that is safe
-// for shared lists in Py_GIL_DISABLED builds.
-static void
-ptr_wise_atomic_memmove(PyListObject *a, PyObject **dest, PyObject **src, Py_ssize_t n)
-{
-#ifndef Py_GIL_DISABLED
-    memmove(dest, src, n * sizeof(PyObject *));
-#else
-    _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
-}
 
 /* a[ilow:ihigh] = v if v != NULL.
  * del a[ilow:ihigh] if v == NULL.
@@ -1066,7 +1036,7 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO
 
     if (d < 0) { /* Delete -d items */
         Py_ssize_t tail = Py_SIZE(a) - ihigh;
-        ptr_wise_atomic_memmove(a, &item[ihigh+d], &item[ihigh], tail);
+        _PyObject_ptr_wise_atomic_memmove(a, &item[ihigh+d], &item[ihigh], tail);
         (void)list_resize(a, Py_SIZE(a) + d); // NB: shrinking a list can't fail
         item = a->ob_item;
     }
@@ -1075,7 +1045,7 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO
         if (list_resize(a, k+d) < 0)
             goto Error;
         item = a->ob_item;
-        ptr_wise_atomic_memmove(a, &item[ihigh+d], &item[ihigh], k - ihigh);
+        _PyObject_ptr_wise_atomic_memmove(a, &item[ihigh+d], &item[ihigh], k - ihigh);
     }
     for (k = 0; k < n; k++, ilow++) {
         PyObject *w = vitem[k];
@@ -1166,7 +1136,7 @@ list_inplace_repeat_lock_held(PyListObject *self, Py_ssize_t n)
     Py_ssize_t copied = input_size;
     while (copied < output_size) {
         Py_ssize_t items_to_copy = Py_MIN(copied, output_size - copied);
-        ptr_wise_atomic_memmove(self, items + copied, items, items_to_copy);
+        _PyObject_ptr_wise_atomic_memmove(self, items + copied, items, items_to_copy);
         copied += items_to_copy;
     }
 #endif
@@ -1664,8 +1634,8 @@ list_pop_impl(PyListObject *self, Py_ssize_t index)
     }
     Py_ssize_t size_after_pop = Py_SIZE(self) - 1;
     if (index < size_after_pop) {
-        ptr_wise_atomic_memmove(self, &items[index], &items[index+1],
-                                size_after_pop - index);
+        _PyObject_ptr_wise_atomic_memmove(self, &items[index], &items[index+1],
+                                    size_after_pop - index);
     }
     list_resize(self, size_after_pop);  // NB: shrinking a list can't fail
     return v;
@@ -3851,12 +3821,12 @@ list_ass_subscript_lock_held(PyObject *_self, PyObject *item, PyObject *value)
                     lim = Py_SIZE(self) - cur - 1;
                 }
 
-                ptr_wise_atomic_memmove(self, self->ob_item + cur - i,
+                _PyObject_ptr_wise_atomic_memmove(self, self->ob_item + cur - i,
                     self->ob_item + cur + 1, lim);
             }
             cur = start + (size_t)slicelength * step;
             if (cur < (size_t)Py_SIZE(self)) {
-                ptr_wise_atomic_memmove(self, self->ob_item + cur - slicelength,
+                _PyObject_ptr_wise_atomic_memmove(self, self->ob_item + cur - slicelength,
                     self->ob_item + cur, Py_SIZE(self) - cur);
             }