From 3c59cf022a0fa1c3a35c295ae3e5e40ca04e3faf Mon Sep 17 00:00:00 2001 From: janis Date: Sat, 4 Apr 2026 00:43:50 +0200 Subject: [PATCH] fix swapchain/surface drop logic error --- crates/renderer/src/images.rs | 11 +++++---- crates/renderer/src/swapchain.rs | 38 +++++++++++++++++++++++--------- crates/renderer/src/util.rs | 18 +++++++++++++++ 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/crates/renderer/src/images.rs b/crates/renderer/src/images.rs index 41caff1..1ac2455 100644 --- a/crates/renderer/src/images.rs +++ b/crates/renderer/src/images.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, sync::Arc}; +use std::{borrow::Cow, mem::ManuallyDrop, sync::Arc}; use crate::{ device::{Allocation, AllocationStrategy, DeviceHandle, DeviceObject, QueueFlags}, @@ -408,7 +408,10 @@ impl Image { // } // } - pub fn create_view(self: &Arc, mut desc: ImageViewDesc) -> crate::Result { + pub fn create_view( + self: &Arc, + mut desc: ImageViewDesc, + ) -> crate::Result> { // validate if !view_kind_compatible(self.desc.kind, desc.kind) { tracing::error!( @@ -471,11 +474,11 @@ impl Image { let device = self.image.device(); let view = unsafe { device.raw.create_image_view(&create_info, None)? }; - Ok(ImageView { + Ok(ManuallyDrop::new(ImageView { view: DeviceObject::new(view, device.clone(), desc.name.clone()), desc, image: self.clone(), - }) + })) } } diff --git a/crates/renderer/src/swapchain.rs b/crates/renderer/src/swapchain.rs index 2bd5ef5..87ec5a8 100644 --- a/crates/renderer/src/swapchain.rs +++ b/crates/renderer/src/swapchain.rs @@ -1,5 +1,6 @@ use std::{ collections::HashMap, + mem::ManuallyDrop, num::NonZero, ops::Deref, sync::{ @@ -21,6 +22,7 @@ use crate::{ device::{Device, DeviceObject}, images::{self, ImageViewDesc}, sync::Fence, + util::DropGuard, }; use derive_more::Debug; @@ -30,16 +32,12 @@ pub struct Surface { pub(crate) raw: vk::SurfaceKHR, #[debug(skip)] pub(crate) functor: khr::surface::Instance, - pub(crate) instance: Instance, pub(crate) swapchain: RwLock>>, -} -impl Drop for Surface { - fn drop(&mut self) { - unsafe { - self.functor.destroy_surface(self.raw, None); - } - } + // destroy surface after any fields that depend on it + _drop_guard: DropGuard, + // drop reference to instance after destroying the surface + pub(crate) instance: Instance, } impl Surface { @@ -82,9 +80,18 @@ impl Surface { Ok(Self { raw, - functor, swapchain: RwLock::new(None), instance: instance.clone(), + + // the surface must be destroyed after the swapchain + _drop_guard: DropGuard::new({ + let functor = functor.clone(); + move || { + functor.destroy_surface(raw, None); + } + }), + + functor, }) } } @@ -117,9 +124,18 @@ impl Surface { Ok(Self { raw, - functor, swapchain: RwLock::new(None), instance: instance.clone(), + + // the surface must be destroyed after the swapchain + _drop_guard: DropGuard::new({ + let functor = functor.clone(); + move || unsafe { + functor.destroy_surface(raw, None); + } + }), + + functor, }) } @@ -571,7 +587,7 @@ impl Swapchain { SwapchainImage { index: idx as u32, swapchain: self, - view, + view: ManuallyDrop::into_inner(view), acquire, release, }, diff --git a/crates/renderer/src/util.rs b/crates/renderer/src/util.rs index 64e77ca..e4b23f8 100644 --- a/crates/renderer/src/util.rs +++ b/crates/renderer/src/util.rs @@ -1,5 +1,6 @@ use std::{ borrow::Cow, + mem::ManuallyDrop, ops::{Deref, DerefMut}, }; @@ -76,6 +77,23 @@ pub(crate) mod weak_vec { } } +#[derive(derive_more::Debug)] +pub(crate) struct DropGuard(#[debug(skip)] ManuallyDrop>); + +impl DropGuard { + pub(crate) fn new(f: impl FnOnce() + 'static) -> Self { + Self(ManuallyDrop::new(Box::new(f))) + } +} + +impl Drop for DropGuard { + fn drop(&mut self) { + unsafe { + ManuallyDrop::take(&mut self.0)(); + } + } +} + #[derive(Clone, PartialEq, Eq, Hash)] pub struct DebugName { #[cfg(debug_assertions)]