Skip to content

Fix use-after-free race condition in C extension for free-threaded CPython (3.13t+)#1317

Open
rodrigobnogueira wants to merge 6 commits intoaio-libs:masterfrom
rodrigobnogueira:fix-free-threaded-race-condition
Open

Fix use-after-free race condition in C extension for free-threaded CPython (3.13t+)#1317
rodrigobnogueira wants to merge 6 commits intoaio-libs:masterfrom
rodrigobnogueira:fix-free-threaded-race-condition

Conversation

@rodrigobnogueira
Copy link
Copy Markdown
Member

Description

This PR fixes a critical memory-safety race condition (Use-After-Free) that results in a process segmentation fault or abort (stack smashing) when iterating over and modifying a MultiDict concurrently in CPython free-threaded mode (3.13t+).

Context

When running without the GIL, iterating over md.keys()/md.items() or view objects could safely yield memory pointers pointing to MultiDictObject->keys. However, if the dictionary resized concurrently using _md_resize(), the keys table was reallocated and immediately free()d, leaving concurrent iterators reading stale heap pointers. This explicitly leaked and corrupted process memory resulting in an ASAN/SEGV error on the READ inside the iterator buffers.

Proposed Approach

To safely resolve this while adhering to Python 3.13 free-threaded lock mechanisms, we matched CPython's own dict concurrent locking pattern. This involves adding Py_BEGIN_CRITICAL_SECTION(self) / Py_END_CRITICAL_SECTION() at every public-facing entry point in the C extension. The pythoncapi_compat.h shim already natively skips these macros on pre-3.13 monolithic GIL designs, making this 100% zero-overhead everywhere except 3.13t.

  • Wrapped all major method slots in _multidict.c (__getitem__, __setitem__, __delitem__, add, getall, pop, copy, etc.),
  • Wrapped all iterator block functions dynamically unlocking inside iter.h,
  • Fixed finder operations correctly propagating to cs_done for safety unloads in views.h.

Verification

  1. Concurrency Testing (test_free_threading.py): Added a new threading test module that heavily threads CIMultiDict adding and deleting inputs while natively reading iterations concurrently.
  2. Safety Assertions: Validated that upon hash resizing, the active pointers dynamically trip into finder->version != finder->md->version successfully, naturally abandoning the iteration and surfacing standard RuntimeError('MultiDict is changed during iteration') without throwing a C segmentation fault! This matches native single-threaded Py-behavior correctly.
  3. No Native Regressions: Formatted files by clang-format and confirmed 100% test coverage matches the baseline branch over python3.13t.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Apr 12, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 12, 2026

Merging this PR will not alter performance

✅ 245 untouched benchmarks


Comparing rodrigobnogueira:fix-free-threaded-race-condition (8121e70) with master (ecc83e1)

Open in CodSpeed

@rodrigobnogueira rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from ff45477 to 0b04769 Compare April 12, 2026 05:29
@rodrigobnogueira rodrigobnogueira marked this pull request as draft April 12, 2026 05:33
@rodrigobnogueira rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from 0af9e6a to e1cbca0 Compare April 12, 2026 15:10
@rodrigobnogueira

This comment was marked as outdated.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.85%. Comparing base (ecc83e1) to head (8121e70).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1317   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files          26       27    +1     
  Lines        3513     3543   +30     
  Branches      253      259    +6     
=======================================
+ Hits         3508     3538   +30     
  Misses          3        3           
  Partials        2        2           
Flag Coverage Δ
CI-GHA 99.85% <100.00%> (+<0.01%) ⬆️
pytest 99.85% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rodrigobnogueira rodrigobnogueira changed the title Fix memory safety race condition in CPython 3.13 free-threaded mode Fix use-after-free race condition in C extension for free-threaded CPython (3.13t+) Apr 12, 2026
Add per-object locking via Py_BEGIN_CRITICAL_SECTION at all public-facing
entry points in the C extension to prevent use-after-free crashes when
iterating and mutating a MultiDict concurrently.

The root cause is _md_resize() reallocating md->keys without any
synchronization, causing concurrent iterators to access freed memory.
This fix wraps all public methods in Py_BEGIN_CRITICAL_SECTION(self),
matching CPython's own dict locking model. On GIL builds the macros
are no-ops (via pythoncapi_compat.h).

Adds tests/test_free_threading.py with a concurrent stress test that
previously crashed with SIGSEGV and now completes cleanly.
On non-free-threaded builds, Py_END_CRITICAL_SECTION() expands to just '}'.
When preceded by a goto label like 'cs_done:', this creates a C23 extension
that clang -Werror rejects. Adding an empty statement (;) after the label
makes it valid C11.
In debug builds, Py_BEGIN_CRITICAL_SECTION can suspend during Python
calls (e.g. __eq__ in md_calc_identity), allowing another thread to
enter. The md_replace finder pattern uses hash=-1 markers internally,
and ASSERT_CONSISTENT inside md_finder_cleanup catches this temporary
inconsistency, calling abort(). Remove del operations from the writer
loop since simple set+read is sufficient to exercise the core resize
race fix.
@rodrigobnogueira rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from 62dce51 to 8121e70 Compare April 13, 2026 03:27
@rodrigobnogueira rodrigobnogueira marked this pull request as ready for review April 13, 2026 03:49
Copy link
Copy Markdown
Member

@Vizonex Vizonex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good surprised this wasn't seen with the sniffer from #1306. Be sure you update your branches if you haven't since #1314 Got merged which I wrote to help with broken coverage but other than that I have no suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants