Skip to content

Commit 851ce16

Browse files
committed
remove hal::Device::exit, add Drop implementations to Device and Queue instead
1 parent f7334ef commit 851ce16

File tree

14 files changed

+62
-72
lines changed

14 files changed

+62
-72
lines changed

wgpu-core/src/device/queue.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use thiserror::Error;
4141
use super::{life::LifetimeTracker, Device};
4242

4343
pub struct Queue {
44-
raw: ManuallyDrop<Box<dyn hal::DynQueue>>,
44+
raw: Box<dyn hal::DynQueue>,
4545
pub(crate) device: Arc<Device>,
4646
pub(crate) pending_writes: Mutex<ManuallyDrop<PendingWrites>>,
4747
life_tracker: Mutex<LifetimeTracker>,
@@ -60,7 +60,6 @@ impl Queue {
6060
let pending_encoder = match pending_encoder {
6161
Ok(pending_encoder) => pending_encoder,
6262
Err(e) => {
63-
device.release_queue(raw);
6463
return Err(e);
6564
}
6665
};
@@ -93,7 +92,7 @@ impl Queue {
9392
);
9493

9594
Ok(Queue {
96-
raw: ManuallyDrop::new(raw),
95+
raw,
9796
device,
9897
pending_writes,
9998
life_tracker: Mutex::new(rank::QUEUE_LIFE_TRACKER, LifetimeTracker::new()),
@@ -198,9 +197,6 @@ impl Drop for Queue {
198197
// SAFETY: We are in the Drop impl and we don't use self.pending_writes anymore after this point.
199198
let pending_writes = unsafe { ManuallyDrop::take(&mut self.pending_writes.lock()) };
200199
pending_writes.dispose(self.device.raw());
201-
// SAFETY: we never access `self.raw` beyond this point.
202-
let queue = unsafe { ManuallyDrop::take(&mut self.raw) };
203-
self.device.release_queue(queue);
204200

205201
closures.fire();
206202
}

wgpu-core/src/device/resource.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,9 @@ use super::{
5757
/// Structure describing a logical device. Some members are internally mutable,
5858
/// stored behind mutexes.
5959
pub struct Device {
60-
raw: ManuallyDrop<Box<dyn hal::DynDevice>>,
60+
raw: Box<dyn hal::DynDevice>,
6161
pub(crate) adapter: Arc<Adapter>,
6262
pub(crate) queue: OnceLock<Weak<Queue>>,
63-
queue_to_drop: OnceLock<Box<dyn hal::DynQueue>>,
6463
pub(crate) zero_buffer: ManuallyDrop<Box<dyn hal::DynBuffer>>,
6564
/// The `label` from the descriptor used to create the resource.
6665
label: String,
@@ -154,23 +153,19 @@ impl Drop for Device {
154153
closure.call(DeviceLostReason::Dropped, String::from("Device dropped."));
155154
}
156155

157-
// SAFETY: We are in the Drop impl and we don't use self.raw anymore after this point.
158-
let raw = unsafe { ManuallyDrop::take(&mut self.raw) };
159156
// SAFETY: We are in the Drop impl and we don't use self.zero_buffer anymore after this point.
160157
let zero_buffer = unsafe { ManuallyDrop::take(&mut self.zero_buffer) };
161158
// SAFETY: We are in the Drop impl and we don't use self.fence anymore after this point.
162159
let fence = unsafe { ManuallyDrop::take(&mut self.fence.write()) };
163-
self.command_allocator.dispose(raw.as_ref());
160+
self.command_allocator.dispose(self.raw.as_ref());
164161
#[cfg(feature = "indirect-validation")]
165162
self.indirect_validation
166163
.take()
167164
.unwrap()
168-
.dispose(raw.as_ref());
165+
.dispose(self.raw.as_ref());
169166
unsafe {
170-
raw.destroy_buffer(zero_buffer);
171-
raw.destroy_fence(fence);
172-
let queue = self.queue_to_drop.take().unwrap();
173-
raw.exit(queue);
167+
self.raw.destroy_buffer(zero_buffer);
168+
self.raw.destroy_fence(fence);
174169
}
175170
}
176171
}
@@ -249,10 +244,9 @@ impl Device {
249244
};
250245

251246
Ok(Self {
252-
raw: ManuallyDrop::new(raw_device),
247+
raw: raw_device,
253248
adapter: adapter.clone(),
254249
queue: OnceLock::new(),
255-
queue_to_drop: OnceLock::new(),
256250
zero_buffer: ManuallyDrop::new(zero_buffer),
257251
label: desc.label.to_string(),
258252
command_allocator,
@@ -325,10 +319,6 @@ impl Device {
325319
DeviceError::from_hal(error)
326320
}
327321

328-
pub(crate) fn release_queue(&self, queue: Box<dyn hal::DynQueue>) {
329-
assert!(self.queue_to_drop.set(queue).is_ok());
330-
}
331-
332322
/// Run some destroy operations that were deferred.
333323
///
334324
/// Destroying the resources requires taking a write lock on the device's snatch lock,

wgpu-hal/examples/halmark/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,8 @@ impl<A: hal::Api> Example<A> {
579579
self.device.destroy_pipeline_layout(self.pipeline_layout);
580580

581581
self.surface.unconfigure(&self.device);
582-
self.device.exit(self.queue);
582+
drop(self.queue);
583+
drop(self.device);
583584
drop(self.surface);
584585
drop(self.adapter);
585586
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,8 @@ impl<A: hal::Api> Example<A> {
10681068
self.device.destroy_shader_module(self.shader_module);
10691069

10701070
self.surface.unconfigure(&self.device);
1071-
self.device.exit(self.queue);
1071+
drop(self.queue);
1072+
drop(self.device);
10721073
drop(self.surface);
10731074
drop(self.adapter);
10741075
}

wgpu-hal/src/dx12/device.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,6 @@ impl super::Device {
391391
impl crate::Device for super::Device {
392392
type A = super::Api;
393393

394-
unsafe fn exit(self, _queue: super::Queue) {
395-
self.rtv_pool.lock().free_handle(self.null_rtv_handle);
396-
}
397-
398394
unsafe fn create_buffer(
399395
&self,
400396
desc: &crate::BufferDescriptor,

wgpu-hal/src/dx12/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,12 @@ pub struct Device {
602602
counters: wgt::HalCounters,
603603
}
604604

605+
impl Drop for Device {
606+
fn drop(&mut self) {
607+
self.rtv_pool.lock().free_handle(self.null_rtv_handle);
608+
}
609+
}
610+
605611
unsafe impl Send for Device {}
606612
unsafe impl Sync for Device {}
607613

wgpu-hal/src/dynamic/device.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ use super::{
1616
};
1717

1818
pub trait DynDevice: DynResource {
19-
unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>);
20-
2119
unsafe fn create_buffer(
2220
&self,
2321
desc: &BufferDescriptor,
@@ -166,10 +164,6 @@ pub trait DynDevice: DynResource {
166164
}
167165

168166
impl<D: Device + DynResource> DynDevice for D {
169-
unsafe fn exit(self: Box<Self>, queue: Box<dyn DynQueue>) {
170-
unsafe { D::exit(*self, queue.unbox()) }
171-
}
172-
173167
unsafe fn create_buffer(
174168
&self,
175169
desc: &BufferDescriptor,

wgpu-hal/src/empty.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ impl crate::Queue for Context {
163163
impl crate::Device for Context {
164164
type A = Api;
165165

166-
unsafe fn exit(self, queue: Context) {}
167166
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<Resource> {
168167
Ok(Resource)
169168
}

wgpu-hal/src/gles/device.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -501,14 +501,6 @@ impl super::Device {
501501
impl crate::Device for super::Device {
502502
type A = super::Api;
503503

504-
unsafe fn exit(self, queue: super::Queue) {
505-
let gl = &self.shared.context.lock();
506-
unsafe { gl.delete_vertex_array(self.main_vao) };
507-
unsafe { gl.delete_framebuffer(queue.draw_fbo) };
508-
unsafe { gl.delete_framebuffer(queue.copy_fbo) };
509-
unsafe { gl.delete_buffer(queue.zero_buffer) };
510-
}
511-
512504
unsafe fn create_buffer(
513505
&self,
514506
desc: &crate::BufferDescriptor,

wgpu-hal/src/gles/mod.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,13 @@ pub struct Device {
295295
counters: wgt::HalCounters,
296296
}
297297

298+
impl Drop for Device {
299+
fn drop(&mut self) {
300+
let gl = &self.shared.context.lock();
301+
unsafe { gl.delete_vertex_array(self.main_vao) };
302+
}
303+
}
304+
298305
pub struct ShaderClearProgram {
299306
pub program: glow::Program,
300307
pub color_uniform_location: glow::UniformLocation,
@@ -316,6 +323,15 @@ pub struct Queue {
316323
current_index_buffer: Mutex<Option<glow::Buffer>>,
317324
}
318325

326+
impl Drop for Queue {
327+
fn drop(&mut self) {
328+
let gl = &self.shared.context.lock();
329+
unsafe { gl.delete_framebuffer(self.draw_fbo) };
330+
unsafe { gl.delete_framebuffer(self.copy_fbo) };
331+
unsafe { gl.delete_buffer(self.zero_buffer) };
332+
}
333+
}
334+
319335
#[derive(Clone, Debug)]
320336
pub struct Buffer {
321337
raw: Option<glow::Buffer>,

wgpu-hal/src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ pub trait Adapter: WasmNotSendSync {
676676
/// 1) Free resources with methods like [`Device::destroy_texture`] or
677677
/// [`Device::destroy_shader_module`].
678678
///
679-
/// 1) Shut down the device by calling [`Device::exit`].
679+
/// 1) Drop the device.
680680
///
681681
/// [`vkDevice`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VkDevice
682682
/// [`ID3D12Device`]: https://learn.microsoft.com/en-us/windows/win32/api/d3d12/nn-d3d12-id3d12device
@@ -706,8 +706,6 @@ pub trait Adapter: WasmNotSendSync {
706706
pub trait Device: WasmNotSendSync {
707707
type A: Api;
708708

709-
/// Exit connection to this logical device.
710-
unsafe fn exit(self, queue: <Self::A as Api>::Queue);
711709
/// Creates a new buffer.
712710
///
713711
/// The initial usage is `BufferUses::empty()`.

wgpu-hal/src/metal/device.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,8 +320,6 @@ impl super::Device {
320320
impl crate::Device for super::Device {
321321
type A = super::Api;
322322

323-
unsafe fn exit(self, _queue: super::Queue) {}
324-
325323
unsafe fn create_buffer(&self, desc: &crate::BufferDescriptor) -> DeviceResult<super::Buffer> {
326324
let map_read = desc.usage.contains(crate::BufferUses::MAP_READ);
327325
let map_write = desc.usage.contains(crate::BufferUses::MAP_WRITE);

wgpu-hal/src/vulkan/device.rs

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -289,18 +289,6 @@ impl super::DeviceShared {
289289
.size((range.end - range.start + mask) & !mask)
290290
}))
291291
}
292-
293-
unsafe fn free_resources(&self) {
294-
for &raw in self.render_passes.lock().values() {
295-
unsafe { self.raw.destroy_render_pass(raw, None) };
296-
}
297-
for &raw in self.framebuffers.lock().values() {
298-
unsafe { self.raw.destroy_framebuffer(raw, None) };
299-
}
300-
if self.drop_guard.is_none() {
301-
unsafe { self.raw.destroy_device(None) };
302-
}
303-
}
304292
}
305293

306294
impl gpu_alloc::MemoryDevice<vk::DeviceMemory> for super::DeviceShared {
@@ -1023,18 +1011,6 @@ impl super::Device {
10231011
impl crate::Device for super::Device {
10241012
type A = super::Api;
10251013

1026-
unsafe fn exit(self, queue: super::Queue) {
1027-
unsafe { self.mem_allocator.into_inner().cleanup(&*self.shared) };
1028-
unsafe { self.desc_allocator.into_inner().cleanup(&*self.shared) };
1029-
unsafe {
1030-
queue
1031-
.relay_semaphores
1032-
.into_inner()
1033-
.destroy(&self.shared.raw)
1034-
};
1035-
unsafe { self.shared.free_resources() };
1036-
}
1037-
10381014
unsafe fn create_buffer(
10391015
&self,
10401016
desc: &crate::BufferDescriptor,

wgpu-hal/src/vulkan/mod.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,20 @@ struct DeviceShared {
646646
memory_allocations_counter: InternalCounter,
647647
}
648648

649+
impl Drop for DeviceShared {
650+
fn drop(&mut self) {
651+
for &raw in self.render_passes.lock().values() {
652+
unsafe { self.raw.destroy_render_pass(raw, None) };
653+
}
654+
for &raw in self.framebuffers.lock().values() {
655+
unsafe { self.raw.destroy_framebuffer(raw, None) };
656+
}
657+
if self.drop_guard.is_none() {
658+
unsafe { self.raw.destroy_device(None) };
659+
}
660+
}
661+
}
662+
649663
pub struct Device {
650664
shared: Arc<DeviceShared>,
651665
mem_allocator: Mutex<gpu_alloc::GpuAllocator<vk::DeviceMemory>>,
@@ -658,6 +672,13 @@ pub struct Device {
658672
counters: wgt::HalCounters,
659673
}
660674

675+
impl Drop for Device {
676+
fn drop(&mut self) {
677+
unsafe { self.mem_allocator.lock().cleanup(&*self.shared) };
678+
unsafe { self.desc_allocator.lock().cleanup(&*self.shared) };
679+
}
680+
}
681+
661682
/// Semaphores for forcing queue submissions to run in order.
662683
///
663684
/// The [`wgpu_hal::Queue`] trait promises that if two calls to [`submit`] are
@@ -741,6 +762,12 @@ pub struct Queue {
741762
relay_semaphores: Mutex<RelaySemaphores>,
742763
}
743764

765+
impl Drop for Queue {
766+
fn drop(&mut self) {
767+
unsafe { self.relay_semaphores.lock().destroy(&self.device.raw) };
768+
}
769+
}
770+
744771
#[derive(Debug)]
745772
pub struct Buffer {
746773
raw: vk::Buffer,

0 commit comments

Comments
 (0)