fix(host): remove unsound unsafe impl Sync for HelperRelay
apple / swift (push) Failing after 0s
release / apple (push) Failing after 0s
apple / screenshots (push) Has been skipped
windows-drivers / probe-and-proto (push) Successful in 29s
audit / cargo-audit (push) Failing after 1m20s
windows-drivers / driver-build (push) Successful in 1m14s
android / android (push) Failing after 2m5s
ci / web (push) Successful in 46s
ci / docs-site (push) Successful in 1m3s
windows-host / package (push) Successful in 6m46s
ci / bench (push) Successful in 4m34s
windows-msix / package (arm64, C:\Users\Public\ffmpeg-arm64, aarch64-pc-windows-msvc, C:\t-a64) (push) Successful in 1m25s
ci / rust (push) Successful in 8m36s
decky / build-publish (push) Successful in 22s
windows-msix / package (x64, C:\Users\Public\ffmpeg, x86_64-pc-windows-msvc, C:\t) (push) Successful in 1m11s
windows / build (aarch64-pc-windows-msvc) (push) Successful in 59s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 2m37s
windows / build (x86_64-pc-windows-msvc) (push) Successful in 1m3s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 29s
deb / build-publish (push) Successful in 7m50s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 2m52s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 1m5s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 2m33s
flatpak / build-publish (push) Successful in 3m56s
rpm / build-publish (bazzite, punktfunk-fedora-rpm) (push) Successful in 8m46s
docker / deploy-docs (push) Successful in 22s
rpm / build-publish (fedora-44, punktfunk-fedora44-rpm) (push) Successful in 8m26s
apple / swift (push) Failing after 0s
release / apple (push) Failing after 0s
apple / screenshots (push) Has been skipped
windows-drivers / probe-and-proto (push) Successful in 29s
audit / cargo-audit (push) Failing after 1m20s
windows-drivers / driver-build (push) Successful in 1m14s
android / android (push) Failing after 2m5s
ci / web (push) Successful in 46s
ci / docs-site (push) Successful in 1m3s
windows-host / package (push) Successful in 6m46s
ci / bench (push) Successful in 4m34s
windows-msix / package (arm64, C:\Users\Public\ffmpeg-arm64, aarch64-pc-windows-msvc, C:\t-a64) (push) Successful in 1m25s
ci / rust (push) Successful in 8m36s
decky / build-publish (push) Successful in 22s
windows-msix / package (x64, C:\Users\Public\ffmpeg, x86_64-pc-windows-msvc, C:\t) (push) Successful in 1m11s
windows / build (aarch64-pc-windows-msvc) (push) Successful in 59s
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Successful in 2m37s
windows / build (x86_64-pc-windows-msvc) (push) Successful in 1m3s
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Successful in 29s
deb / build-publish (push) Successful in 7m50s
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Successful in 2m52s
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Successful in 1m5s
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Successful in 2m33s
flatpak / build-publish (push) Successful in 3m56s
rpm / build-publish (bazzite, punktfunk-fedora-rpm) (push) Successful in 8m46s
docker / deploy-docs (push) Successful in 22s
rpm / build-publish (fedora-44, punktfunk-fedora44-rpm) (push) Successful in 8m26s
The one genuine soundness defect the unsafe-proof program surfaced (flagged SUSPECT in program 3/N). `HelperRelay` holds an `rx: Receiver<RelayAu>`, which is `!Sync` (std mpsc is single-consumer), so asserting `Sync` claimed more than the fields support — an `Arc<HelperRelay>` recv'd from two threads would compile and be UB. It was never live-exploited, and it turns out `Sync` is also unnecessary: the relay is a single-owner `mut relay` local in the punktfunk1 two-process mux loop (recv_timeout/try_recv/request_keyframe all called on the owning thread; no `Arc`, no `thread::spawn` capturing it). So the fix is simply to delete the impl — the struct keeps its sound `unsafe impl Send` (needed for the raw `HANDLE` fields), which is all the code uses. Box-verified: cargo clippy -p punktfunk-host --features nvenc --target x86_64-pc-windows-msvc -- -D warnings stays green without the Sync impl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -64,14 +64,10 @@ pub struct HelperRelay {
|
|||||||
// on Drop), `stdin_w` is a `Mutex<HANDLE>`, and `rx` is an mpsc `Receiver<RelayAu>` (which is `Send`).
|
// on Drop), `stdin_w` is a `Mutex<HANDLE>`, and `rx` is an mpsc `Receiver<RelayAu>` (which is `Send`).
|
||||||
// The relay is moved to one thread and owned there, so transferring it across threads is sound.
|
// The relay is moved to one thread and owned there, so transferring it across threads is sound.
|
||||||
unsafe impl Send for HelperRelay {}
|
unsafe impl Send for HelperRelay {}
|
||||||
// SAFETY: SUSPECT — `rx: Receiver<RelayAu>` is `!Sync` (std mpsc is single-consumer; two threads
|
// NOTE: `HelperRelay` is deliberately NOT `Sync`. Its `rx: Receiver<RelayAu>` is `!Sync` (std mpsc
|
||||||
// calling `recv_timeout`/`try_recv` through a shared `&HelperRelay` would be a data race on the
|
// is single-consumer), and the relay is only ever a single-owner local in the punktfunk1 two-process
|
||||||
// channel's consumer state → UB), and both are `&self` methods, so this `unsafe impl Sync` asserts
|
// mux loop — never shared by `&` across threads — so `Sync` is neither sound nor needed. (A prior
|
||||||
// more than the field types support. It is not a LIVE bug only because the sole consumer (the
|
// `unsafe impl Sync` here asserted more than the fields support; removed.)
|
||||||
// punktfunk1 two-process mux loop) owns the relay and never `&`-shares it for receiving — other
|
|
||||||
// threads reach only `request_keyframe`, which is `stdin_w`-Mutex-guarded — but nothing in the type
|
|
||||||
// enforces that invariant. An `Arc<HelperRelay>` recv'd from two threads would compile and be UB.
|
|
||||||
unsafe impl Sync for HelperRelay {}
|
|
||||||
|
|
||||||
/// Control byte on the helper's stdin: force the next encoded frame to be an IDR (client decode
|
/// Control byte on the helper's stdin: force the next encoded frame to be an IDR (client decode
|
||||||
/// recovery). Mirrors `enc.request_keyframe()` in the single-process path.
|
/// recovery). Mirrors `enc.request_keyframe()` in the single-process path.
|
||||||
|
|||||||
Reference in New Issue
Block a user