remove hal::Device::exit, add Drop implementations to Device and Queue instead

Author: teoxoy

wgpu-core/src/device/queue.rs

@@ -41,7 +41,7 @@ use thiserror::Error;
 use super::{life::LifetimeTracker, Device};
 
 pub struct Queue {
-    raw: ManuallyDrop<Box<dyn hal::DynQueue>>,
+    raw: Box<dyn hal::DynQueue>,
     pub(crate) device: Arc<Device>,
     pub(crate) pending_writes: Mutex<ManuallyDrop<PendingWrites>>,
     life_tracker: Mutex<LifetimeTracker>,
@@ -60,7 +60,6 @@ impl Queue {
         let pending_encoder = match pending_encoder {
             Ok(pending_encoder) => pending_encoder,
             Err(e) => {
-                device.release_queue(raw);
                 return Err(e);
             }
         };
@@ -93,7 +92,7 @@ impl Queue {
         );
 
         Ok(Queue {
-            raw: ManuallyDrop::new(raw),
+            raw,
             device,
             pending_writes,
             life_tracker: Mutex::new(rank::QUEUE_LIFE_TRACKER, LifetimeTracker::new()),
@@ -198,9 +197,6 @@ impl Drop for Queue {
         // SAFETY: We are in the Drop impl and we don't use self.pending_writes anymore after this point.
         let pending_writes = unsafe { ManuallyDrop::take(&mut self.pending_writes.lock()) };
         pending_writes.dispose(self.device.raw());
-        // SAFETY: we never access `self.raw` beyond this point.
-        let queue = unsafe { ManuallyDrop::take(&mut self.raw) };
-        self.device.release_queue(queue);
 
         closures.fire();
     }

wgpu-core/src/device/resource.rs

@@ -57,10 +57,9 @@ use super::{
 /// Structure describing a logical device. Some members are internally mutable,
 /// stored behind mutexes.
 pub struct Device {
-    raw: ManuallyDrop<Box<dyn hal::DynDevice>>,
+    raw: Box<dyn hal::DynDevice>,
     pub(crate) adapter: Arc<Adapter>,
     pub(crate) queue: OnceLock<Weak<Queue>>,
-    queue_to_drop: OnceLock<Box<dyn hal::DynQueue>>,
     pub(crate) zero_buffer: ManuallyDrop<Box<dyn hal::DynBuffer>>,
     /// The `label` from the descriptor used to create the resource.
     label: String,
@@ -154,23 +153,19 @@ impl Drop for Device {
             closure.call(DeviceLostReason::Dropped, String::from("Device dropped."));
         }
 
-        // SAFETY: We are in the Drop impl and we don't use self.raw anymore after this point.
-        let raw = unsafe { ManuallyDrop::take(&mut self.raw) };
         // SAFETY: We are in the Drop impl and we don't use self.zero_buffer anymore after this point.
         let zero_buffer = unsafe { ManuallyDrop::take(&mut self.zero_buffer) };
         // SAFETY: We are in the Drop impl and we don't use self.fence anymore after this point.
         let fence = unsafe { ManuallyDrop::take(&mut self.fence.write()) };
-        self.command_allocator.dispose(raw.as_ref());
+        self.command_allocator.dispose(self.raw.as_ref());
         #[cfg(feature = "indirect-validation")]
         self.indirect_validation
             .take()
             .unwrap()
-            .dispose(raw.as_ref());
+            .dispose(self.raw.as_ref());
         unsafe {
-            raw.destroy_buffer(zero_buffer);
-            raw.destroy_fence(fence);
-            let queue = self.queue_to_drop.take().unwrap();
-            raw.exit(queue);
+            self.raw.destroy_buffer(zero_buffer);
+            self.raw.destroy_fence(fence);
         }
     }
 }
@@ -249,10 +244,9 @@ impl Device {
         };
 
         Ok(Self {
-            raw: ManuallyDrop::new(raw_device),
+            raw: raw_device,
             adapter: adapter.clone(),
             queue: OnceLock::new(),
-            queue_to_drop: OnceLock::new(),
             zero_buffer: ManuallyDrop::new(zero_buffer),
             label: desc.label.to_string(),
             command_allocator,
@@ -325,10 +319,6 @@ impl Device {
         DeviceError::from_hal(error)
     }
 
-    pub(crate) fn release_queue(&self, queue: Box<dyn hal::DynQueue>) {
-        assert!(self.queue_to_drop.set(queue).is_ok());
-    }
-
     /// Run some destroy operations that were deferred.
     ///
     /// Destroying the resources requires taking a write lock on the device's snatch lock,

wgpu-hal/examples/halmark/main.rs

@@ -579,7 +579,8 @@ impl<A: hal::Api> Example<A> {
             self.device.destroy_pipeline_layout(self.pipeline_layout);
 
             self.surface.unconfigure(&self.device);
-            self.device.exit(self.queue);
+            drop(self.queue);
+            drop(self.device);
             drop(self.surface);
             drop(self.adapter);
         }

wgpu-hal/examples/ray-traced-triangle/main.rs

@@ -1068,7 +1068,8 @@ impl<A: hal::Api> Example<A> {
             self.device.destroy_shader_module(self.shader_module);
 
             self.surface.unconfigure(&self.device);
-            self.device.exit(self.queue);
+            drop(self.queue);
+            drop(self.device);
             drop(self.surface);
             drop(self.adapter);
         }

wgpu-hal/src/dx12/device.rs

@@ -391,10 +391,6 @@ impl super::Device {
 impl crate::Device for super::Device {
     type A = super::Api;
 
-    unsafe fn exit(self, _queue: super::Queue) {
-        self.rtv_pool.lock().free_handle(self.null_rtv_handle);
-    }
-
     unsafe fn create_buffer(
         &self,
         desc: &crate::BufferDescriptor,

wgpu-hal/src/dx12/mod.rs

@@ -602,6 +602,12 @@ pub struct Device {
     counters: wgt::HalCounters,
 }
 
+impl Drop for Device {
+    fn drop(&mut self) {
+        self.rtv_pool.lock().free_handle(self.null_rtv_handle);
+    }
+}
+
 unsafe impl Send for Device {}
 unsafe impl Sync for Device {}
 

wgpu-hal/src/dynamic/device.rs

@@ -16,8 +16,6 @@ use super::{
 };
 
 pub trait DynDevice: DynResource {
-    unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>);
-
     unsafe fn create_buffer(
         &self,
         desc: &BufferDescriptor,
@@ -166,10 +164,6 @@ pub trait DynDevice: DynResource {
 }
 
 impl<D: Device + DynResource> DynDevice for D {
-    unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>) {
-        unsafe { D::exit(*self, queue.unbox()) }
-    }
-
     unsafe fn create_buffer(
         &self,
         desc: &BufferDescriptor,

wgpu-hal/src/empty.rs

@@ -163,7 +163,6 @@ impl crate::Queue for Context {
 impl crate::Device for Context {
     type A = Api;
 
-    unsafe fn exit(self, queue: Context) {}
     unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> {
         Ok(Resource)
     }

wgpu-hal/src/gles/device.rs

@@ -501,14 +501,6 @@ impl super::Device {
 impl crate::Device for super::Device {
     type A = super::Api;
 
-    unsafe fn exit(self, queue: super::Queue) {
-        let gl = &self.shared.context.lock();
-        unsafe { gl.delete_vertex_array(self.main_vao) };
-        unsafe { gl.delete_framebuffer(queue.draw_fbo) };
-        unsafe { gl.delete_framebuffer(queue.copy_fbo) };
-        unsafe { gl.delete_buffer(queue.zero_buffer) };
-    }
-
     unsafe fn create_buffer(
         &self,
         desc: &crate::BufferDescriptor,

wgpu-hal/src/gles/mod.rs

@@ -295,6 +295,13 @@ pub struct Device {
     counters: wgt::HalCounters,
 }
 
+impl Drop for Device {
+    fn drop(&mut self) {
+        let gl = &self.shared.context.lock();
+        unsafe { gl.delete_vertex_array(self.main_vao) };
+    }
+}
+
 pub struct ShaderClearProgram {
     pub program: glow::Program,
     pub color_uniform_location: glow::UniformLocation,
@@ -316,6 +323,15 @@ pub struct Queue {
     current_index_buffer: Mutex<Option<glow::Buffer>>,
 }
 
+impl Drop for Queue {
+    fn drop(&mut self) {
+        let gl = &self.shared.context.lock();
+        unsafe { gl.delete_framebuffer(self.draw_fbo) };
+        unsafe { gl.delete_framebuffer(self.copy_fbo) };
+        unsafe { gl.delete_buffer(self.zero_buffer) };
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct Buffer {
     raw: Option<glow::Buffer>,