Merge #1578

1578: Fix destruction of resources r=kvark a=kvark

**Connections**
Fixes #1552
Also fixes the VVL about swapchain frame destruction.

**Description**
Swapchain view was never registered in the device's root Hub. So the old logic of adding it to "suspected resources" didn't fire up correctly. The new logic goes straight into the submission tracker.
This path will cease to exist when either Hubs are removed, or API changes of https://github.com/webgpu-native/webgpu-headers/issues/89 are accepted.

The other resource destruction errors happened because we gathered all the resources that were abandoned by the user, and held alive only by the submitted command buffers, and we added them to the suspected list. Then we'd scan the list in `maintain`, see that they can be removed, and try to associate their destruction with one of the submissions. But the current submission was not added yet! So the logic thinks it can just remove the resources right away in this case, assuming the submission is not found because it's long gone in past.

**Testing**
Tested on our examples.


Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>

wgpu-core/src/device/life.rs

@@ -16,7 +16,7 @@ use hal::Device as _;
 use parking_lot::Mutex;
 use thiserror::Error;
 
-use std::sync::atomic::Ordering;
+use std::{mem, sync::atomic::Ordering};
 
 /// A struct that keeps lists of resources that are no longer needed by the user.
 #[derive(Debug, Default)]
@@ -230,6 +230,15 @@ impl<A: hal::Api> LifetimeTracker<A> {
             }
         }
 
+        self.active.alloc().init(ActiveSubmission {
+            index,
+            last_resources,
+            mapped: Vec::new(),
+            encoders,
+        });
+    }
+
+    pub fn post_submit(&mut self) {
         self.suspected_resources.buffers.extend(
             self.future_suspected_buffers
                 .drain(..)
@@ -240,13 +249,6 @@ impl<A: hal::Api> LifetimeTracker<A> {
                 .drain(..)
                 .map(|stored| stored.value),
         );
-
-        self.active.alloc().init(ActiveSubmission {
-            index,
-            last_resources,
-            mapped: Vec::new(),
-            encoders,
-        });
     }
 
     pub(crate) fn map(&mut self, value: id::Valid<id::BufferId>, ref_count: RefCount) {
@@ -305,6 +307,20 @@ impl<A: hal::Api> LifetimeTracker<A> {
 }
 
 impl<A: HalApi> LifetimeTracker<A> {
+    pub(super) fn schedule_texture_view_for_destruction(
+        &mut self,
+        id: id::Valid<id::TextureViewId>,
+        view: resource::TextureView<A>,
+    ) {
+        let submit_index = view.life_guard.submission_index.load(Ordering::Acquire);
+        self.active
+            .iter_mut()
+            .find(|a| a.index == submit_index)
+            .map_or(&mut self.free_resources, |a| &mut a.last_resources)
+            .texture_views
+            .push((id, view.raw));
+    }
+
     pub(super) fn triage_suspected<G: GlobalIdentityHandlerFactory>(
         &mut self,
         hub: &Hub<A, G>,
@@ -362,7 +378,8 @@ impl<A: HalApi> LifetimeTracker<A> {
             let (mut guard, _) = hub.texture_views.write(token);
             let mut trackers = trackers.lock();
 
-            for id in self.suspected_resources.texture_views.drain(..) {
+            let mut list = mem::take(&mut self.suspected_resources.texture_views);
+            for id in list.drain(..) {
                 if trackers.views.remove_abandoned(id) {
                     #[cfg(feature = "trace")]
                     if let Some(t) = trace {
@@ -371,22 +388,16 @@ impl<A: HalApi> LifetimeTracker<A> {
 
                     if let Some(res) = hub.texture_views.unregister_locked(id.0, &mut *guard) {
                         match res.source {
-                            resource::TextureViewSource::Native(source_id) => {
+                            resource::TextureViewSource::Native(ref source_id) => {
                                 self.suspected_resources.textures.push(source_id.value);
                             }
                             resource::TextureViewSource::SwapChain { .. } => {}
                         };
-
-                        let submit_index = res.life_guard.submission_index.load(Ordering::Acquire);
-                        self.active
-                            .iter_mut()
-                            .find(|a| a.index == submit_index)
-                            .map_or(&mut self.free_resources, |a| &mut a.last_resources)
-                            .texture_views
-                            .push((id, res.raw));
+                        self.schedule_texture_view_for_destruction(id, res);
                     }
                 }
             }
+            self.suspected_resources.texture_views = list;
         }
 
         if !self.suspected_resources.textures.is_empty() {

wgpu-core/src/device/mod.rs

@@ -333,30 +333,22 @@ impl<A: HalApi> Device<A> {
         })
     }
 
-    fn lock_life_internal<'this, 'token: 'this>(
-        tracker: &'this Mutex<life::LifetimeTracker<A>>,
-        _token: &mut Token<'token, Self>,
-    ) -> MutexGuard<'this, life::LifetimeTracker<A>> {
-        tracker.lock()
-    }
-
     fn lock_life<'this, 'token: 'this>(
         &'this self,
         //TODO: fix this - the token has to be borrowed for the lock
-        token: &mut Token<'token, Self>,
+        _token: &mut Token<'token, Self>,
     ) -> MutexGuard<'this, life::LifetimeTracker<A>> {
-        Self::lock_life_internal(&self.life_tracker, token)
+        self.life_tracker.lock()
     }
 
-    pub(crate) fn suspect_texture_view_for_destruction<'this, 'token: 'this>(
+    pub(crate) fn schedule_rogue_texture_view_for_destruction<'this, 'token: 'this>(
         &'this self,
         view_id: id::Valid<id::TextureViewId>,
+        view: resource::TextureView<A>,
         token: &mut Token<'token, Self>,
     ) {
         self.lock_life(token)
-            .suspected_resources
-            .texture_views
-            .push(view_id);
+            .schedule_texture_view_for_destruction(view_id, view);
     }
 
     fn maintain<'this, 'token: 'this, G: GlobalIdentityHandlerFactory>(

wgpu-core/src/device/queue.rs

@@ -808,14 +808,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 }
             }
 
