fix swapchain/surface drop logic error

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<Self>, mut desc: ImageViewDesc) -> crate::Result<ImageView> {
+    pub fn create_view(
+        self: &Arc<Self>,
+        mut desc: ImageViewDesc,
+    ) -> crate::Result<ManuallyDrop<ImageView>> {
         // 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(),
-        })
+        }))
     }
 }
 

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<Option<Arc<Swapchain>>>,
-}
 
-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,
                 },

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<Box<dyn FnOnce()>>);
+
+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)]