Skip to content

require PyCFunction::new_closure closures to be Sync#6096

Merged
davidhewitt merged 3 commits into
PyO3:mainfrom
davidhewitt:sync-bound-new-closure
Jun 4, 2026
Merged

require PyCFunction::new_closure closures to be Sync#6096
davidhewitt merged 3 commits into
PyO3:mainfrom
davidhewitt:sync-bound-new-closure

Conversation

@davidhewitt

Copy link
Copy Markdown
Member

It turns out that PyCFunction::new_closure has a thread-safety bug - the closure is not required to be Sync. This is worse under the free-threaded build, but also a problem under the GIL-enabled build because of the existence of Python::detach() allowing the closure to do work without synchronization.

I think this is worth reporting to the RustSec Advisory Database; it is possible that existing user code is affected and subtly broken.

I couldn't think of a way to test for this other than UI tests.

(Credit to Claude for the discovery.)

@davidhewitt davidhewitt mentioned this pull request Jun 2, 2026
@davidhewitt

Copy link
Copy Markdown
Member Author

Wow, I hadn't noticed that we had violated this bound within our own test suite 😬

@davidhewitt davidhewitt enabled auto-merge June 4, 2026 07:40
@davidhewitt davidhewitt added this pull request to the merge queue Jun 4, 2026
Merged via the queue into PyO3:main with commit 9cc7862 Jun 4, 2026
48 checks passed
@davidhewitt davidhewitt deleted the sync-bound-new-closure branch June 4, 2026 08:33
mikbry added a commit to mikbry/ui that referenced this pull request Jun 12, 2026
) (#115)

Two RUSTSEC advisories published 2026-06-11 against pyo3 0.28.x, both fixed
in 0.29.0:

- RUSTSEC-2026-0176 — out-of-bounds read (memory exposure) in
  BoundListIterator/BoundTupleIterator nth/nth_back (PyO3/pyo3#6086)
- RUSTSEC-2026-0177 — missing Sync bound on PyCFunction::new_closure
  closures (PyO3/pyo3#6096)

mkui-py calls none of the affected APIs, so this is a pure dependency bump
with no source changes. Version lives in [workspace.dependencies]; mkui-py
consumes it via { workspace = true }.

Verified locally:
- cargo deny check advisories — ok (both IDs resolved)
- cargo audit — no vulnerabilities
- cargo test -p mkui-py --no-default-features --features parity-test,console — pass
- cargo fmt/clippy/test/doc/release — all green
- no new transitive deps, no MSRV change (0.29 needs 1.63; workspace is 1.87)

Closes #114

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants