From 3e7c9bd059b062c7e7e6a79a60297e8d318a6a76 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 26 Jun 2026 10:00:40 +0000 Subject: [PATCH] fix(host): remove unsound `unsafe impl Sync for HelperRelay` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one genuine soundness defect the unsafe-proof program surfaced (flagged SUSPECT in program 3/N). `HelperRelay` holds an `rx: Receiver`, which is `!Sync` (std mpsc is single-consumer), so asserting `Sync` claimed more than the fields support — an `Arc` 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) --- .../punktfunk-host/src/capture/windows/wgc_relay.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/punktfunk-host/src/capture/windows/wgc_relay.rs b/crates/punktfunk-host/src/capture/windows/wgc_relay.rs index 2676b0a..3a6f170 100644 --- a/crates/punktfunk-host/src/capture/windows/wgc_relay.rs +++ b/crates/punktfunk-host/src/capture/windows/wgc_relay.rs @@ -64,14 +64,10 @@ pub struct HelperRelay { // on Drop), `stdin_w` is a `Mutex`, and `rx` is an mpsc `Receiver` (which is `Send`). // The relay is moved to one thread and owned there, so transferring it across threads is sound. unsafe impl Send for HelperRelay {} -// SAFETY: SUSPECT — `rx: Receiver` is `!Sync` (std mpsc is single-consumer; two threads -// calling `recv_timeout`/`try_recv` through a shared `&HelperRelay` would be a data race on the -// channel's consumer state → UB), and both are `&self` methods, so this `unsafe impl Sync` asserts -// more than the field types support. It is not a LIVE bug only because the sole consumer (the -// 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` recv'd from two threads would compile and be UB. -unsafe impl Sync for HelperRelay {} +// NOTE: `HelperRelay` is deliberately NOT `Sync`. Its `rx: Receiver` is `!Sync` (std mpsc +// is single-consumer), and the relay is only ever a single-owner local in the punktfunk1 two-process +// mux loop — never shared by `&` across threads — so `Sync` is neither sound nor needed. (A prior +// `unsafe impl Sync` here asserted more than the fields support; removed.) /// 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.