aio-libs / multidict / 34813154583

87%

push

github

Fix a data race on self->state in update()/extend()/merge() (#1452)

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

## What do these changes do?

`update()`, `extend()`, and `merge()` used to read `self->state` to
parse
their arguments before acquiring `self`'s critical section, while
`__init__()` on an already-published, shared multidict
(`d.__init__(other)`)
rewrote that same field under lock via `md_clone_from_ht()`/`md_init()`
on
every reinit, racing with those unlocked reads.

Rather than routing reads around the racy field, this closes the race at
its root: `self->state` is now written exactly once, at object
construction (`multidict_tp_new`, the vectorcall constructor, and
`multidict_copy`), and `__init__()`/clone never touch it again.
`md_init()`
no longer takes a `state` parameter and instead asserts
`md->state != NULL`; `md_clone_from_ht()` no longer touches `state` at
all. `self->state` is therefore safe to read unlocked from anywhere, not
just the specific call sites that happened to need fixing.

## Are there changes in behavior for the user?

No.

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

No, it narrows `md_init()`'s responsibility and moves the one-time state
assignment to the three real construction sites.

## Related issue number

N/A, found via a ThreadSanitizer run against a TSan-instrumented
free-threaded CPython build, not a filed issue.

## Checklist

- [x] I think the code is well written
- [ ] Unit tests for the changes exist
- [ ] Documentation reflects the changes
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
- [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>

No dedicated regression test: since `__init__()` no longer writes
`self->state` at all, racing it against `update()`/`extend()`/`me... (continued)

715 of 1430 branches covered (50.0%)

Branch coverage included in aggregate %.

5179 of 5425 relevant lines covered (95.47%)

1.91 hits per line