From 6cd23bf02a003a576705b3af4eecb7fb92dfb3c6 Mon Sep 17 00:00:00 2001 From: Valentin Popov Date: Thu, 25 Jun 2026 22:35:08 +0400 Subject: fix(smoke): capture final validation after explicit shutdown --- adapters/fparkan-render-vulkan/src/ffi.rs | 2 +- adapters/fparkan-render-vulkan/src/ffi/smoke.rs | 128 +++++++++++++++++++-- .../fparkan-render-vulkan/src/ffi/smoke_types.rs | 13 ++- apps/fparkan-vulkan-smoke/src/main.rs | 114 ++++++++++++++---- 4 files changed, 220 insertions(+), 37 deletions(-) diff --git a/adapters/fparkan-render-vulkan/src/ffi.rs b/adapters/fparkan-render-vulkan/src/ffi.rs index ad65e81..0406544 100644 --- a/adapters/fparkan-render-vulkan/src/ffi.rs +++ b/adapters/fparkan-render-vulkan/src/ffi.rs @@ -61,7 +61,7 @@ pub use self::runtime::{ pub use self::smoke_types::{ VulkanSmokeBootstrapProgress, VulkanSmokeBootstrapSnapshot, VulkanSmokeFrameOutcome, VulkanSmokeRenderer, VulkanSmokeRendererCreateInfo, VulkanSmokeRendererError, - VulkanSmokeRendererReport, VulkanValidationReport, + VulkanSmokeRendererReport, VulkanSmokeShutdownReport, VulkanValidationReport, }; #[cfg(test)] use self::surface::extension_name; diff --git a/adapters/fparkan-render-vulkan/src/ffi/smoke.rs b/adapters/fparkan-render-vulkan/src/ffi/smoke.rs index c8bf792..d42970e 100644 --- a/adapters/fparkan-render-vulkan/src/ffi/smoke.rs +++ b/adapters/fparkan-render-vulkan/src/ffi/smoke.rs @@ -10,8 +10,9 @@ use super::{ destroy_allocated_buffer, destroy_swapchain_resources, plan_vulkan_surface, VulkanAllocatedBuffer, VulkanInstanceConfig, VulkanInstanceProbe, VulkanLogicalDeviceProbe, VulkanSmokeFrameOutcome, VulkanSmokeRenderer, VulkanSmokeRendererCreateInfo, - VulkanSmokeRendererError, VulkanSmokeRendererReport, VulkanSurfaceProbe, VulkanSwapchainProbe, - VulkanSwapchainResources, VulkanValidationMessenger, VulkanValidationReport, + VulkanSmokeRendererError, VulkanSmokeRendererReport, VulkanSmokeShutdownReport, + VulkanSurfaceProbe, VulkanSwapchainProbe, VulkanSwapchainResources, VulkanValidationMessenger, + VulkanValidationReport, }; use crate::policy::KHR_PORTABILITY_SUBSET_EXTENSION; use crate::shader_manifest::{triangle_shader_manifest, validate_shader_manifest}; @@ -30,6 +31,30 @@ fn take_runtime_owners_in_dependency_order( + instance: &mut Option, + validation: &mut Option, + surface: &mut Option, + device: &mut Option, + swapchain: &mut Option, + capture: Capture, +) -> Option +where + Capture: FnOnce(&Validation) -> Snapshot, +{ + let snapshot = validation.as_ref().map(capture); + take_runtime_owners_in_dependency_order(instance, validation, surface, device, swapchain); + snapshot +} + struct RollbackOnDrop where F: FnOnce(T), @@ -226,6 +251,16 @@ impl VulkanSmokeRenderer { self.swapchain_recreate_count } + /// Explicitly idles and tears down the renderer while the native window is still alive. + /// + /// # Errors + /// + /// Returns [`VulkanSmokeRendererError`] when the renderer cannot reach a + /// stable idle point before teardown begins. + pub fn shutdown(mut self) -> Result { + self.shutdown_inner() + } + /// Requests swapchain recreation for a new drawable extent. pub fn request_resize(&mut self, extent: (u32, u32)) { self.pending_extent = Some(extent); @@ -595,11 +630,7 @@ impl VulkanSmokeRenderer { self.current_frame = 0; } - fn teardown(&mut self) { - if let Some(device) = self.device.as_ref() { - // SAFETY: The logical device remains live until teardown finishes and idling prevents in-flight work from touching swapchain, buffers, sync objects or the command pool after destruction starts. - let _ = unsafe { device.device().device_wait_idle() }; - } + fn destroy_device_owned_resources(&mut self) { self.destroy_swapchain_resources(); if let Some(device) = self.device.as_ref() { if let Some(buffer) = self.index_buffer.take() { @@ -623,13 +654,49 @@ impl VulkanSmokeRenderer { .destroy_command_pool(self.command_pool, None); }; } - // Drop child Vulkan owners explicitly before their parents instead of relying on field order. - take_runtime_owners_in_dependency_order( + self.pending_extent = None; + } + + fn shutdown_inner(&mut self) -> Result { + if let Some(device) = self.device.as_ref() { + // SAFETY: The logical device remains live until teardown finishes and idling prevents in-flight work from touching swapchain, buffers, sync objects or the command pool after destruction starts. + unsafe { device.device().device_wait_idle() }.map_err(|error| { + VulkanSmokeRendererError::VulkanOperation { + context: "vkDeviceWaitIdle", + result: error, + } + })?; + } + self.destroy_device_owned_resources(); + let validation = take_runtime_owners_with_validation_snapshot( + &mut self.instance, + &mut self.validation, + &mut self.surface, + &mut self.device, + &mut self.swapchain, + VulkanValidationMessenger::report, + ) + .unwrap_or_default(); + Ok(VulkanSmokeShutdownReport { + renderer_report: self.report.clone(), + swapchain_recreate_count: self.swapchain_recreate_count, + validation, + }) + } + + fn teardown(&mut self) { + if let Some(device) = self.device.as_ref() { + // SAFETY: The logical device remains live until teardown finishes and idling prevents in-flight work from touching swapchain, buffers, sync objects or the command pool after destruction starts. + let _ = unsafe { device.device().device_wait_idle() }; + } + self.destroy_device_owned_resources(); + let _ = take_runtime_owners_with_validation_snapshot( &mut self.instance, &mut self.validation, &mut self.surface, &mut self.device, &mut self.swapchain, + VulkanValidationMessenger::report, ); } } @@ -642,12 +709,16 @@ impl Drop for VulkanSmokeRenderer { #[cfg(test)] mod tests { - use super::{take_runtime_owners_in_dependency_order, RollbackOnDrop}; + use super::{ + take_runtime_owners_in_dependency_order, take_runtime_owners_with_validation_snapshot, + RollbackOnDrop, + }; use std::cell::RefCell; use std::rc::Rc; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum TeardownStep { + Snapshot, Instance, Validation, Surface, @@ -778,6 +849,43 @@ mod tests { } } + #[test] + fn final_validation_snapshot_is_captured_before_validation_drop() { + let log = Rc::new(RefCell::new(Vec::new())); + let mut instance = Some(tracker(TeardownStep::Instance, &log)); + let mut validation = Some(tracker(TeardownStep::Validation, &log)); + let mut surface = Some(tracker(TeardownStep::Surface, &log)); + let mut device = Some(tracker(TeardownStep::Device, &log)); + let mut swapchain = Some(tracker(TeardownStep::Swapchain, &log)); + + let snapshot = take_runtime_owners_with_validation_snapshot( + &mut instance, + &mut validation, + &mut surface, + &mut device, + &mut swapchain, + |_| { + log.borrow_mut().push(TeardownStep::Snapshot); + TeardownStep::Validation + }, + ); + + assert_eq!(snapshot, Some(TeardownStep::Validation)); + assert_eq!( + Rc::into_inner(log) + .expect("all drop trackers released") + .into_inner(), + vec![ + TeardownStep::Snapshot, + TeardownStep::Swapchain, + TeardownStep::Device, + TeardownStep::Surface, + TeardownStep::Validation, + TeardownStep::Instance, + ] + ); + } + #[test] fn rollback_guard_runs_cleanup_when_later_step_fails() { let log = Rc::new(RefCell::new(Vec::new())); diff --git a/adapters/fparkan-render-vulkan/src/ffi/smoke_types.rs b/adapters/fparkan-render-vulkan/src/ffi/smoke_types.rs index ea4a471..a86de1b 100644 --- a/adapters/fparkan-render-vulkan/src/ffi/smoke_types.rs +++ b/adapters/fparkan-render-vulkan/src/ffi/smoke_types.rs @@ -128,7 +128,7 @@ pub struct VulkanSmokeRendererReport { } /// Measured validation counters from the live smoke loop. -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct VulkanValidationReport { /// Validation warnings observed by the debug messenger. pub warning_count: u32, @@ -138,6 +138,17 @@ pub struct VulkanValidationReport { pub vuids: Vec, } +/// Final smoke renderer shutdown evidence captured after explicit teardown. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct VulkanSmokeShutdownReport { + /// Stable renderer bootstrap and swapchain report. + pub renderer_report: VulkanSmokeRendererReport, + /// Measured swapchain recreation count for the completed smoke loop. + pub swapchain_recreate_count: u32, + /// Final validation snapshot captured before the debug messenger is destroyed. + pub validation: VulkanValidationReport, +} + /// Result of one rendered smoke frame. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum VulkanSmokeFrameOutcome { diff --git a/apps/fparkan-vulkan-smoke/src/main.rs b/apps/fparkan-vulkan-smoke/src/main.rs index 8e12ac5..814d503 100644 --- a/apps/fparkan-vulkan-smoke/src/main.rs +++ b/apps/fparkan-vulkan-smoke/src/main.rs @@ -15,7 +15,8 @@ use fparkan_platform::RenderRequest; use fparkan_platform_winit::{window_native_handles, WinitWindowPlan}; use fparkan_render_vulkan::{ VulkanSmokeBootstrapProgress, VulkanSmokeFrameOutcome, VulkanSmokeRenderer, - VulkanSmokeRendererCreateInfo, + VulkanSmokeRendererCreateInfo, VulkanSmokeRendererReport, VulkanSmokeShutdownReport, + VulkanValidationReport, }; use serde::Serialize; use std::path::PathBuf; @@ -176,6 +177,7 @@ struct SmokeApp { window_id: Option, window: Option, renderer: Option, + final_renderer: Option, error: Option, output: Option, frames_presented: u32, @@ -193,6 +195,23 @@ fn drop_renderer_before_window( drop(window.take()); } +#[derive(Clone, Debug)] +struct RendererSnapshot { + report: VulkanSmokeRendererReport, + swapchain_recreate_count: u32, + validation: VulkanValidationReport, +} + +impl From for RendererSnapshot { + fn from(report: VulkanSmokeShutdownReport) -> Self { + Self { + report: report.renderer_report, + swapchain_recreate_count: report.swapchain_recreate_count, + validation: report.validation, + } + } +} + impl SmokeApp { fn new( options: SmokeOptions, @@ -206,6 +225,7 @@ impl SmokeApp { window_id: None, window: None, renderer: None, + final_renderer: None, error: None, output: None, frames_presented: 0, @@ -244,20 +264,31 @@ impl SmokeApp { .map_err(|err| format!("{}: {err}", self.options.out.display())) } + fn live_renderer_snapshot(&self) -> Option { + self.renderer.as_ref().map(|renderer| RendererSnapshot { + report: renderer.report().clone(), + swapchain_recreate_count: renderer.swapchain_recreate_count(), + validation: renderer.validation_report(), + }) + } + + fn renderer_snapshot(&self) -> Option { + self.final_renderer + .clone() + .or_else(|| self.live_renderer_snapshot()) + } + fn render_report( &self, status: &'static str, failure_reason: Option<&str>, ) -> Result { - let renderer = self.renderer.as_ref(); - let validation = renderer.map_or( - fparkan_render_vulkan::VulkanValidationReport { - warning_count: 0, - error_count: 0, - vuids: Vec::new(), - }, - VulkanSmokeRenderer::validation_report, - ); + let renderer = self.renderer_snapshot(); + let validation = renderer + .as_ref() + .map_or_else(VulkanValidationReport::default, |snapshot| { + snapshot.validation.clone() + }); let smoke_report = SmokeReport { schema_version: SCHEMA_VERSION, commit_sha: compiled_commit_sha(), @@ -272,14 +303,16 @@ impl SmokeApp { frames: self.frames_presented, resize_count: self.resize_count, swapchain_recreate_count: renderer - .map_or(0, VulkanSmokeRenderer::swapchain_recreate_count), + .as_ref() + .map_or(0, |snapshot| snapshot.swapchain_recreate_count), validation_warning_count: validation.warning_count, validation_error_count: validation.error_count, validation_vuids: &validation.vuids, requested_frames: self.options.frames, timeout_seconds: self.options.timeout_seconds, shader_manifest_hash: renderer - .map_or("", |value| value.report().shader_manifest_hash.as_str()), + .as_ref() + .map_or("", |snapshot| snapshot.report.shader_manifest_hash.as_str()), vulkan_loader_status: if renderer.is_some() { "available" } else { @@ -305,31 +338,43 @@ impl SmokeApp { } else { "failed" }, - vulkan_device_name: renderer.map_or("", |value| value.report().device_name.as_str()), + vulkan_device_name: renderer + .as_ref() + .map_or("", |snapshot| snapshot.report.device_name.as_str()), vulkan_logical_device_status: if renderer.is_some() { "created" } else { "failed" }, vulkan_logical_device_graphics_queue_family: renderer - .map_or(0, |value| value.report().graphics_queue_family), + .as_ref() + .map_or(0, |snapshot| snapshot.report.graphics_queue_family), vulkan_logical_device_present_queue_family: renderer - .map_or(0, |value| value.report().present_queue_family), + .as_ref() + .map_or(0, |snapshot| snapshot.report.present_queue_family), vulkan_logical_device_enabled_extension_count: renderer - .map_or(0, |value| value.report().enabled_extension_count), + .as_ref() + .map_or(0, |snapshot| snapshot.report.enabled_extension_count), vulkan_swapchain_status: if renderer.is_some() { "created" } else { "failed" }, - vulkan_swapchain_width: renderer.map_or(0, |value| value.report().swapchain_extent.0), - vulkan_swapchain_height: renderer.map_or(0, |value| value.report().swapchain_extent.1), + vulkan_swapchain_width: renderer + .as_ref() + .map_or(0, |snapshot| snapshot.report.swapchain_extent.0), + vulkan_swapchain_height: renderer + .as_ref() + .map_or(0, |snapshot| snapshot.report.swapchain_extent.1), vulkan_swapchain_image_count: renderer - .map_or(0, |value| value.report().swapchain_image_count), + .as_ref() + .map_or(0, |snapshot| snapshot.report.swapchain_image_count), vulkan_portability_enumeration: renderer - .is_some_and(|value| value.report().portability_enumeration), + .as_ref() + .is_some_and(|snapshot| snapshot.report.portability_enumeration), vulkan_portability_subset_enabled: renderer - .is_some_and(|value| value.report().portability_subset_enabled), + .as_ref() + .is_some_and(|snapshot| snapshot.report.portability_subset_enabled), }; serde_json::to_string_pretty(&smoke_report) .map(|json| format!("{json}\n")) @@ -362,7 +407,6 @@ impl SmokeApp { event_loop.exit(); return; }; - let validation = renderer.validation_report(); if self.frames_presented < self.options.frames { self.error = Some("native smoke did not reach the required frame count".to_string()); event_loop.exit(); @@ -376,15 +420,34 @@ impl SmokeApp { event_loop.exit(); return; } - if validation.warning_count != 0 || validation.error_count != 0 { + let renderer = match self.renderer.take() { + Some(renderer) => renderer, + None => { + self.error = Some("native smoke renderer was not initialized".to_string()); + event_loop.exit(); + return; + } + }; + let final_renderer = match renderer.shutdown() { + Ok(report) => RendererSnapshot::from(report), + Err(err) => { + self.error = Some(err.to_string()); + event_loop.exit(); + return; + } + }; + if final_renderer.validation.warning_count != 0 + || final_renderer.validation.error_count != 0 + { + self.final_renderer = Some(final_renderer.clone()); self.error = Some(format!( "native smoke validation must stay clean (warnings={}, errors={})", - validation.warning_count, validation.error_count + final_renderer.validation.warning_count, final_renderer.validation.error_count )); event_loop.exit(); return; } - let _ = renderer; + self.final_renderer = Some(final_renderer); let report = match self.render_report("passed", None) { Ok(report) => report, Err(err) => { @@ -1077,6 +1140,7 @@ mod tests { window_id: None, window: None, renderer: None, + final_renderer: None, error: Some("native smoke timed out after 7 seconds".to_string()), output: None, frames_presented: 42, -- cgit v1.2.3