fix(presenter): resize crash — never vkDeviceWaitIdle while the pump decodes

vkDeviceWaitIdle's external-sync rule claims EVERY queue on the device;
with Vulkan Video the session pump concurrently submits FFmpeg decode
work to the shared device's video queue, so the resize path's wait-idle
(and the video-image/staging rebuilds') raced it — observed as a crash
on window resize mid-stream (software/VAAPI never touched the device
from the pump, which is why this only appeared now).

Mid-session quiescing is now fence-only ( waits the single
in-flight fence, which covers every command buffer WE submitted), and
the replaced swapchain + its per-image semaphores/overlay targets are
parked in a retire list — the presentation engine may still hold the old
swapchain's final semaphore wait, which no fence covers — and destroyed
after the next present on the successor completes a fence cycle. The one
legitimate device-wide idle left is teardown, and the run loop now JOINS
the pump thread first (SessionHandle carries the JoinHandle; quick — the
pump notices stop within its 20 ms receive timeout).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

crates/pf-client-core/src/session.rs

@@ -131,6 +131,10 @@ pub struct SessionHandle {
     pub events: async_channel::Receiver<SessionEvent>,
     pub frames: async_channel::Receiver<DecodedFrame>,
     pub stop: Arc<AtomicBool>,
+    /// The pump thread. A Vulkan-Video pump SUBMITS to the shared device's decode
+    /// queue — the presenter must join this before any `vkDeviceWaitIdle`/teardown
+    /// (external-sync rule over every device queue).
+    pub thread: Option<std::thread::JoinHandle<()>>,
 }
 
 pub fn start(params: SessionParams) -> SessionHandle {
@@ -139,7 +143,7 @@ pub fn start(params: SessionParams) -> SessionHandle {
     let (frame_tx, frame_rx) = async_channel::bounded(2);
     let stop = Arc::new(AtomicBool::new(false));
     let stop_w = stop.clone();
-    std::thread::Builder::new()
+    let thread = std::thread::Builder::new()
         .name("punktfunk-session".into())
         .spawn(move || pump(params, ev_tx, frame_tx, stop_w))
         .expect("spawn session thread");
@@ -147,6 +151,7 @@ pub fn start(params: SessionParams) -> SessionHandle {
         events: ev_rx,
         frames: frame_rx,
         stop,
+        thread: Some(thread),
     }
 }
 

crates/pf-presenter/src/run.rs

@@ -170,6 +170,17 @@ impl StreamState {
         }
     }
 
+    /// Stop the pump and JOIN its thread — required before any device-wide idle or
+    /// teardown (the pump submits decode work to the shared device). Quick: the pump
+    /// notices `stop` within its 20 ms receive timeout, and on a normal end it's
+    /// already returning.
+    fn shutdown(mut self) {
+        self.handle.stop.store(true, Ordering::SeqCst);
+        if let Some(t) = self.handle.thread.take() {
+            let _ = t.join();
+        }
+    }
+
     /// Deliberate user exit (chord / window close): release capture, close with
     /// QUIT_CLOSE_CODE so the host tears down instead of lingering, stop the pump.
     /// The pump then emits `Ended(None)` — the loop's normal end path picks it up.
@@ -530,7 +541,9 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
                     }
                     ModeCtl::Browse(_) => {
                         tracing::warn!(%msg, "connect failed — back to the library");
-                        stream = None;
+                        if let Some(st) = stream.take() {
+                            st.shutdown();
+                        }
                         mouse.set_relative_mouse_mode(&window, false);
                         mouse.show_cursor(true);
                         if let Some(o) = overlay.as_mut() {
@@ -550,7 +563,9 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
                         ModeCtl::Single(_) => break 'main Some(Outcome::Ended(reason)),
                         ModeCtl::Browse(_) => {
                             window.set_title(&opts.window_title).ok();
-                            stream = None;
+                            if let Some(st) = stream.take() {
+                                st.shutdown();
+                            }
                             if let Some(o) = overlay.as_mut() {
                                 o.session_phase(SessionPhase::Ended(reason.as_deref()));
                             }
@@ -736,8 +751,10 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
         }
     };
 
-    if let Some(st) = &stream {
+    // Join the pump BEFORE the device-wide idle: its decode submissions on the shared
-        st.handle.stop.store(true, Ordering::SeqCst);
+    // device would race vkDeviceWaitIdle otherwise.
+    if let Some(st) = stream.take() {
+        st.shutdown();
     }
     // Overlay resources live on the presenter's device: quiesce the queue first, drop
     // the overlay (its Drop destroys the Skia surfaces), THEN the presenter tears down.

crates/pf-presenter/src/vk.rs

@@ -65,6 +65,37 @@ impl Retired {
     }
 }
 
+/// A replaced swapchain and its per-image objects, parked until a present + fence on
+/// the NEW swapchain proves the presentation engine is past them (its semaphore waits
+/// are outside fence visibility). This is what lets resize avoid `vkDeviceWaitIdle`,
+/// which would race FFmpeg's decode submissions from the pump thread (external-sync
+/// rule: wait-idle claims EVERY queue on the device).
+struct DisplayGarbage {
+    swapchain: vk::SwapchainKHR,
+    render_sems: Vec<vk::Semaphore>,
+    overlay_views: Vec<vk::ImageView>,
+    overlay_framebuffers: Vec<vk::Framebuffer>,
+}
+
+impl DisplayGarbage {
+    fn destroy(self, device: &ash::Device, swap_d: &ash::khr::swapchain::Device) {
+        unsafe {
+            for fb in self.overlay_framebuffers {
+                device.destroy_framebuffer(fb, None);
+            }
+            for v in self.overlay_views {
+                device.destroy_image_view(v, None);
+            }
+            for s in self.render_sems {
+                device.destroy_semaphore(s, None);
+            }
+            if self.swapchain != vk::SwapchainKHR::null() {
+                swap_d.destroy_swapchain(self.swapchain, None);
+            }
+        }
+    }
+}
+
 /// The overlay composite: one premultiplied-alpha quad blended over the swapchain image
 /// after the video blit (the §6.1 contract's presenter half). Always built — it has no
 /// Skia dependency and costs nothing while no overlay frame arrives (the render pass
@@ -188,7 +219,16 @@ impl OverlayPipe {
         })
     }
 
