Skip to content

gh-146022: Fix free-threading crash in xml.etree.ElementTree's Element - #157000

Closed
NAVEENKUMARKR777 wants to merge 1 commit into
python:mainfrom
NAVEENKUMARKR777:gh-146022-elementtree-critical-sections
Closed

gh-146022: Fix free-threading crash in xml.etree.ElementTree's Element#157000
NAVEENKUMARKR777 wants to merge 1 commit into
python:mainfrom
NAVEENKUMARKR777:gh-146022-elementtree-critical-sections

Conversation

@NAVEENKUMARKR777

Copy link
Copy Markdown

Summary

Fixes gh-146022. Modules/_elementtree.c declares Py_MOD_GIL_NOT_USED but has no locking at all, so concurrent readers and writers race on Element's internal ElementObjectExtra struct (self->extra). For example, Element.clear() sets self->extra to NULL and frees the old struct while another thread concurrently indexes or measures the length of the same element, causing a use-after-free/NULL-deref segfault — exactly as reported, with a reliable repro script.

Changes

Add the standard critical-section locking used throughout the rest of the free-threaded build:

  • Every raw C slot touching self->extra: element_getitem (__getitem__), element_setitem (__setitem__), element_length (__len__), element_bool (__bool__), and the two subscript slots (element_subscr/element_ass_subscr, via the established *_lock_held extraction pattern already used in listobject.c for functions with multiple return paths).
  • All 8 tag/text/tail/attrib property getters/setters.
  • @critical_section Argument Clinic annotations on the methods that read or write self->extra: append, clear, extend, insert, remove, set, get, items, keys, find/findtext/findall, __copy__/__deepcopy__/__sizeof__/__getstate__/__setstate__.

Along the way, this surfaced a real MSVC portability issue: a goto-to-label pattern right before Py_END_CRITICAL_SECTION() is invalid pre-C23 when the critical-section macros expand to plain braces (as they do in GIL-enabled builds) — element_setitem was restructured to avoid it, in line with how the rest of the codebase avoids bare labels immediately preceding the macro's closing brace.

This is intentionally scoped to Element's core state (self->extra, tag, text, tail). The lazy tree-walking iterator (iter/itertext, backing ElementIter) and TreeBuilder/XMLParser are a separate, larger subsystem and are out of scope here.

Test plan

  • The issue's own crash reproducer segfaults reliably (3/3) against the pre-fix code and completes cleanly (3/3) with the fix, on a --disable-gil Windows build.
  • A broader multithreaded stress test exercising every touched method concurrently (mutation, reading, copy/deepcopy/pickle) from multiple threads shows no crashes or errors.
  • test_xml_etree and test_xml_etree_c (507 tests) pass on both a regular and a free-threaded build.

🤖 Generated with Claude Code

…Element

_elementtree.c declares Py_MOD_GIL_NOT_USED but has no locking at all,
so concurrent readers and writers race on the Element type's internal
ElementObjectExtra struct (self->extra). For example, Element.clear()
sets self->extra to NULL and frees the old struct while another thread
concurrently indexes or measures the length of the same element,
causing a use-after-free/NULL-deref segfault.

Add the standard Py_BEGIN/END_CRITICAL_SECTION locking to every raw C
slot (__getitem__, __setitem__, __len__, __bool__, and the two
subscript slots, via the established *_lock_held extraction pattern
for functions with multiple return paths) and every tag/text/tail/attrib
property accessor, and add @critical_section Argument Clinic
annotations to the methods that read or write self->extra (append,
clear, extend, insert, remove, set, get, items, keys, find/findtext/
findall, __copy__/__deepcopy__/__sizeof__/__getstate__/__setstate__).

Verified against the reported crash reproducer (reliably segfaults
without this change, does not with it), a broader multithreaded stress
test exercising every touched method concurrently, and the full
test_xml_etree/test_xml_etree_c suites on both a regular and a
free-threaded (--disable-gil) Windows build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@picnixz

picnixz commented Sep 5, 2026

Copy link
Copy Markdown
Member

I clearly said on the issue:

FTR: I don't want an LLM doing those changes. I want a core developer doing them or someone that has contributed enough to CPython because the implementation is non-trivial.

If you don't respect this, we will restrict your access to our repositories next time.

@picnixz picnixz closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make xml.etree.ElementTree.Element usable on free-threaded builds

2 participants