aio-libs / multidict / 36133675941

87%

push

github

Hold the values across a reentrant compare in MultiDict.__eq__ (#1580)

<!-- Thank you for your contribution! -->

## What do these changes do?

`md_eq()` cached both entry arrays and passed borrowed values to
`PyObject_RichCompareBool()`. A value's `__eq__` runs on the same
thread, so the critical sections do not stop it from clearing or growing
either dict, which freed the cached table and the values being compared.
The C extension now holds references to both values and reloads the
entry arrays around any compare that can run Python code. Identical
values and exact `str` values skip that and are compared directly, which
makes the common header case faster than before.

The pure-Python implementation is memory safe here and is unchanged;
after a mid-compare mutation the result is unspecified on both backends,
as it is for `dict`.

## Are there changes in behavior for the user?

No, apart from the crash being fixed.

## Is it a substantial burden for the maintainers to support this?

No.

## Related issue number

Fixes #1579

## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes (N/A, no doc changes needed)
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt` (N/A)
- [x] Add a new news fragment into the `CHANGES/` folder
- [x] `make doc-spelling` passes and any new technical words are added
to `docs/spelling_wordlist.txt`

<details>
<summary>Agent run details (optional, for reviewers)</summary>

Tests (all on this branch):
- ASan GIL 3.14.7 (`MULTIDICT_DEBUG_BUILD=1 MULTIDICT_ASAN_BUILD=1
MULTIDICT_NO_FREELIST=1`, `PYTHONMALLOC=malloc`): 2685 passed. The new
`test_eq_value_mutates_dict` reports heap-use-after-free in `md_eq` on
the base commit.
- 3.14.7t C extension: 2691 passed.
- 3.14.7t pure Python (`--no-c-extensions`): 1282 passed.
- 3.10.17 C extension: 2704 passed.

Callgrind, instructions per `a == b` on 200 entries (`PYTHONHASHSEED=0`,... (continued)

779 of 1558 branches covered (50.0%)

Branch coverage included in aggregate %.

12 of 12 new or added lines in 1 file covered. (100.0%)

8019 of 8548 relevant lines covered (93.81%)

1.88 hits per line