Merging this PR will not alter performance

✅ 266 untouched benchmarks

_{Comparing asvetlov:claude/multidict-ci-failure-605017 (aa98110) with master (cd528d6)}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (cd528d6) to head (aa98110).

@@            Coverage Diff            @@
##            master     #1496   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         6021      6069   +48     
  Branches       538       538           
=========================================
+ Hits          6021      6069   +48     

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

Confidence Score: 5/5

Safe to merge.

_{Reviews (2) · Last reviewed commit: "Give cloned multidicts a new version"}

@aiolibsbot rr

PR Review — Stop items() iteration from reading a freed entry in CIMultiDict

Strong root-cause work with two residual holes in the same guard; not merge-ready yet.

What's solid here is real: moving the *pvalue = Py_NewRef(entry->value) read ahead of _md_ensure_key() in md_next(), md_prev(), md_find_next() and md_readonly_find_next() is exactly right, and the // last entry access markers plus the "caller must not touch entry after this returns" comment make the new contract legible. Holding own references to old_key and identity across _md_calc_key() correctly fixes the IStr_New()-freed-entry->identity crash, and deferring the last Py_DECREF(old_key) past the entry write is the right ordering for the __del__ case. Giving clones a fresh version in md_clone_from_ht() is a better answer to the reported reproducer than tracking the table pointer, since a freed htkeys_t can be reallocated at the same address; it also aligns the C backend with _multidict_py.py:881, which already bumped on __init__. The three new tests each pin a distinct trigger, and the CHANGES fragment follows the repo's conventions.

• hashtable.h:869 assumes entry->key == old_key after the conversion. Caching does not bump the version, so a re-entrant _md_ensure_key() on the same entry passes the version check, leaks the inner istr's entry reference, and double-decrefs old_key (refcount underflow on a plain release build, no threads needed). Py_SETREF(entry->key, Py_NewRef(key)) closes it.
• The version guard at hashtable.h:868 is not airtight on GIL builds: _md_resize() bumps the version only under #ifdef Py_GIL_DISABLED, and md_reserve() can free and move the entries array from a size hint with no bump to follow. Hoisting the bump out of the #ifdef makes the guard sound on all builds.
• Nit: tests/test_version.py:324-328 compares versions with >, which docs/multidict.rst:414-417 explicitly says is not allowed; != proves the same regression.

🟡 Important

class Key(str):
    depth = 0
    def __str__(self):
        Key.depth += 1
        if Key.depth == 1:
            list(d.items())      # inner conversion caches istr I1 into the entry
        return str.__str__(self)

d = CIMultiDict()
d[Key("a")] = "v"
list(d.items())                  # outer conversion resumes with a stale assumption

if (key != NULL && md->version == version) {
    /* A nested conversion of this same entry may already have cached
       an istr here, and caching does not bump the version. */
    Py_SETREF(entry->key, Py_NewRef(key));
}
Py_DECREF(identity);
Py_DECREF(old_key);

    if (key != NULL && md->version == version) {
        entry->key = Py_NewRef(key);
        Py_DECREF(old_key);
    }

🟢 Suggestions

assert multidict_getversion_callable(m.copy()) != v
...
assert multidict_getversion_callable(m2) != v

    assert multidict_getversion_callable(m.copy()) > v
    m2 = any_multidict_class()
    m2.__init__(m)  # type: ignore[misc]
    assert multidict_getversion_callable(m2) > v

Checklist

• Key-conversion path free of use-after-free / refcount errors — warning ＃1
• Dual-backend parity (C extension and pure Python)
• Error paths release all held references (identity, key, value)
• New behaviour exercised by tests — warning ＃1
• Documented public contract respected — suggestion ＃2
• CHANGES fragment present, correct category, correct attribution
• PR description matches the diff (no scope creep)

To rebase and address feedback, mention me: @aiolibsbot rebase critical (fixes 🔴 only), @aiolibsbot rebase important (fixes 🔴 + 🟡), or @aiolibsbot rebase --fix for all. (A bare @aiolibsbot rebase only rebases onto the base branch.)

Automated review by Kōan (Claude) HEAD=aa98110 10 min 4s