From cd701a95946015f543392aa1f8a78f5594665127 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 13 Jul 2026 00:19:20 +0200 Subject: [PATCH] fix(vdisplay): preserve FramePublisher across swap-chain reassign (sibling-join freeze) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a second client got its own virtual display mid-stream, the FIRST client's IDD-push stream froze (video only; `new_fps=0 repeat_fps=240` forever). Adding/removing/resizing a sibling display re-commits the CCD topology, which makes the OS unassign+reassign the first monitor's IddCx swap chain. `unassign_swap_chain` dropped the SwapChainProcessor, dropping `run_core`'s local FramePublisher and closing the sealed-channel handles. The fresh worker then polled the frame-channel stash — but that stash is consumed once at session open, and the host only re-delivers on a ring recreate (a descriptor change). The first monitor's descriptor didn't change and WUDFHost stayed alive, so no watchdog fired: the driver drained the swap chain without publishing and the host repeated its last frame indefinitely. Confirmed twice on the .173 box (host.log 21:12 & 21:15). Preserve the live FramePublisher across the flap instead of dropping it: the host-owned ring (header/event/textures) it holds stays valid — only the swap chain died. - frame_transport.rs: FramePublisher records its render-adapter LUID + exposes render_adapter(). - monitor.rs: MonitorObject.preserved_publisher + preserve_publisher() (mirrors set_frame_channel) + take_preserved_publisher() (mirrors take_frame_channel). Monitor teardown drops the stashed publisher and closes its ring handles, so nothing leaks. - swap_chain_processor.rs run_core: after SetDevice OK, re-adopt a preserved publisher ONLY when the new swap chain renders on the same LUID (same pooled Direct3DDevice → its context + opened textures are valid); on loop exit, stash the publisher back on the monitor. Safe: the old worker is fully joined (drop-outside-lock discipline) before the new one runs, so no concurrent context use; a stale re-adopted publisher is superseded by the existing is_stale() + has_frame_channel() newest-wins checks at the loop top. Verified clippy -D warnings clean on rustc 1.96.0 via a faithful mock crate (the real crate needs the WDK to compile). Needs a driver rebuild + reinstall on the host to take effect; not yet hardware-validated. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pf-vdisplay/src/frame_transport.rs | 15 +++++ .../drivers/pf-vdisplay/src/monitor.rs | 55 +++++++++++++++++++ .../pf-vdisplay/src/swap_chain_processor.rs | 37 ++++++++++++- 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs b/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs index 356d94a8..c2e8fc21 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs @@ -146,6 +146,12 @@ pub struct FramePublisher { /// Set when a surface is dropped for a descriptor mismatch (a game mode-set the display), cleared on a /// matched publish — throttles the drop log to once per mismatch episode (game-capture bug GB1). mismatch_logged: bool, + /// The render adapter (LUID) this publisher's device + opened ring textures live on. A worker + /// re-adopts a publisher preserved across a swap-chain unassign→reassign flap ONLY when the + /// freshly-assigned swap-chain renders on this SAME adapter (else the opened textures would be + /// cross-device); see [`Self::render_adapter`] + `swap_chain_processor::run_core`. + render_luid_low: u32, + render_luid_high: i32, } // SAFETY: created and used only on the swap-chain processor thread. @@ -356,6 +362,8 @@ impl FramePublisher { ring_format: unsafe { (*header).dxgi_format }, generation: header_gen, mismatch_logged: false, + render_luid_low, + render_luid_high, }) } @@ -379,6 +387,13 @@ impl FramePublisher { cur != self.generation } + /// The render adapter (LUID) this publisher's device + opened ring textures live on. The swap-chain + /// worker re-adopts a publisher preserved across an unassign→reassign flap only when the freshly- + /// assigned swap-chain renders on this same adapter (see the field docs + `run_core`). + pub fn render_adapter(&self) -> (u32, i32) { + (self.render_luid_low, self.render_luid_high) + } + /// Copy `surface` into the next free ring slot and signal the host. Never blocks (0 ms try-acquire). pub fn publish(&mut self, surface: &ID3D11Texture2D) { let ring_len = self.slots.len() as u32; diff --git a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs index c0eac21b..2f9d950c 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs @@ -58,6 +58,17 @@ pub struct MonitorObject { /// closes an unconsumed channel's handles via [`FrameChannel`]'s `Drop`, so no delivery can leak /// handles in the WUDFHost table whatever the monitor's fate. pub frame_channel: Option, + /// A live [`FramePublisher`](crate::frame_transport::FramePublisher) preserved across a swap-chain + /// unassign→reassign flap (STEP 6 sibling-join fix). The OS unassigns a monitor's swap-chain + /// whenever a SIBLING display churns the desktop topology (a second client joining / leaving / + /// resizing), which drops the swap-chain worker — but the HOST-owned ring (header / event / + /// textures) the publisher holds stays valid, and the host only re-delivers the frame channel on a + /// ring RECREATE (a descriptor change), so a fresh worker had nothing to re-attach from and the + /// first client's stream froze (repeat frames forever). The exiting worker stashes its still-live + /// publisher here ([`preserve_publisher`]); the next worker on the SAME render adapter takes it back + /// ([`take_preserved_publisher`]) and resumes publishing into the same ring. Dropped with the + /// `MonitorObject` on teardown (closing its ring handles) if no worker ever reclaims it. + pub preserved_publisher: Option, /// When the entry was created — the watchdog skips still-initializing monitors. pub created_at: Instant, } @@ -318,6 +329,49 @@ pub fn has_frame_channel(target_id: u32) -> bool { .any(|m| m.target_id == target_id && m.frame_channel.is_some()) } +/// Stash a swap-chain worker's still-live [`FramePublisher`](crate::frame_transport::FramePublisher) on +/// its monitor across a swap-chain unassign→reassign flap (STEP 6 sibling-join fix; see the field docs +/// on [`MonitorObject::preserved_publisher`]). Called from the EXITING worker thread — the caller must +/// NOT hold `MONITOR_MODES` (this locks it), matching the same drop-outside-the-lock discipline the +/// processor teardown paths use. Returns `Err(publisher)` when no monitor with `target_id` exists (a +/// genuine teardown, not a flap: the entry was already removed) so the caller drops it, closing the ring +/// handles. Replacing an already-stashed publisher (should not happen — one worker exits at a time) +/// drops the old one, so it can never accumulate. Returning the publisher in the `Err` makes the +/// `Result` itself `#[must_use]`, so a caller can't silently drop the not-preserved publisher. +pub fn preserve_publisher( + target_id: u32, + publisher: crate::frame_transport::FramePublisher, +) -> Result<(), crate::frame_transport::FramePublisher> { + if target_id == 0 { + return Err(publisher); + } + let mut lock = lock_monitors(); + if let Some(m) = lock.iter_mut().find(|m| m.target_id == target_id) { + m.preserved_publisher = Some(publisher); + Ok(()) + } else { + Err(publisher) + } +} + +/// Take (remove) a preserved [`FramePublisher`](crate::frame_transport::FramePublisher) for a freshly- +/// (re)assigned swap-chain worker (STEP 6 sibling-join fix). The caller re-adopts it ONLY when the new +/// swap-chain's render adapter matches the publisher's ([`FramePublisher::render_adapter`]) — same +/// pooled device, so its context + opened ring textures are still valid; on a mismatch the caller drops +/// it and waits for a fresh channel delivery instead. `None` until a worker has stashed one. +pub fn take_preserved_publisher( + target_id: u32, +) -> Option { + if target_id == 0 { + return None; + } + lock_monitors() + .iter_mut() + .find(|m| m.target_id == target_id)? + .preserved_publisher + .take() +} + /// Install a swap-chain processor on the monitor whose handle matches, returning any PREVIOUS processor /// for the caller to drop OUTSIDE the lock. Dropping a processor RAII-joins its worker thread, so it must /// never happen while holding `MONITOR_MODES` (the worker would block the whole control plane / risk a @@ -406,6 +460,7 @@ pub fn create_monitor( adapter_luid_high: 0, swap_chain_processor: None, frame_channel: None, + preserved_publisher: None, created_at: Instant::now(), }); id diff --git a/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs b/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs index 902d9c20..381ead74 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs @@ -241,7 +241,31 @@ impl SwapChainProcessor { // the values via IOCTL_SET_FRAME_CHANNEL, which the control plane stashes on our monitor // (`monitor::take_frame_channel`). Until a delivery lands we just drain — exactly the STEP-5 // behaviour — so a non-IDD-push session never stalls. The stash is polled every ~30 iterations. - let mut publisher: Option = None; + // STEP 6 sibling-join fix: re-adopt a FramePublisher PRESERVED across a swap-chain + // unassign→reassign flap. When a SIBLING display churns the desktop topology (a second client + // joining / leaving / resizing), the OS reassigns THIS monitor's swap-chain and the previous + // worker exited — but the host-owned ring it published into is still live, and the host only + // re-delivers the frame channel on a ring RECREATE (a descriptor change). Without this the + // fresh worker has nothing to attach to and the first client's stream freezes (repeat frames + // forever). Re-adopt ONLY when the freshly-assigned swap-chain renders on the SAME adapter as + // the preserved publisher (same pooled Direct3DDevice → its immediate context + opened ring + // textures are valid); a mismatch drops it and falls back to a fresh channel delivery. A + // preserved publisher that the host superseded meanwhile (ring recreate → is_stale, or a + // pending delivery) is dropped + replaced by the existing re-attach logic at the loop top. + let mut publisher: Option = + crate::monitor::take_preserved_publisher(target_id).and_then(|p| { + if p.render_adapter() == (render_luid_low, render_luid_high) { + dbglog!( + "[pf-vd] swap-chain run_core: re-adopted preserved publisher (target={target_id}) — resuming the host ring across the swap-chain flap" + ); + Some(p) + } else { + dbglog!( + "[pf-vd] swap-chain run_core: preserved publisher's render adapter changed (target={target_id}) — dropping it, will re-attach from a fresh channel" + ); + None + } + }); let mut frames_since_try: u32 = u32::MAX; // attach attempt on the first loop iteration let mut logged_pending = false; @@ -392,6 +416,17 @@ impl SwapChainProcessor { break; } } + + // STEP 6 sibling-join fix: the drain loop exited (the OS unassigned this swap-chain — typically + // because a SIBLING display churned the desktop topology — or it errored), but the host-owned + // ring the publisher holds is still live. Hand it to the monitor so the NEXT worker assigned to + // this monitor resumes publishing into the same ring instead of freezing (the host re-delivers + // the channel only on a ring recreate). If the monitor is GONE (a genuine teardown, not a flap), + // `preserve_publisher` hands the publisher back inside the `Err` and dropping the returned + // `Result` closes the ring handles here — no leak, no stale ring left behind. + if let Some(p) = publisher.take() { + let _ = crate::monitor::preserve_publisher(target_id, p); + } } }