unom/punktfunk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 16 Wiki Activity

fix(host): remove unsound unsafe impl Sync for HelperRelay

Browse Source 
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:
Enrico Bühler
2026-06-26 10:00:40 +00:00
parent 33328ee417
commit bdc2e44fb2
1 changed files with 4 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/punktfunk-host/src/capture/windows/wgc_relay.rs
+4 -8
View File
@@ -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`).
// 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<RelayAu>` 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<HelperRelay>` recv'd from two threads would compile and be UB.
unsafe impl Sync for HelperRelay {}
// NOTE: `HelperRelay` is deliberately NOT `Sync`. Its `rx: Receiver<RelayAu>` 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.