+    /// Detach the current per-swapchain-image targets (for deferred destruction).
+    fn take_targets(&mut self) -> (Vec<vk::ImageView>, Vec<vk::Framebuffer>) {
+        (
+            std::mem::take(&mut self.views),
+            std::mem::take(&mut self.framebuffers),
+        )
+    }
+
     /// Rebuild the per-swapchain-image views + framebuffers (swapchain recreation).
+    /// The caller has already taken the old targets for deferred destruction.
     fn rebuild_targets(
         &mut self,
         device: &ash::Device,
@@ -196,7 +236,7 @@ impl OverlayPipe {
         format: vk::Format,
         extent: vk::Extent2D,
     ) -> Result<()> {
-        self.destroy_targets(device);
+        self.destroy_targets(device); // no-op after take_targets; safety net otherwise
         for &image in images {
             let view = unsafe {
                 device.create_image_view(
@@ -292,6 +332,9 @@ pub struct Presenter {
     /// frame + our plane views): its GPU reads end with the in-flight fence, so it's
     /// destroyed right after the next fence wait.
     retired_hw: Option<Retired>,
+    /// Replaced swapchains (resize), destroyed once a present on the successor has
+    /// gone through a fence cycle.
+    retired_display: Vec<DisplayGarbage>,
     format: vk::SurfaceFormatKHR,
     present_mode: vk::PresentModeKHR,
     swapchain: vk::SwapchainKHR,
@@ -590,6 +633,7 @@ impl Presenter {
             video_export,
             overlay_pipe,
             retired_hw: None,
+            retired_display: Vec::new(),
             format,
             present_mode,
             swapchain: vk::SwapchainKHR::null(),
@@ -608,10 +652,23 @@ impl Presenter {
         Ok(p)
     }
 
+    /// Wait the in-flight fence: OUR command buffers are done (staging, video image,
+    /// old-swapchain images). Deliberately NOT `vkDeviceWaitIdle` — the pump thread
+    /// submits FFmpeg's Vulkan decode work concurrently, and wait-idle's external-sync
+    /// rule over every device queue would race it (observed as a resize crash).
+    fn quiesce_own(&mut self) -> Result<()> {
+        unsafe {
+            if self.submitted {
+                self.device.wait_for_fences(&[self.fence], true, u64::MAX)?;
+                self.submitted = false;
+            }
+        }
+        Ok(())
+    }
+
     /// (Re)build the swapchain for the window's current pixel size. Also the resize path.
     pub fn recreate_swapchain(&mut self, window: &sdl3::video::Window) -> Result<()> {
-        unsafe { self.device.device_wait_idle() }.ok();
+        self.quiesce_own()?;
-        self.submitted = false;
 
         let caps = unsafe {
             self.surface_i
@@ -655,18 +712,23 @@ impl Presenter {
             .old_swapchain(old);
         let swapchain = unsafe { self.swap_d.create_swapchain(&info, None) }
             .context("vkCreateSwapchainKHR")?;
-        if old != vk::SwapchainKHR::null() {
+        // The old swapchain (and everything tied to its images) is parked, not
-            unsafe { self.swap_d.destroy_swapchain(old, None) };
+        // destroyed: the presentation engine may still hold its final present's
-        }
+        // semaphore wait, which no fence covers. It dies after the next present on
+        // the NEW swapchain has gone through a fence cycle.
+        let (overlay_views, overlay_framebuffers) = self.overlay_pipe.take_targets();
+        self.retired_display.push(DisplayGarbage {
+            swapchain: old,
+            render_sems: std::mem::take(&mut self.render_sems),
+            overlay_views,
+            overlay_framebuffers,
+        });
         self.swapchain = swapchain;
         self.images = unsafe { self.swap_d.get_swapchain_images(swapchain) }?;
         self.extent = extent;
         self.overlay_pipe
             .rebuild_targets(&self.device, &self.images, self.format.format, extent)?;
 
-        for s in self.render_sems.drain(..) {
-            unsafe { self.device.destroy_semaphore(s, None) };
-        }
         for _ in 0..self.images.len() {
             self.render_sems.push(unsafe {
                 self.device
@@ -695,8 +757,9 @@ impl Presenter {
         self.video_export.clone()
     }
 
-    /// Quiesce the queue — the run loop calls this before dropping the overlay so
+    /// Full device idle — TEARDOWN ONLY, and only after the session pump thread has
-    /// nothing in flight still references its images.
+    /// been joined (it submits FFmpeg decode work; wait-idle's external-sync rule
+    /// covers every queue on the device). Mid-session code uses the fence quiesce.
     pub fn wait_idle(&self) {
         unsafe { self.device.device_wait_idle() }.ok();
     }
@@ -763,6 +826,11 @@ impl Presenter {
         if let Some(old) = self.retired_hw.take() {
             old.destroy(&self.device);
         }
+        // A fence cycle has completed since the swapchain swap — the old display
+        // objects are past every wait now.
+        for g in self.retired_display.drain(..) {
+            g.destroy(&self.device, &self.swap_d);
+        }
 
         if let Some(f) = cpu_frame {
             self.stage_frame(f)?;
@@ -1231,8 +1299,8 @@ impl Presenter {
     }
 
     fn rebuild_video_image(&mut self, width: u32, height: u32) -> Result<()> {
-        unsafe { self.device.device_wait_idle() }.ok();
+        // Fence-quiesce: the old image is only ever referenced by OUR command buffers.
-        self.submitted = false;
+        self.quiesce_own()?;
         if let Some(v) = self.video.take() {
             unsafe {
                 if v.framebuffer != vk::Framebuffer::null() {
@@ -1308,8 +1376,7 @@ impl Presenter {
     }
 
     fn rebuild_staging(&mut self, capacity: usize) -> Result<()> {
-        unsafe { self.device.device_wait_idle() }.ok();
+        self.quiesce_own()?;
-        self.submitted = false;
         if let Some(s) = self.staging.take() {
             unsafe {
                 self.device.unmap_memory(s.memory);
@@ -1392,6 +1459,10 @@ impl Drop for Presenter {
                 self.device.destroy_image(v.image, None);
                 self.device.free_memory(v.memory, None);
             }
+            for g in self.retired_display.drain(..) {
+                let device = self.device.clone();
+                g.destroy(&device, &self.swap_d);
+            }
             self.hw.take();
             self.csc.destroy(&self.device);
             self.overlay_pipe.destroy(&self.device);