-            let callbacks = match device.maintain(&hub, false, &mut token) {
-                Ok(callbacks) => callbacks,
-                Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
-                Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
-            };
-
-            device.temp_suspected.clear();
-
             profiling::scope!("cleanup");
             if let Some(pending_execution) = device.pending_writes.post_submit(
                 &device.command_allocator,
@@ -824,12 +816,27 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             ) {
                 active_executions.push(pending_execution);
             }
-            super::Device::lock_life_internal(&device.life_tracker, &mut token).track_submission(
+
+            // this will register the new submission to the life time tracker
+            let mut pending_write_resources = mem::take(&mut device.pending_writes.temp_resources);
+            device.lock_life(&mut token).track_submission(
                 submit_index,
-                device.pending_writes.temp_resources.drain(..),
+                pending_write_resources.drain(..),
                 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 callbacks = match device.maintain(&hub, false, &mut token) {
+                Ok(callbacks) => callbacks,
+                Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
+                Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
+            };
+
+            device.pending_writes.temp_resources = pending_write_resources;
+            device.temp_suspected.clear();
+            device.lock_life(&mut token).post_submit();
+
             callbacks
         };
 

wgpu-core/src/swap_chain.rs

@@ -267,11 +267,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 .ok_or(SwapChainError::AlreadyAcquired)?;
 
             drop(swap_chain_guard);
-            device.suspect_texture_view_for_destruction(view_id.value, &mut token);
 
-            let (mut view_guard, _) = hub.texture_views.write(&mut token);
-            let view = &mut view_guard[view_id.value];
-            let _ = view.life_guard.ref_count.take();
+            let (view, _) = hub.texture_views.unregister(view_id.value.0, &mut token);
+            if let Some(view) = view {
+                device.schedule_rogue_texture_view_for_destruction(view_id.value, view, &mut token);
+            }
 
             suf_texture
         };