Prevent panic in untrack if a resource is already destroyed

gfx-rs · Nov 15, 2023 · f5665f7 · f5665f7
1 parent 611471c
commit f5665f7
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 11 deletions.
diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs
@@ -551,7 +551,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let (ref_count, last_submit_index, device_id) = {
             let (mut buffer_guard, _) = hub.buffers.write(&mut token);
-            match buffer_guard.get_occupied_or_destroyed(buffer_id) {
+            match buffer_guard.get_occupied_or_destroyed_mut(buffer_id) {
                 Ok(buffer) => {
                     let ref_count = buffer.life_guard.ref_count.take().unwrap();
                     let last_submit_index = buffer.life_guard.life_count();
@@ -855,7 +855,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let (ref_count, last_submit_index, device_id) = {
             let (mut texture_guard, _) = hub.textures.write(&mut token);
-            match texture_guard.get_occupied_or_destroyed(texture_id) {
+            match texture_guard.get_occupied_or_destroyed_mut(texture_id) {
                 Ok(texture) => {
                     let ref_count = texture.life_guard.ref_count.take().unwrap();
                     let last_submit_index = texture.life_guard.life_count();

diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs
@@ -357,42 +357,90 @@ impl<A: HalApi> Device<A> {
             let (sampler_guard, _) = hub.samplers.read(&mut token);
 
             for id in trackers.buffers.used() {
-                if buffer_guard[id].life_guard.ref_count.is_none() {
+                if buffer_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.buffers.push(id);
                 }
             }
             for id in trackers.textures.used() {
-                if texture_guard[id].life_guard.ref_count.is_none() {
+                if texture_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.textures.push(id);
                 }
             }
             for id in trackers.views.used() {
-                if texture_view_guard[id].life_guard.ref_count.is_none() {
+                if texture_view_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.texture_views.push(id);
                 }
             }
             for id in trackers.bind_groups.used() {
-                if bind_group_guard[id].life_guard.ref_count.is_none() {
+                if bind_group_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.bind_groups.push(id);
                 }
             }
             for id in trackers.samplers.used() {
-                if sampler_guard[id].life_guard.ref_count.is_none() {
+                if sampler_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.samplers.push(id);
                 }
             }
             for id in trackers.compute_pipelines.used() {
-                if compute_pipe_guard[id].life_guard.ref_count.is_none() {
+                if compute_pipe_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.compute_pipelines.push(id);
                 }
             }
             for id in trackers.render_pipelines.used() {
-                if render_pipe_guard[id].life_guard.ref_count.is_none() {
+                if render_pipe_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.render_pipelines.push(id);
                 }
             }
             for id in trackers.query_sets.used() {
-                if query_set_guard[id].life_guard.ref_count.is_none() {
+                if query_set_guard
+                    .get_occupied_or_destroyed(id.0)
+                    .unwrap()
+                    .life_guard
+                    .ref_count
+                    .is_none()
+                {
                     self.temp_suspected.query_sets.push(id);
                 }
             }

diff --git a/wgpu-core/src/storage.rs b/wgpu-core/src/storage.rs
@@ -149,7 +149,7 @@ impl<T, I: id::TypedId> Storage<T, I> {
     /// destroyed resource leads to a validation error. This should be used internally
     /// in places where we want to do some manipulation potentially after the element
     /// was destroyed (for example the drop implementation).
-    pub(crate) fn get_occupied_or_destroyed(&mut self, id: I) -> Result<&mut T, InvalidId> {
+    pub(crate) fn get_occupied_or_destroyed_mut(&mut self, id: I) -> Result<&mut T, InvalidId> {
         let (index, epoch, _) = id.unzip();
         let (result, storage_epoch) = match self.map.get_mut(index as usize) {
             Some(&mut Element::Occupied(ref mut v, epoch))
@@ -165,6 +165,23 @@ impl<T, I: id::TypedId> Storage<T, I> {
         result
     }
 
+    pub(crate) fn get_occupied_or_destroyed(&self, id: I) -> Result<&T, InvalidId> {
+        let (index, epoch, _) = id.unzip();
+        let (result, storage_epoch) = match self.map.get(index as usize) {
+            Some(&Element::Occupied(ref v, epoch)) | Some(&Element::Destroyed(ref v, epoch)) => {
+                (Ok(v), epoch)
+            }
+            Some(&Element::Vacant) | None => panic!("{}[{}] does not exist", self.kind, index),
+            Some(&Element::Error(epoch, ..)) => (Err(InvalidId), epoch),
+        };
+        assert_eq!(
+            epoch, storage_epoch,
+            "{}[{}] is no longer alive",
+            self.kind, index
+        );
+        result
+    }
+
     pub(crate) unsafe fn get_unchecked(&self, id: u32) -> &T {
         match self.map[id as usize] {
             Element::Occupied(ref v, _) => v,