Fix use-after-free race condition in C extension for free-threaded CPython (3.13t+) by rodrigobnogueira · Pull Request #1317 · aio-libs/multidict

rodrigobnogueira · 2026-04-12T05:17:20Z

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

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.
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.
No Native Regressions: Formatted files by clang-format and confirmed 100% test coverage matches the baseline branch over python3.13t.

codspeed-hq · 2026-04-12T05:22:11Z

Merging this PR will not alter performance

✅ 245 untouched benchmarks

_{Comparing rodrigobnogueira:fix-free-threaded-race-condition (31d372c) with master (4122516)}

codecov · 2026-04-12T16:47:33Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (4122516) to head (31d372c).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1317   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files          28       29    +1     
  Lines        3602     3640   +38     
  Branches      265      271    +6     
=======================================
+ Hits         3597     3635   +38     
  Misses          3        3           
  Partials        2        2

Flag	Coverage Δ
CI-GHA	`99.86% <100.00%> (+<0.01%)`	⬆️
pytest	`99.86% <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.

Vizonex

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.

Vizonex · 2026-04-28T20:04:46Z

@rodrigobnogueira it's been 2 weeks so I'll see if we can get someone else in here to merge since I don't have that authorization to do so yet. for now I recommend staying updated with the main branch.

webknjaz · 2026-04-29T12:55:01Z

@@ -0,0 +1,2 @@
+Fixed a memory-safety race condition resulting in segmentation faults (Use-After-Free) when iterating and modifying a ``MultiDict`` concurrently in CPython free-threaded mode (3.13t+). Read/Write accesses to the internal ``keys`` buffer are now wrapped in ``Py_BEGIN_CRITICAL_SECTION``


Can we use RST roles to reference the highlighted identifiers?

webknjaz · 2026-04-29T12:57:46Z

+@pytest.mark.parametrize("cls", [CIMultiDict, MultiDict])
+def test_race_condition_iterator_vs_mutation(
+    cls: Union[Type[CIMultiDict[str]], Type[MultiDict[str]]],


This should probably be just

Suggested change

@pytest.mark.parametrize("cls", [CIMultiDict, MultiDict])

def test_race_condition_iterator_vs_mutation(

cls: Union[Type[CIMultiDict[str]], Type[MultiDict[str]]],

def test_race_condition_iterator_vs_mutation(

any_multidict_class: type[CIMultiDict[str] | type[MultiDict[str]],

I'm pretty sure it wasn't updated with some of the newer branches. I'm sure if @rodrigobnogueira were to update the branch pre-commit would catch it and try to autofix it.

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.

…ests

- Use `any_multidict_class` fixture instead of manual `@pytest.mark.parametrize` - Use modern type annotations (`type[]` instead of `Type[]`, `|` instead of `Union`) - Add explanatory comments to empty except clauses (CodeQL feedback) - Use RST roles in changelog (`:class:`, `:c:func:`) for highlighted identifiers - Remove manual pure-python skip (fixture handles both variants) - Use `MutableMultiMapping` for internal type hints

rodrigobnogueira · 2026-05-02T03:24:01Z

While fixing the C extension race condition, our test suite (now running the any_multidict_class fixture against both implementations) uncovered a related but separate thread-safety bug in _multidict_py.py on free-threaded CPython (3.13t).

Root cause: __setitem__ temporarily sets e.hash = -1 as a sentinel marker while traversing entries, relying on restore_hash() to clean up afterwards. Under free-threaded CPython, a concurrent thread can enter _add_with_hash → _resize → build_indices(update=False) in that window, where it hits:

assert hash_ != -1  # line 543

...and raises AssertionError because it observed the in-progress -1 sentinel from the other thread.

The fix for the C extension in this PR uses Py_BEGIN_CRITICAL_SECTION, but the pure-Python implementation has no equivalent synchronization. Fixing it properly would require adding a threading.Lock to MultiDict.

I've skipped the test for the pure-Python variant for now to unblock this PR.

rodrigobnogueira · 2026-05-02T03:24:20Z

Error showed up in CI (traceback from Thread B):

  File "_multidict_py.py", line 881, in __setitem__
    self._add_with_hash(_Entry(hash_, identity, key, value))
       ↓ (usable <= 0, triggers resize)
  File "_multidict_py.py", line 1088, in _add_with_hash
    self._resize((self._used * 3 | _HtKeys.MINSIZE - 1).bit_length(), False)
       ↓
  File "_multidict_py.py", line 1083, in _resize
    newkeys.build_indices(update)
       ↓
  File "_multidict_py.py", line 543, in build_indices
    assert hash_ != -1
AssertionError

Two concurrent __setitem__ calls race:

Thread A is updating an existing key — it sets e.hash = -1 (line 874) as a sentinel while iterating, and has not yet called restore_hash().
Thread B is inserting a new key — it enters the not found branch (line 880–881), calls _add_with_hash → _resize → build_indices(update=False), which iterates the same shared entries and hits Thread A's -1 sentinel → AssertionError.

Vizonex · 2026-05-02T03:57:47Z

See #1327

Dreamsorcerer · 2026-05-02T13:40:34Z

I'm unclear how much we should be concerned with such issues. The entire point of aio-libs is to provide asyncio libraries, so threading issues shouldn't be relevant. I'd rather not start introducing threading locks into our libraries..

Vizonex · 2026-05-02T14:59:58Z

I'm unclear how much we should be concerned with such issues. The entire point of aio-libs is to provide asyncio libraries, so threading issues shouldn't be relevant. I'd rather not start introducing threading locks into our libraries..

@Dreamsorcerer You have to remember to that multidict is used in other libraries or projects besides our own. An example that I have is litestar however I'm with you on questioning weather or not thread locks should be considered acceptable or unacceptable however.

rodrigobnogueira · 2026-05-03T03:16:58Z

I'm unclear how much we should be concerned with such issues. The entire point of aio-libs is to provide asyncio libraries, so threading issues shouldn't be relevant. I'd rather not start introducing threading locks into our libraries..

Adding threading.Lock to the pure-Python fallback would add overhead for all users to solve a problem that is probably an edge case. Would it be acceptable to emit a RuntimeWarning at import time only when both conditions are true: running on free-threaded CPython (GIL disabled) AND the C extension is not available?

Dreamsorcerer · 2026-05-03T16:15:18Z

Adding threading.Lock to the pure-Python fallback would add overhead for all users to solve a problem that is probably an edge case. Would it be acceptable to emit a RuntimeWarning at import time only when both conditions are true: running on free-threaded CPython (GIL disabled) AND the C extension is not available?

We're talking about just iterating and modifying the same multidict, right? Can just mention that this shouldn't be done in a threaded situation. Maybe issue a warning.

rodrigobnogueira · 2026-05-04T20:37:22Z

Adding threading.Lock to the pure-Python fallback would add overhead for all users to solve a problem that is probably an edge case. Would it be acceptable to emit a RuntimeWarning at import time only when both conditions are true: running on free-threaded CPython (GIL disabled) AND the C extension is not available?

We're talking about just iterating and modifying the same multidict, right? Can just mention that this shouldn't be done in a threaded situation. Maybe issue a warning.

Added a RuntimeWarning in init.py that fires only when the pure-Python fallback is used on free-threaded CPython (GIL disabled + C extension unavailable).

for more information, see https://pre-commit.ci

rodrigobnogueira requested review from asvetlov and webknjaz as code owners April 12, 2026 05:17

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 12, 2026

github-advanced-security AI found potential problems Apr 12, 2026

View reviewed changes

Comment thread tests/test_free_threading.py Fixed

Comment thread tests/test_free_threading.py Fixed

rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from ff45477 to 0b04769 Compare April 12, 2026 05:29

rodrigobnogueira marked this pull request as draft April 12, 2026 05:33

rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from 0af9e6a to e1cbca0 Compare April 12, 2026 15:10

This comment was marked as outdated.

Sign in to view

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

rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from 62dce51 to 8121e70 Compare April 13, 2026 03:27

rodrigobnogueira marked this pull request as ready for review April 13, 2026 03:49

Vizonex reviewed Apr 13, 2026

View reviewed changes

webknjaz reviewed Apr 29, 2026

View reviewed changes

Vizonex added the free-threading FT-related discussion, issue or PR label May 1, 2026

rodrigobnogueira added 6 commits May 1, 2026 20:46

Add author attribution to changelog entry

3cdfed9

Add coverage pragmas to test exception handlers

a1add47

test: add pragma no cover to RuntimeError handler in free threading t…

ab0b478

…ests

github-advanced-security AI found potential problems May 2, 2026

View reviewed changes

Comment thread tests/test_free_threading.py Fixed

rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch 2 times, most recently from b9ecbaf to 1d70349 Compare May 2, 2026 02:22

rodrigobnogueira force-pushed the fix-free-threaded-race-condition branch from 1d70349 to 469dda9 Compare May 2, 2026 02:36

rodrigobnogueira requested a review from webknjaz May 2, 2026 03:28

Warn when pure-Python fallback is used on free-threaded CPython

9d4e097

rodrigobnogueira and others added 2 commits May 4, 2026 18:20

Add test for pure-Python free-threaded warning

e4cab00

[pre-commit.ci] auto fixes from pre-commit.com hooks

31d372c

for more information, see https://pre-commit.ci

		@@ -0,0 +1,2 @@
		Fixed a memory-safety race condition resulting in segmentation faults (Use-After-Free) when iterating and modifying a ``MultiDict`` concurrently in CPython free-threaded mode (3.13t+). Read/Write accesses to the internal ``keys`` buffer are now wrapped in ``Py_BEGIN_CRITICAL_SECTION``

Uh oh!

Conversation

rodrigobnogueira commented Apr 12, 2026

Description

Context

Proposed Approach

Verification

Uh oh!

Uh oh!

Uh oh!

codspeed-hq Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

This comment was marked as outdated.

codecov Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Vizonex left a comment

Choose a reason for hiding this comment

Uh oh!

Vizonex commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webknjaz Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

webknjaz Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Vizonex Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rodrigobnogueira commented May 2, 2026

Uh oh!

rodrigobnogueira commented May 2, 2026

Uh oh!

Vizonex commented May 2, 2026

Uh oh!

Dreamsorcerer commented May 2, 2026

Uh oh!

Vizonex commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rodrigobnogueira commented May 3, 2026

Uh oh!

Dreamsorcerer commented May 3, 2026

Uh oh!

rodrigobnogueira commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codspeed-hq Bot commented Apr 12, 2026 •

edited

Loading

codecov Bot commented Apr 12, 2026 •

edited

Loading

Vizonex commented Apr 28, 2026 •

edited

Loading

Vizonex commented May 2, 2026 •

edited

Loading