[core] Ensure all lock guards are properly nested.

wgpu-core/src/device/global.rs

@@ -2033,7 +2033,6 @@ impl Global {
                 // Wait for all work to finish before configuring the surface.
                 let snatch_guard = device.snatchable_lock.read();
                 let fence = device.fence.read();
-                let fence = fence.as_ref().unwrap();
                 match device.maintain(fence, wgt::Maintain::Wait, snatch_guard) {
                     Ok((closures, _)) => {
                         user_callbacks = closures;
@@ -2146,7 +2145,6 @@ impl Global {
     ) -> Result<DevicePoll, WaitIdleError> {
         let snatch_guard = device.snatchable_lock.read();
         let fence = device.fence.read();
-        let fence = fence.as_ref().unwrap();
         let (closures, queue_empty) = device.maintain(fence, maintain, snatch_guard)?;
 
         // Some deferred destroys are scheduled in maintain so run this right after

wgpu-core/src/device/queue.rs

@@ -14,7 +14,7 @@ use crate::{
     hal_label,
     id::{self, DeviceId, QueueId},
     init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange},
-    lock::{rank, Mutex},
+    lock::{rank, Mutex, RwLockWriteGuard},
     resource::{
         Buffer, BufferAccessError, BufferMapState, DestroyedBuffer, DestroyedTexture, Resource,
         ResourceInfo, ResourceType, StagingBuffer, Texture, TextureInner,
@@ -1162,8 +1162,8 @@ impl Global {
             let snatch_guard = device.snatchable_lock.read();
 
             // Fence lock must be acquired after the snatch lock everywhere to avoid deadlocks.
-            let mut fence = device.fence.write();
-            let fence = fence.as_mut().unwrap();
+            let mut fence_guard = device.fence.write();
+            let fence = fence_guard.as_mut().unwrap();
             let submit_index = device
                 .active_submission_index
                 .fetch_add(1, Ordering::Relaxed)
@@ -1459,8 +1459,8 @@ impl Global {
                 }
             }
 
-            let mut pending_writes = device.pending_writes.lock();
-            let pending_writes = pending_writes.as_mut().unwrap();
+            let mut pending_writes_guard = device.pending_writes.lock();
+            let pending_writes = pending_writes_guard.as_mut().unwrap();
 
             {
                 used_surface_textures.set_size(hub.textures.read().len());
@@ -1550,18 +1550,22 @@ impl Global {
                 active_executions,
             );
 
-            // This will schedule destruction of all resources that are no longer needed
-            // by the user but used in the command stream, among other things.
-            let (closures, _) = match device.maintain(fence, wgt::Maintain::Poll, snatch_guard) {
-                Ok(closures) => closures,
-                Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
-                Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
-                Err(WaitIdleError::WrongSubmissionIndex(..)) => unreachable!(),
-            };
-
             // pending_write_resources has been drained, so it's empty, but we
             // want to retain its heap allocation.
             pending_writes.temp_resources = pending_write_resources;
+            drop(pending_writes_guard);
+
+            // This will schedule destruction of all resources that are no longer needed
+            // by the user but used in the command stream, among other things.
+            let fence_guard = RwLockWriteGuard::downgrade(fence_guard);
+            let (closures, _) =
+                match device.maintain(fence_guard, wgt::Maintain::Poll, snatch_guard) {
+                    Ok(closures) => closures,
+                    Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
+                    Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
+                    Err(WaitIdleError::WrongSubmissionIndex(..)) => unreachable!(),
+                };
+
             device.lock_life().post_submit();
 
             (submit_index, closures)

wgpu-core/src/device/resource.rs

@@ -397,11 +397,12 @@ impl<A: HalApi> Device<A> {
     ///   return it to our callers.)
     pub(crate) fn maintain<'this>(
         &'this self,
-        fence: &A::Fence,
+        fence_guard: crate::lock::RwLockReadGuard<Option<A::Fence>>,
         maintain: wgt::Maintain<queue::WrappedSubmissionIndex>,
         snatch_guard: SnatchGuard,
     ) -> Result<(UserClosures, bool), WaitIdleError> {
         profiling::scope!("Device::maintain");
+        let fence = fence_guard.as_ref().unwrap();
         let last_done_index = if maintain.is_wait() {
             let index_to_wait_for = match maintain {
                 wgt::Maintain::WaitForSubmissionIndex(submission_index) => {
@@ -481,6 +482,7 @@ impl<A: HalApi> Device<A> {
 
         // Don't hold the locks while calling release_gpu_resources.
         drop(life_tracker);
+        drop(fence_guard);
         drop(snatch_guard);
 
         if should_release_gpu_resource {

wgpu-core/src/lock/ranked.rs

@@ -81,7 +81,7 @@ pub struct Mutex<T> {
 /// [mod]: crate::lock::ranked
 pub struct MutexGuard<'a, T> {
     inner: parking_lot::MutexGuard<'a, T>,
-    saved: LockState,
+    saved: LockStateGuard,
 }
 
 thread_local! {
@@ -107,6 +107,26 @@ impl LockState {
     };
 }
 
+/// A container that restores a [`LockState`] when dropped.
+///
+/// This type serves two purposes:
+///
+/// - Operations like `RwLockWriteGuard::downgrade` would like to be able to
+///   destructure lock guards and reassemble their pieces into new guards, but
+///   if the guard type itself implements `Drop`, we can't destructure it
+///   without unsafe code or pointless `Option`s whose state is almost always
+///   statically known.
+///
+/// - We can just implement `Drop` for this type once, and then use it in lock
+///   guards, rather than implementing `Drop` separately for each guard type.
+struct LockStateGuard(LockState);
+
+impl Drop for LockStateGuard {
+    fn drop(&mut self) {
+        release(self.0)
+    }
+}
+
 /// Check and record the acquisition of a lock with `new_rank`.
 ///
 /// Check that acquiring a lock with `new_rank` is permitted at this point, and
@@ -170,17 +190,11 @@ impl<T> Mutex<T> {
         let saved = acquire(self.rank, Location::caller());
         MutexGuard {
             inner: self.inner.lock(),
-            saved,
+            saved: LockStateGuard(saved),
         }
     }
 }
 
-impl<'a, T> Drop for MutexGuard<'a, T> {
-    fn drop(&mut self) {
-        release(self.saved);
-    }
-}
-
 impl<'a, T> std::ops::Deref for MutexGuard<'a, T> {
     type Target = T;
 
@@ -224,7 +238,7 @@ pub struct RwLock<T> {
 /// [mod]: crate::lock::ranked
 pub struct RwLockReadGuard<'a, T> {
     inner: parking_lot::RwLockReadGuard<'a, T>,
-    saved: LockState,
+    saved: LockStateGuard,
 }
 
 /// A write guard produced by locking [`RwLock`] for writing.
@@ -237,7 +251,7 @@ pub struct RwLockReadGuard<'a, T> {
 /// [mod]: crate::lock::ranked
 pub struct RwLockWriteGuard<'a, T> {
     inner: parking_lot::RwLockWriteGuard<'a, T>,
-    saved: LockState,
+    saved: LockStateGuard,
 }
 
 impl<T> RwLock<T> {
@@ -253,7 +267,7 @@ impl<T> RwLock<T> {
         let saved = acquire(self.rank, Location::caller());
         RwLockReadGuard {
             inner: self.inner.read(),
-            saved,
+            saved: LockStateGuard(saved),
         }
     }
 
@@ -262,26 +276,23 @@ impl<T> RwLock<T> {
         let saved = acquire(self.rank, Location::caller());
         RwLockWriteGuard {
             inner: self.inner.write(),
-            saved,
+            saved: LockStateGuard(saved),
         }
     }
 }
 
-impl<T: std::fmt::Debug> std::fmt::Debug for RwLock<T> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.inner.fmt(f)
-    }
-}
-
-impl<'a, T> Drop for RwLockReadGuard<'a, T> {
-    fn drop(&mut self) {
-        release(self.saved);
+impl<'a, T> RwLockWriteGuard<'a, T> {
+    pub fn downgrade(this: Self) -> RwLockReadGuard<'a, T> {
+        RwLockReadGuard {
+            inner: parking_lot::RwLockWriteGuard::downgrade(this.inner),
+            saved: this.saved,
+        }
     }
 }
 
-impl<'a, T> Drop for RwLockWriteGuard<'a, T> {
-    fn drop(&mut self) {
-        release(self.saved);
+impl<T: std::fmt::Debug> std::fmt::Debug for RwLock<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.inner.fmt(f)
     }
 }
 

wgpu-core/src/lock/vanilla.rs

@@ -86,6 +86,12 @@ impl<T> RwLock<T> {
     }
 }
 
+impl<'a, T> RwLockWriteGuard<'a, T> {
+    pub fn downgrade(this: Self) -> RwLockReadGuard<'a, T> {
+        RwLockReadGuard(parking_lot::RwLockWriteGuard::downgrade(this.0))
+    }
+}
+
 impl<T: std::fmt::Debug> std::fmt::Debug for RwLock<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         self.0.fmt(f)