From b4f4d57f52b1208b73260f8b416919cd47ddf19a Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Wed, 8 Apr 2026 18:39:11 +0200 Subject: [PATCH 1/3] bug: prevent race condition in list.remove --- Objects/listobject.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Objects/listobject.c b/Objects/listobject.c index 4a98c8e54ab03f..12347963639aad 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3390,6 +3390,11 @@ list_remove_impl(PyListObject *self, PyObject *value) int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); Py_DECREF(obj); if (cmp > 0) { + if (i >= Py_SIZE(self) || self->ob_item[i] != obj) { + PyErr_SetString(PyExc_RuntimeError, + "list mutated during remove"); + return NULL; + } if (list_ass_slice_lock_held(self, i, i+1, NULL) == 0) Py_RETURN_NONE; return NULL; From b96fa2251a3b9f9579c0826e00a4088fd695ac4c Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Mon, 13 Apr 2026 16:50:37 -0400 Subject: [PATCH 2/3] test: fix test_xml_etree --- Lib/test/test_xml_etree.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py index 93162f52ba0344..12f7bab20ab0e7 100644 --- a/Lib/test/test_xml_etree.py +++ b/Lib/test/test_xml_etree.py @@ -2816,15 +2816,15 @@ def test_remove_with_clear_assume_missing(self): def test_remove_with_clear_assume_existing(self): # gh-126033: Check that a concurrent clear() for an assumed-to-be - # existing element does not make the interpreter crash. + # existing element raises RuntimeError but does not crash. self.do_test_remove_with_clear(raises=False) def do_test_remove_with_clear(self, *, raises): - - # Until the discrepency between "del root[:]" and "root.clear()" is - # resolved, we need to keep two tests. Previously, using "del root[:]" - # did not crash with the reproducer of gh-126033 while "root.clear()" - # did. + # 'del root[:]' mutates the children list in-place, while + # 'root.clear()' replaces self._children with a new list. When + # raises=False (element "found"), the in-place mutation is detected + # by list.remove and raises RuntimeError, whereas root.clear() is + # invisible to list.remove (the old list is unchanged). class E(ET.Element): """Local class to be able to mock E.__eq__ for introspection.""" @@ -2839,16 +2839,21 @@ def __eq__(self, o): root.clear() return not raises - if raises: - get_checker_context = lambda: self.assertRaises(ValueError) - else: - get_checker_context = nullcontext - self.assertIs(E.__eq__, object.__eq__) + get_checker_context = nullcontext for Z, side_effect in [(X, 'del root[:]'), (Y, 'root.clear()')]: self.enterContext(self.subTest(side_effect=side_effect)) + # X uses 'del root[:]' which mutates the list in-place; this is + # detected by list.remove when raises=False (element "found"). + # Y uses 'root.clear()' which swaps out self._children; the old + # list that list.remove is iterating is untouched, so no error. + if raises: + get_checker_context = lambda: self.assertRaises(ValueError) + elif Z is X: + get_checker_context = lambda: self.assertRaises(RuntimeError) + # test removing R() from [U()] for R, U, description in [ (E, Z, "remove missing E() from [Z()]"), @@ -2913,7 +2918,7 @@ def test_remove_with_mutate_root_assume_missing(self): def test_remove_with_mutate_root_assume_existing(self): # gh-126033: Check that a concurrent mutation for an assumed-to-be - # existing element does not make the interpreter crash. + # existing element raises RuntimeError. self.do_test_remove_with_mutate_root(raises=False) def do_test_remove_with_mutate_root(self, *, raises): @@ -2927,7 +2932,7 @@ def __eq__(self, o): if raises: get_checker_context = lambda: self.assertRaises(ValueError) else: - get_checker_context = nullcontext + get_checker_context = lambda: self.assertRaises(RuntimeError) # test removing R() from [U(), V()] cases = self.cases_for_remove_missing_with_mutations(E, Z) From 858e505462fc578a49ac61d1a205033ed77962fe Mon Sep 17 00:00:00 2001 From: Thomas Kowalski Date: Mon, 13 Apr 2026 16:53:17 -0400 Subject: [PATCH 3/3] news: add entry --- .../2026-04-13-20-51-34.gh-issue-148259.76UyK8x8.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-04-13-20-51-34.gh-issue-148259.76UyK8x8.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-04-13-20-51-34.gh-issue-148259.76UyK8x8.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-13-20-51-34.gh-issue-148259.76UyK8x8.rst new file mode 100644 index 00000000000000..3da424b2523c72 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-13-20-51-34.gh-issue-148259.76UyK8x8.rst @@ -0,0 +1,4 @@ +:meth:`list.remove` now raises :exc:`RuntimeError` if the list is mutated +during the ``__eq__`` rich comparison and a matching element was found. This +prevents silent data corruption where the wrong element could be removed from +a list under concurrent mutation.