From f4c3a5d0c33a2a603ad121573caf91bd44aa0d10 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 9 Jul 2026 12:42:11 +0200 Subject: [PATCH] =?UTF-8?q?fix(clients):=20shared-VkQueue=20race=20+=20swa?= =?UTF-8?q?pchain=20recreate=20destroy-in-use=20=E2=80=94=20the=20intermit?= =?UTF-8?q?tent=20device-lost=20stream=20killer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-diagnosed on the RTX box during the adaptive-bitrate A/B: 2 of 3 streams died with VK_ERROR_DEVICE_LOST at stream start or at a mid-stream encoder rebuild, then zombied — the demote-to-software path rebuilt the decoder against the same dead device, FFmpeg wedged inside the rebuild, and the pump flushed a never-draining backlog every 2 s forever. No OS TDR: the client's own Vulkan misuse. Two causes: 1. The presenter creates ONE graphics-family queue and hands FFmpeg's AVVulkanDeviceContext the same family (nb_graphics_queues=1 ⇒ queue 0) for its transfer/compute prep — so the pump thread and the presenter thread submitted to the SAME VkQueue with no shared lock, violating vkQueueSubmit's external-sync rule exactly when FFmpeg puts work on the graphics queue (decoder open / frames-context rebuild = stream start and every ABR encoder re-target). New guard-less QueueLock (FFmpeg's lock_queue/unlock_queue callbacks are a raw pair) shared by all four queue users: FFmpeg (callbacks installed via user_opaque), the presenter's submit/present/wait-idle, and the Skia overlay's flushes. 2. Swapchain recreation destroyed the old swapchain + render semaphores after ONE fence cycle — the fence proves our submit, not the presentation engine's semaphore consumption (VUID-vkDestroySemaphore-05149 + VUID-vkDestroySwapchainKHR-01282 on every recreate). Recreate now drains the queue (vkQueueWaitIdle under the shared lock — safe now that FFmpeg honours it) and destroys immediately; the deferred DisplayGarbage machinery is gone. Resilience: VK_ERROR_DEVICE_LOST anywhere in a present error chain is now fatal — the run loop fails the session loudly instead of demoting to software on a dead device (the zombie path). Verified on the RTX box (RTX 4090 → host .21) under VK_LAYER_KHRONOS_validation: 3/3 stream start/stop cycles clean, then 8 mid-stream encoder rebuilds in one session (4 ABR down-steps under 10% induced loss + 4 clean-link recovery up-steps — the exact scenarios that previously killed the device): 0 device losses, 0 wedges, both recreate VUIDs gone (previously fired on every path). Remaining validation messages are FFmpeg's own video-session VUIDs, untouched by this change. Linux: clippy -D warnings + tests green (home-worker-2). Co-Authored-By: Claude Fable 5 --- crates/pf-client-core/src/video.rs | 118 +++++++++++++++++ crates/pf-console-ui/src/skia_overlay.rs | 50 +++++--- crates/pf-presenter/src/overlay.rs | 4 + crates/pf-presenter/src/run.rs | 29 ++++- crates/pf-presenter/src/vk.rs | 154 ++++++++++++----------- 5 files changed, 265 insertions(+), 90 deletions(-) diff --git a/crates/pf-client-core/src/video.rs b/crates/pf-client-core/src/video.rs index 28b6d2c3..4a05dd27 100644 --- a/crates/pf-client-core/src/video.rs +++ b/crates/pf-client-core/src/video.rs @@ -797,6 +797,79 @@ impl VaapiDecoder { } } +/// Guard-less mutex serializing every `vkQueueSubmit`/`vkQueuePresentKHR`/ +/// `vkQueueWaitIdle` on the device the presenter shares with FFmpeg. +/// +/// Why it exists: the presenter created the device with ONE graphics-family queue and +/// told FFmpeg's `AVVulkanDeviceContext` to use that same family (`nb_graphics_queues +/// = 1` ⇒ queue index 0) for its transfer/compute prep work — so the presenter thread +/// and the session pump thread were submitting to the SAME `VkQueue` with no shared +/// lock. `vkQueueSubmit` requires external synchronization on the queue; the race +/// surfaced as intermittent `VK_ERROR_DEVICE_LOST` at exactly the moments FFmpeg puts +/// work on the graphics queue (decoder open / frames-context rebuild — i.e. stream +/// start and every adaptive-bitrate encoder rebuild; live-diagnosed 2026-07-09). +/// +/// FFmpeg's hook for this is the `lock_queue`/`unlock_queue` callback pair on +/// `AVVulkanDeviceContext` — a raw lock/unlock shape with no RAII scope, hence this +/// guard-less primitive (`std::sync::Mutex`'s guard can't cross the C callbacks). +/// Contention is a handful of µs-scale critical sections per frame; a plain +/// Mutex+Condvar is more than enough. +pub struct QueueLock { + locked: std::sync::Mutex, + cv: std::sync::Condvar, +} + +impl QueueLock { + #[allow(clippy::new_without_default)] + pub fn new() -> QueueLock { + QueueLock { + locked: std::sync::Mutex::new(false), + cv: std::sync::Condvar::new(), + } + } + + /// Block until the queue is free, then take it. Pair with [`QueueLock::unlock`] + /// (FFmpeg's callbacks), or use [`QueueLock::guard`] from Rust callers. + pub fn lock(&self) { + let mut g = self + .locked + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + while *g { + g = self + .cv + .wait(g) + .unwrap_or_else(std::sync::PoisonError::into_inner); + } + *g = true; + } + + pub fn unlock(&self) { + let mut g = self + .locked + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + *g = false; + drop(g); + self.cv.notify_one(); + } + + /// RAII form for Rust call sites (presenter submits/presents, Skia flushes). + pub fn guard(&self) -> QueueLockGuard<'_> { + self.lock(); + QueueLockGuard(self) + } +} + +/// Releases the [`QueueLock`] on drop. +pub struct QueueLockGuard<'a>(&'a QueueLock); + +impl Drop for QueueLockGuard<'_> { + fn drop(&mut self) { + self.0.unlock(); + } +} + /// The presenter's Vulkan device handles, exported so FFmpeg's Vulkan Video decoder /// runs on the SAME device the presenter samples from — the whole point: the decoded /// VkImage is composited directly, no interop, no copy (plan: Vulkan Video phase). @@ -839,6 +912,10 @@ pub struct VulkanDecodeDevice { /// backend creates its decode device on the SAME adapter so shared textures never cross /// GPUs. `None` when not reported (or off Windows, where it's unused). pub adapter_luid: Option<[u8; 8]>, + /// The device's shared queue lock (see [`QueueLock`]). The presenter holds it around + /// its own submits/presents; the decoder wires it into FFmpeg's + /// `lock_queue`/`unlock_queue` callbacks so both sides serialize on the same queues. + pub queue_lock: std::sync::Arc, } /// `fourcc(a,b,c,d)` — the DRM FourCC packing (little-endian, `a | b<<8 | c<<16 | d<<24`). @@ -934,6 +1011,36 @@ struct VkCtxStorage { f11: pf_ffvk::VkPhysicalDeviceVulkan11Features, f12: pf_ffvk::VkPhysicalDeviceVulkan12Features, f13: pf_ffvk::VkPhysicalDeviceVulkan13Features, + /// Keeps the shared queue lock alive for `AVHWDeviceContext.user_opaque` — the + /// `lock_queue`/`unlock_queue` trampolines below dereference it for as long as the + /// hw device context can fire them. + _queue_lock: std::sync::Arc, +} + +/// FFmpeg `AVVulkanDeviceContext.lock_queue` trampoline: take the device's shared +/// [`QueueLock`] (stashed in `AVHWDeviceContext.user_opaque`; owned by +/// [`VkCtxStorage`], which outlives the context). Replaces FFmpeg's internal default, +/// which only serializes FFmpeg against itself — the presenter submits to the same +/// graphics queue from another thread and holds this same lock around its calls. +unsafe extern "C" fn ffvk_lock_queue( + ctx: *mut pf_ffvk::AVHWDeviceContext, + _queue_family: u32, + _index: u32, +) { + let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext; + let lock = (*dev).user_opaque as *const QueueLock; + (*lock).lock(); +} + +/// The matching `unlock_queue` trampoline — see [`ffvk_lock_queue`]. +unsafe extern "C" fn ffvk_unlock_queue( + ctx: *mut pf_ffvk::AVHWDeviceContext, + _queue_family: u32, + _index: u32, +) { + let dev = ctx as *mut ffmpeg::ffi::AVHWDeviceContext; + let lock = (*dev).user_opaque as *const QueueLock; + (*lock).unlock(); } impl VulkanDecoder { @@ -957,6 +1064,7 @@ impl VulkanDecoder { f11: std::mem::zeroed(), f12: std::mem::zeroed(), f13: std::mem::zeroed(), + _queue_lock: vk.queue_lock.clone(), }); store.inst_ptrs = store._inst.iter().map(|c| c.as_ptr()).collect(); store.dev_ptrs = store._dev.iter().map(|c| c.as_ptr()).collect(); @@ -1030,6 +1138,16 @@ impl VulkanDecoder { (*hwctx).nb_qf = 2; } + // Shared-queue external sync (see [`QueueLock`]): FFmpeg must take the + // same lock the presenter holds around its own submits/presents — set + // BEFORE init so FFmpeg never installs its internal defaults (which only + // serialize FFmpeg against itself; the cross-thread race with the + // presenter's queue was an intermittent VK_ERROR_DEVICE_LOST). + (*devctx).user_opaque = + std::sync::Arc::as_ptr(&store._queue_lock) as *mut std::ffi::c_void; + (*hwctx).lock_queue = Some(ffvk_lock_queue); + (*hwctx).unlock_queue = Some(ffvk_unlock_queue); + let r = ffi::av_hwdevice_ctx_init(hw_device); if r < 0 { ffi::av_buffer_unref(&mut hw_device); diff --git a/crates/pf-console-ui/src/skia_overlay.rs b/crates/pf-console-ui/src/skia_overlay.rs index beb41787..d2cd942a 100644 --- a/crates/pf-console-ui/src/skia_overlay.rs +++ b/crates/pf-console-ui/src/skia_overlay.rs @@ -91,6 +91,10 @@ struct Gpu { device: ash::Device, queue_family_index: u32, context: DirectContext, + /// The device's shared queue lock (see `SharedDevice::queue_lock`): Skia submits + /// on the presenter's graphics queue, which FFmpeg's decode prep also uses from + /// the pump thread — every `flush*`/`submit` below holds this. + queue_lock: std::sync::Arc, // Keep the loader library + instance dispatch alive as long as the DirectContext // (its baked fn pointers live inside libvulkan). _entry: ash::Entry, @@ -157,6 +161,7 @@ impl Drop for SkiaOverlay { unsafe { gpu.device.destroy_image_view(slot.view, None) }; drop(slot.surface); } + let _q = gpu.queue_lock.guard(); // queue external sync vs FFmpeg's pump gpu.context.flush_and_submit(); } } @@ -215,6 +220,7 @@ impl Overlay for SkiaOverlay { device: shared.device.clone(), queue_family_index: shared.queue_family_index, context, + queue_lock: shared.queue_lock.clone(), _entry: shared.entry.clone(), _instance: shared.instance.clone(), }); @@ -310,15 +316,19 @@ impl Overlay for SkiaOverlay { ctx.pad_pref, ctx.pads, ); - gpu.context.flush_surface_with_texture_state( - &mut slot.surface, - &gpu::FlushInfo::default(), - Some(&skvk::mutable_texture_states::new_vulkan( - skvk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, - gpu.queue_family_index, - )), - ); - gpu.context.submit(None); + { + // Queue external sync vs FFmpeg's pump-thread submits (same queue). + let _q = gpu.queue_lock.guard(); + gpu.context.flush_surface_with_texture_state( + &mut slot.surface, + &gpu::FlushInfo::default(), + Some(&skvk::mutable_texture_states::new_vulkan( + skvk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + gpu.queue_family_index, + )), + ); + gpu.context.submit(None); + } self.current = next; self.drawn = Drawn::default(); // stream chrome re-renders when it returns let slot = self.slots[next].as_ref().expect("just rendered"); @@ -386,15 +396,19 @@ impl Overlay for SkiaOverlay { // Flush on the shared queue, ending in SHADER_READ_ONLY on our family — the // layout the presenter's composite samples (its own barrier covers visibility). - gpu.context.flush_surface_with_texture_state( - &mut slot.surface, - &gpu::FlushInfo::default(), - Some(&skvk::mutable_texture_states::new_vulkan( - skvk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, - gpu.queue_family_index, - )), - ); - gpu.context.submit(None); + // Lock: queue external sync vs FFmpeg's pump-thread submits (same queue). + { + let _q = gpu.queue_lock.guard(); + gpu.context.flush_surface_with_texture_state( + &mut slot.surface, + &gpu::FlushInfo::default(), + Some(&skvk::mutable_texture_states::new_vulkan( + skvk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + gpu.queue_family_index, + )), + ); + gpu.context.submit(None); + } self.current = next; self.drawn = want; diff --git a/crates/pf-presenter/src/overlay.rs b/crates/pf-presenter/src/overlay.rs index 3d357153..31e64bbe 100644 --- a/crates/pf-presenter/src/overlay.rs +++ b/crates/pf-presenter/src/overlay.rs @@ -20,6 +20,10 @@ pub struct SharedDevice { pub device: ash::Device, pub queue: vk::Queue, pub queue_family_index: u32, + /// External-sync lock for `queue` — FFmpeg's decode prep submits to the same queue + /// from the pump thread, so every overlay flush/submit must hold it (the presenter + /// and FFmpeg's `lock_queue` callbacks serialize on this same lock). + pub queue_lock: std::sync::Arc, } /// What the overlay may draw this frame — composed by the run loop from session state. diff --git a/crates/pf-presenter/src/run.rs b/crates/pf-presenter/src/run.rs index fc1de433..df44e174 100644 --- a/crates/pf-presenter/src/run.rs +++ b/crates/pf-presenter/src/run.rs @@ -235,6 +235,18 @@ impl StreamState { } } +/// Whether a present error is `VK_ERROR_DEVICE_LOST` anywhere in its chain. A lost +/// device is unrecoverable by spec — every object on it (decoder frames, swapchain, +/// the Skia context) is dead, and the demote-to-software path would rebuild the +/// decoder against that same dead device (observed live 2026-07-09: FFmpeg wedges +/// inside the rebuild, the decode thread never returns, and the client zombies with +/// the pump flushing a never-draining backlog every 2 s). The only correct response +/// is to fail the session loudly and let the shell relaunch. +fn device_lost(e: &anyhow::Error) -> bool { + e.chain() + .any(|c| c.downcast_ref::() == Some(&ash::vk::Result::ERROR_DEVICE_LOST)) +} + fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result> { // Before any window exists: unpackaged runs adopt the shell's AppUserModelID so the // shell⇄session windows group as one taskbar app (win32.rs; MSIX identity wins). @@ -789,8 +801,13 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result // Import/CSC failure is survivable (the stream continues on // the next frame) — but a streak means this box can't do the // hw path: demote the decoder to software, same contract as - // the GTK presenter's GL-converter failures. + // the GTK presenter's GL-converter failures. A lost DEVICE + // is not survivable and must not demote — see [`device_lost`]. Err(e) => { + if device_lost(&e) { + return Err(e) + .context("GPU device lost — the session cannot continue"); + } st.hw_fails += 1; tracing::warn!(error = %format!("{e:#}"), fails = st.hw_fails, "hardware present failed"); @@ -832,6 +849,11 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result p } Err(e) => { + // Lost device ⇒ unrecoverable, never demote ([`device_lost`]). + if device_lost(&e) { + return Err(e) + .context("GPU device lost — the session cannot continue"); + } st.hw_fails += 1; tracing::warn!(error = %format!("{e:#}"), fails = st.hw_fails, "hardware present failed"); @@ -873,6 +895,11 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result p } Err(e) => { + // Lost device ⇒ unrecoverable, never demote ([`device_lost`]). + if device_lost(&e) { + return Err(e) + .context("GPU device lost — the session cannot continue"); + } st.hw_fails += 1; tracing::warn!(error = %format!("{e:#}"), fails = st.hw_fails, "vulkan-video present failed"); diff --git a/crates/pf-presenter/src/vk.rs b/crates/pf-presenter/src/vk.rs index 675b09d0..c7ce2281 100644 --- a/crates/pf-presenter/src/vk.rs +++ b/crates/pf-presenter/src/vk.rs @@ -88,37 +88,6 @@ 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, - overlay_views: Vec, - overlay_framebuffers: Vec, -} - -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 @@ -360,9 +329,13 @@ 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, - /// Replaced swapchains (resize), destroyed once a present on the successor has - /// gone through a fence cycle. - retired_display: Vec, + /// External-sync lock over this device's queues, shared with FFmpeg (via + /// [`pf_client_core::video::VulkanDecodeDevice::queue_lock`] → its + /// `lock_queue`/`unlock_queue` callbacks) and the Skia overlay: FFmpeg preps on the + /// SAME graphics queue from the pump thread, so every `vkQueueSubmit`/ + /// `vkQueuePresentKHR`/`vkQueueWaitIdle`/`vkDeviceWaitIdle` here must hold it — + /// the unsynchronized overlap was an intermittent `VK_ERROR_DEVICE_LOST`. + queue_lock: std::sync::Arc, format: vk::SurfaceFormatKHR, /// The surface's HDR10/ST.2084 pairing, when the stack offers one. hdr10_format: Option, @@ -650,6 +623,9 @@ impl Presenter { // consumer needs it; `video_decode`/`d3d11_import` tell the decoder chain which // paths are real. Extension lists must mirror creation exactly — FFmpeg keys its // code paths off the strings. + // One lock per device for queue external sync (FFmpeg + Skia + this presenter + // all funnel their queue calls through it — see the `queue_lock` field docs). + let queue_lock = std::sync::Arc::new(pf_client_core::video::QueueLock::new()); #[cfg(windows)] let export_worthy = video_ok || win_capable; #[cfg(not(windows))] @@ -698,6 +674,7 @@ impl Presenter { #[cfg(not(windows))] d3d11_import: false, adapter_luid, + queue_lock: queue_lock.clone(), }) } else { None @@ -758,7 +735,7 @@ impl Presenter { video_export, overlay_pipe, retired_hw: None, - retired_display: Vec::new(), + queue_lock, format, hdr10_format, hdr_active: false, @@ -799,6 +776,21 @@ impl Presenter { /// (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<()> { self.quiesce_own()?; + // Drain the queue before touching presentation objects: after this, every prior + // present's semaphore-wait operation has completed, so the OLD swapchain and its + // render semaphores are safe to destroy immediately below. (The previous scheme + // parked them and destroyed after one fence cycle — but the fence proves only + // OUR submit, not the presentation engine's semaphore consumption: + // VUID-vkDestroySemaphore-05149 / VUID-vkDestroySwapchainKHR-01282 on every + // recreate, and destroy-in-use is exactly the kind of misuse that turns into an + // intermittent VK_ERROR_DEVICE_LOST.) Safe against the pump's FFmpeg submits — + // both sides hold the shared queue lock — and cheap: a recreate already stalls + // the stream for a frame, and only happens on resize/HDR-flip/OUT_OF_DATE. + { + let _q = self.queue_lock.guard(); + unsafe { self.device.queue_wait_idle(self.queue) } + .context("vkQueueWaitIdle (swapchain recreate)")?; + } let caps = unsafe { self.surface_i @@ -842,17 +834,24 @@ impl Presenter { .old_swapchain(old); let swapchain = unsafe { self.swap_d.create_swapchain(&info, None) }.context("vkCreateSwapchainKHR")?; - // The old swapchain (and everything tied to its images) is parked, not - // 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. + // The old swapchain and everything tied to its images dies NOW: the fence + // quiesce covered our own command buffers, the queue drain above covered the + // presentation engine's semaphore waits — nothing can still reference them. 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, - }); + unsafe { + for fb in overlay_framebuffers { + self.device.destroy_framebuffer(fb, None); + } + for v in overlay_views { + self.device.destroy_image_view(v, None); + } + for s in self.render_sems.drain(..) { + self.device.destroy_semaphore(s, None); + } + if old != vk::SwapchainKHR::null() { + self.swap_d.destroy_swapchain(old, None); + } + } self.swapchain = swapchain; self.images = unsafe { self.swap_d.get_swapchain_images(swapchain) }?; self.extent = extent; @@ -967,7 +966,9 @@ impl Presenter { /// Full device idle — TEARDOWN ONLY, and only after the session pump thread has /// 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. + /// The queue lock is held as cheap insurance against a straggling submitter. pub fn wait_idle(&self) { + let _q = self.queue_lock.guard(); unsafe { self.device.device_wait_idle() }.ok(); } @@ -981,6 +982,7 @@ impl Presenter { device: self.device.clone(), queue: self.queue, queue_family_index: self.qfi, + queue_lock: self.queue_lock.clone(), } } @@ -1013,19 +1015,23 @@ impl Presenter { self.device.free_memory(v.memory, None); } } - // New overlay pipe against the new swapchain format; the old one's targets are - // parked (empty sems/swapchain — those ride the recreate below). + // New overlay pipe against the new swapchain format. The old one's targets + // (views/framebuffers over the current swapchain's images) are only ever + // referenced by our own command buffers — the fence quiesce above makes them + // safe to destroy right here; the swapchain itself rides the recreate below. let mut old_pipe = std::mem::replace( &mut self.overlay_pipe, OverlayPipe::new(&self.device, target.format)?, ); let (overlay_views, overlay_framebuffers) = old_pipe.take_targets(); - self.retired_display.push(DisplayGarbage { - swapchain: vk::SwapchainKHR::null(), - render_sems: Vec::new(), - overlay_views, - overlay_framebuffers, - }); + unsafe { + for fb in overlay_framebuffers { + self.device.destroy_framebuffer(fb, None); + } + for v in overlay_views { + self.device.destroy_image_view(v, None); + } + } old_pipe.destroy(&self.device); self.format = target; self.hdr_active = on; @@ -1113,11 +1119,6 @@ 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)?; @@ -1505,7 +1506,11 @@ impl Presenter { submit = submit.push_next(&mut keyed_info); } } - let submitted = self.device.queue_submit(self.queue, &[submit], self.fence); + let submitted = { + // Queue external sync vs the pump's FFmpeg submits (see `queue_lock`). + let _q = self.queue_lock.guard(); + self.device.queue_submit(self.queue, &[submit], self.fence) + }; // Write the new sync state back and release the frames lock REGARDLESS of // the submit outcome (an abandoned lock would wedge the decoder). if let Some(sync) = vk_sync.take() { @@ -1540,13 +1545,19 @@ impl Presenter { let swapchains = [self.swapchain]; let indices = [index]; let present_sems = [render_sem]; - match self.swap_d.queue_present( - self.queue, - &vk::PresentInfoKHR::default() - .wait_semaphores(&present_sems) - .swapchains(&swapchains) - .image_indices(&indices), - ) { + // Same queue external-sync rule as the submit above. Scoped tightly: the + // OUT_OF_DATE arm re-enters the lock via recreate_swapchain's queue drain. + let present_res = { + let _q = self.queue_lock.guard(); + self.swap_d.queue_present( + self.queue, + &vk::PresentInfoKHR::default() + .wait_semaphores(&present_sems) + .swapchains(&swapchains) + .image_indices(&indices), + ) + }; + match present_res { Ok(_) => Ok(true), Err(vk::Result::ERROR_OUT_OF_DATE_KHR) => { self.recreate_swapchain(window)?; @@ -1867,7 +1878,12 @@ impl Presenter { impl Drop for Presenter { fn drop(&mut self) { unsafe { - self.device.device_wait_idle().ok(); + { + // Insurance against a straggling submitter (the run loop joins the + // pump before dropping us, so this is normally uncontended). + let _q = self.queue_lock.guard(); + self.device.device_wait_idle().ok(); + } if let Some(f) = self.retired_hw.take() { f.destroy(&self.device); // idle above — the GPU reads are done } @@ -1886,10 +1902,6 @@ 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); - } #[cfg(target_os = "linux")] self.hw.take(); self.csc.destroy(&self.device);