Remove the Destroyed state from Storage

wgpu-core/src/device/global.rs

@@ -489,8 +489,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let buffer = hub
             .buffers
-            .write()
-            .get_and_mark_destroyed(buffer_id)
+            .get(buffer_id)
             .map_err(|_| resource::DestroyError::Invalid)?;
 
         let _ = buffer.unmap();
@@ -732,8 +731,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
         let texture = hub
             .textures
-            .write()
-            .get_and_mark_destroyed(texture_id)
+            .get(texture_id)
             .map_err(|_| resource::DestroyError::Invalid)?;
 
         texture.destroy()
@@ -799,6 +797,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 Err(_) => break resource::CreateTextureViewError::InvalidTexture,
             };
             let device = &texture.device;
+            {
+                let snatch_guard = device.snatchable_lock.read();
+                if texture.is_destroyed(&snatch_guard) {
+                    break resource::CreateTextureViewError::InvalidTexture;
+                }
+            }
             #[cfg(feature = "trace")]
             if let Some(ref mut trace) = *device.trace.lock() {
                 trace.add(trace::Action::CreateTextureView {
@@ -2386,6 +2390,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                     },
                 ));
             }
+
+            let snatch_guard = device.snatchable_lock.read();
+            if buffer.is_destroyed(&snatch_guard) {
+                return Err((op, BufferAccessError::Destroyed));
+            }
+
             {
                 let map_state = &mut *buffer.map_state.lock();
                 *map_state = match *map_state {
@@ -2410,10 +2420,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 let mut trackers = buffer.device.as_ref().trackers.lock();
                 trackers.buffers.set_single(&buffer, internal_use);
                 //TODO: Check if draining ALL buffers is correct!
-                let snatch_guard = device.snatchable_lock.read();
                 let _ = trackers.buffers.drain_transitions(&snatch_guard);
             }
 
+            drop(snatch_guard);
+
             buffer
         };
 
@@ -2438,6 +2449,13 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             .get(buffer_id)
             .map_err(|_| BufferAccessError::Invalid)?;
 
