From 68eabae22c480b9810e4ce25225dcfcf6a370a8b Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 05:51:36 +0000 Subject: [PATCH] gh-161062: Fix crash on concurrent first read of Element.text or tail On the free-threaded build, join the tree builder's pending text chunks in the element's critical section so that two readers cannot both join and release the same list. --- .../test_free_threading/test_xml_etree.py | 48 ++++++++++++++ ...-10-22-05-50-21.gh-issue-161062.q7TjNe.rst | 4 ++ Modules/_elementtree.c | 65 +++++++++++-------- 3 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 Lib/test/test_free_threading/test_xml_etree.py create mode 100644 Misc/NEWS.d/next/Library/2026-10-22-05-50-21.gh-issue-161062.q7TjNe.rst diff --git a/Lib/test/test_free_threading/test_xml_etree.py b/Lib/test/test_free_threading/test_xml_etree.py new file mode 100644 index 00000000000000..baa82aba586fc9 --- /dev/null +++ b/Lib/test/test_free_threading/test_xml_etree.py @@ -0,0 +1,48 @@ +import unittest + +from test.support import import_helper, threading_helper +from test.support.threading_helper import run_concurrently + +# The C accelerator keeps text delivered by expat in several chunks as a +# list and joins it on first access. +ET = import_helper.import_fresh_module('xml.etree.ElementTree', + fresh=['_elementtree']) + +NTHREADS = 8 +NITEMS = 500 + +# Each item text arrives as three chunks ('name ', '&', ' N') and each +# tail as two ('\n', ' '). +DOC = ("\n" + + "".join(f" name & {i}\n" for i in range(NITEMS)) + + " \n") + + +@threading_helper.requires_working_threading() +@unittest.skipIf(ET is None, 'requires _elementtree') +class TestElementConcurrentReads(unittest.TestCase): + def check(self, worker, nrounds=20): + for _ in range(nrounds): + root = ET.fromstring(DOC) + run_concurrently(worker_func=worker, nthreads=NTHREADS, + args=(root,)) + + def test_concurrent_text_tail(self): + def worker(root): + for i, item in enumerate(root): + self.assertEqual(item.text, f"name & {i}") + self.assertEqual(item.tail, "\n ") + + self.check(worker) + + def test_concurrent_tostring(self): + expected = ET.tostring(ET.fromstring(DOC)) + + def worker(root): + self.assertEqual(ET.tostring(root), expected) + + self.check(worker) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-10-22-05-50-21.gh-issue-161062.q7TjNe.rst b/Misc/NEWS.d/next/Library/2026-10-22-05-50-21.gh-issue-161062.q7TjNe.rst new file mode 100644 index 00000000000000..e206c5d9ea1388 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-22-05-50-21.gh-issue-161062.q7TjNe.rst @@ -0,0 +1,4 @@ +Fix a crash in the :term:`free-threaded build` when multiple threads +concurrently read the :attr:`~xml.etree.ElementTree.Element.text` or +:attr:`~xml.etree.ElementTree.Element.tail` attribute of a freshly parsed +:class:`xml.etree.ElementTree.Element` for the first time. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index f8f661a7341891..b752d0d410d646 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -571,45 +571,56 @@ element_get_attrib(ElementObject* self) } LOCAL(PyObject*) -element_get_text(ElementObject* self) +element_get_joined(ElementObject* self, PyObject **slot) { - /* return new reference to text attribute */ + /* return new reference to the text or tail attribute stored in *slot, + joining a list of chunks left there by the tree builder on first + access. */ - PyObject *res = self->text; + PyObject *res; - if (JOIN_GET(res)) { - res = JOIN_OBJ(res); - if (PyList_CheckExact(res)) { - PyObject *tmp = list_join(res); - if (!tmp) - return NULL; - self->text = tmp; - Py_SETREF(res, tmp); + Py_BEGIN_CRITICAL_SECTION(self); + res = *slot; + if (JOIN_GET(res) && PyList_CheckExact(JOIN_OBJ(res))) { + /* The critical section may be suspended while PyUnicode_Join() + locks the list, letting another thread join and release it + first: hold a reference and re-check *slot before storing. */ + PyObject *list = Py_NewRef(JOIN_OBJ(res)); + PyObject *joined = list_join(list); + if (joined == NULL) { + res = NULL; + } + else if (*slot == res) { + _set_joined_ptr(slot, joined); + res = joined; } + else { + Py_DECREF(joined); + res = JOIN_OBJ(*slot); + } + Py_DECREF(list); + } + else { + res = JOIN_OBJ(res); } + Py_XINCREF(res); + Py_END_CRITICAL_SECTION(); - return Py_NewRef(res); + return res; +} + +LOCAL(PyObject*) +element_get_text(ElementObject* self) +{ + /* return new reference to text attribute */ + return element_get_joined(self, &self->text); } LOCAL(PyObject*) element_get_tail(ElementObject* self) { /* return new reference to tail attribute */ - - PyObject *res = self->tail; - - if (JOIN_GET(res)) { - res = JOIN_OBJ(res); - if (PyList_CheckExact(res)) { - PyObject *tmp = list_join(res); - if (!tmp) - return NULL; - self->tail = tmp; - Py_SETREF(res, tmp); - } - } - - return Py_NewRef(res); + return element_get_joined(self, &self->tail); } /*[clinic input]