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

[rodrigobnogueira]

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.

[codspeed-hq]

Merging this PR will not alter performance

✅ 242 untouched benchmarks

---

_{Comparing rodrigobnogueira:fix-free-threaded-race-condition (b3b4434) with master (2aed5c2)}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (2aed5c2) to head (b3b4434).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1317   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files          28       29    +1     
  Lines        3627     3666   +39     
  Branches      265      271    +6     
=======================================
+ Hits         3622     3661   +39     
  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 Harness.
📢 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]

@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]

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]],

[Vizonex]

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.

[rodrigobnogueira]

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]

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]

See #1327

[Dreamsorcerer]

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]

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]

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]

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]

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).

[Vizonex]

Are we still planning to move forward with these free-threaded PRs at all or will these be closed?

[rodrigobnogueira]

pre-commit surfaced the new PLC0415 rule, fixed in 4079a3e.

[rodrigobnogueira]

Are we still planning to move forward with these free-threaded PRs at all or will these be closed?

IMO this solves the issue, and the main cost is how many functions it has to touch: every entry point that reads md->keys needs the critical section, the same way CPython's dict does it. The sections are no-ops on regular builds, so only free-threaded builds pay.

I think it deserves fixing: this is memory unsafety, not a logical race. A pure-Python user on 3.13t can segfault the process, where CPython's dict raises RuntimeError. The lighter alternative is documenting it as unsupported and keeping only the RuntimeWarning, but that leaves a crash reachable from Python code.

[devdanzin]

This is the right approach — mirroring CPython dict's per-object critical sections at every entry point, no-op on the default build. Two follow-ups:

1. The cross-object paths lock only self. multidict_tp_richcompare correctly uses Py_BEGIN_CRITICAL_SECTION2(self, other_md), but multidict_extend / update / merge-from-another-multidict are wrapped Py_BEGIN_CRITICAL_SECTION(self) only — and md_update_from_ht / md_clone_from_ht read the source other->keys. A thread mutating other while another does a.update(other) still races the read. These want CRITICAL_SECTION2(self, other) too. (multidict_copy is fine — there self is the source.)

2. A few of the crashes are reachable single-threaded, so they survive the critical sections. A critical section is a no-op on the default (GIL) build and doesn't guard same-thread reentrancy, so these need the function bodies fixed regardless of locking — they're borrowed-pointer-across-callback bugs, not pure data races:

• md_update_from_ht caches htkeys_entries(other->keys) and holds borrowed entry->key/value across a resize / key.lower(). Reproduces on a plain debug and ASan build, no threads:

  from multidict import MultiDict
  d = MultiDict([(f"k{i}", i) for i in range(64)])
  d.extend(d)   # self-alias: growing d resizes/frees d->keys mid-copy -> SIGSEGV

• md_update_from_seq caches the list size once, then a CI key's .lower() can shrink the list → PyList_GET_ITEM past the end (OOB). Also reproduces single-threaded.

• md_eq holds lft_entries/rht_entries across PyObject_RichCompareBool; a value's __eq__ that mutates a dict dangles the cached base (reentrant even under the GIL).

The fix for these is in the bodies: re-fetch the entries base after any call that can mutate/resize (the md_next/md_find_next re-fetch pattern already used elsewhere), snapshot the size, and for the self-alias case detect other == self. I have runnable reproducers for each and am glad to open them as separate issues (they overlap the C-extension report #1306). None of this blocks the concurrency fix here — just flagging that the critical sections alone won't close these particular crashes.