+        {
+            let snatch_guard = buffer.device.snatchable_lock.read();
+            if buffer.is_destroyed(&snatch_guard) {
+                return Err(BufferAccessError::Destroyed);
+            }
+        }
+
         let range_size = if let Some(size) = size {
             size
         } else if offset > buffer.size {
@@ -2500,6 +2518,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             .get(buffer_id)
             .map_err(|_| BufferAccessError::Invalid)?;
 
+        let snatch_guard = buffer.device.snatchable_lock.read();
+        if buffer.is_destroyed(&snatch_guard) {
+            return Err(BufferAccessError::Destroyed);
+        }
+        drop(snatch_guard);
+
         if !buffer.device.is_valid() {
             return Err(DeviceError::Lost.into());
         }

wgpu-core/src/device/queue.rs

@@ -1158,8 +1158,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         // it, so make sure to set_size on it.
                         used_surface_textures.set_size(hub.textures.read().len());
 
-                        // TODO: ideally we would use `get_and_mark_destroyed` but the code here
-                        // wants to consume the command buffer.
                         #[allow(unused_mut)]
                         let mut cmdbuf = match command_buffer_guard.replace_with_error(cmb_id) {
                             Ok(cmdbuf) => cmdbuf,

wgpu-core/src/registry.rs

@@ -15,7 +15,6 @@ pub struct RegistryReport {
     pub num_allocated: usize,
     pub num_kept_from_user: usize,
     pub num_released_from_user: usize,
-    pub num_destroyed_from_user: usize,
     pub num_error: usize,
     pub element_size: usize,
 }
@@ -192,7 +191,6 @@ impl<I: id::TypedId, T: Resource<I>> Registry<I, T> {
         for element in storage.map.iter() {
             match *element {
                 Element::Occupied(..) => report.num_kept_from_user += 1,
-                Element::Destroyed(..) => report.num_destroyed_from_user += 1,
                 Element::Vacant => report.num_released_from_user += 1,
                 Element::Error(..) => report.num_error += 1,
             }

wgpu-core/src/resource.rs

@@ -433,6 +433,10 @@ impl<A: HalApi> Buffer<A> {
         self.raw.get(guard)
     }
 
+    pub(crate) fn is_destroyed(&self, guard: &SnatchGuard) -> bool {
+        self.raw.get(guard).is_none()
+    }
+
     // Note: This must not be called while holding a lock.
     pub(crate) fn unmap(self: &Arc<Self>) -> Result<(), BufferAccessError> {
         if let Some((mut operation, status)) = self.unmap_inner()? {
@@ -818,6 +822,10 @@ impl<A: HalApi> Texture<A> {
         self.inner.get(snatch_guard)?.raw()
     }
 
+    pub(crate) fn is_destroyed(&self, guard: &SnatchGuard) -> bool {
+        self.inner.get(guard).is_none()
+    }
+
     pub(crate) fn inner_mut<'a>(
         &'a self,
         guard: &mut ExclusiveSnatchGuard,

wgpu-core/src/storage.rs

@@ -14,10 +14,6 @@ pub(crate) enum Element<T> {
     /// epoch.
     Occupied(Arc<T>, Epoch),
 
-    /// Like `Occupied`, but the resource has been marked as destroyed
-    /// and hasn't been dropped yet.
-    Destroyed(Epoch),
-
     /// Like `Occupied`, but an error occurred when creating the
     /// resource.
     ///
@@ -78,11 +74,9 @@ where
         let (index, epoch, _) = id.unzip();
         match self.map.get(index as usize) {
             Some(&Element::Vacant) => false,
-            Some(
-                &Element::Occupied(_, storage_epoch)
-                | &Element::Destroyed(storage_epoch)
-                | &Element::Error(storage_epoch, _),
-            ) => storage_epoch == epoch,
+            Some(&Element::Occupied(_, storage_epoch) | &Element::Error(storage_epoch, _)) => {
+                storage_epoch == epoch
+            }
             None => false,
         }
     }
@@ -99,9 +93,7 @@ where
         let (result, storage_epoch) = match self.map.get(index as usize) {
             Some(&Element::Occupied(ref v, epoch)) => (Ok(Some(v)), epoch),
             Some(&Element::Vacant) => return Ok(None),
-            Some(&Element::Error(epoch, ..)) | Some(&Element::Destroyed(.., epoch)) => {
-                (Err(InvalidId), epoch)
-            }
+            Some(&Element::Error(epoch, ..)) => (Err(InvalidId), epoch),
             None => return Err(InvalidId),
         };
         assert_eq!(
@@ -120,7 +112,6 @@ where
             Some(&Element::Occupied(ref v, epoch)) => (Ok(v), epoch),
             Some(&Element::Vacant) => panic!("{}[{:?}] does not exist", self.kind, id),
             Some(&Element::Error(epoch, ..)) => (Err(InvalidId), epoch),
-            Some(&Element::Destroyed(.., epoch)) => (Err(InvalidId), epoch),
             None => return Err(InvalidId),
         };
         assert_eq!(
@@ -151,14 +142,6 @@ where
         }
         match std::mem::replace(&mut self.map[index], element) {
             Element::Vacant => {}
-            Element::Destroyed(storage_epoch) => {
-                assert_ne!(
-                    epoch,
-                    storage_epoch,
-                    "Index {index:?} of {} is already occupied",
-                    T::TYPE
-                );
-            }
             Element::Occupied(_, storage_epoch) => {
                 assert_ne!(
                     epoch,
@@ -209,22 +192,6 @@ where
         }
     }
 
-    pub(crate) fn get_and_mark_destroyed(&mut self, id: I) -> Result<Arc<T>, InvalidId> {
-        let (index, epoch, _) = id.unzip();
-        let slot = &mut self.map[index as usize];
-        // borrowck dance: we have to move the element out before we can replace it
-        // with another variant with the same value.
-        if let &mut Element::Occupied(_, e) = slot {
-            if let Element::Occupied(value, storage_epoch) =
-                std::mem::replace(slot, Element::Destroyed(e))
-            {
-                debug_assert_eq!(storage_epoch, epoch);
-                return Ok(value);
-            }
-        }
-        Err(InvalidId)
-    }
-
     pub(crate) fn force_replace(&mut self, id: I, value: T) {
         log::trace!("User is replacing {}{:?}", T::TYPE, id);
         let (index, epoch, _) = id.unzip();
@@ -239,10 +206,6 @@ where
                 assert_eq!(epoch, storage_epoch);
                 Some(value)
             }
-            Element::Destroyed(storage_epoch) => {
-                assert_eq!(epoch, storage_epoch);
-                None
-            }
             Element::Error(..) => None,
             Element::Vacant => panic!("Cannot remove a vacant resource"),
         }