aio-libs / multidict / 34864619006

87%

push

github

Lock pure-Python MultiDict mutators against concurrent corruption (#1449)

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

## What do these changes do?

Every method in the pure-Python `MultiDict`/`CIMultiDict` fallback
(`multidict/_multidict_py.py`) that mutates `self._keys`/`self._used`
(`add()`, `__setitem__`/`__delitem__`, `setdefault()`,
`pop()`/`popone()`/`popall()`, `popitem()`, `update()`, `extend()`,
`merge()`, `clear()`, and re-`__init__`) ran without any
synchronization on a GIL-enabled build. Pure-Python bytecode is not
atomic even under the GIL, so two threads calling these concurrently
on the same instance (or reading one as the argument to another
thread's `update()`/`extend()`/`merge()`) can interleave mid
hash-table insert or deletion and corrupt the shared index table
(hanging in an infinite probe loop or raising `AttributeError`), or
silently drop a completed write (e.g. a concurrent `add()` writing
into a table an in-progress `extend()`'s resize is about to replace).
This is reproducible on a regular, GIL-enabled interpreter given
enough contention; the coverage tracing this test suite already runs
under is enough to hit it reliably, no free-threaded build required.

Every `MultiDict`/`CIMultiDict` instance now gets its own `RLock`,
held for the duration of these operations. Two-object operations
(`__init__`/`update()`/`extend()`/`merge()` reading another
`MultiDict` as their argument) lock both instances in a fixed order to
avoid deadlock. `_add_with_hash()`/`_add_with_hash_for_upd()` and
`popitem()` also had their write ordering fixed so an index is only
published once its entry is reachable, and an entry's index is
cleared before the entry is dropped from the entries list.

Read-only methods (`getall()`, `getone()`, `__contains__()`,
`__eq__()`, `__repr__()`, `to_dict()`, iteration, view operations) are
unaffected and stay locked only on a free-threaded build, matching the
scope of #1447: this PR is about lost or corrupted writes bet... (continued)

727 of 1454 branches covered (50.0%)

Branch coverage included in aggregate %.

74 of 78 new or added lines in 2 files covered. (94.87%)

5296 of 5543 relevant lines covered (95.54%)

1.91 hits per line