Better handle destroying textures and buffers (#4657)

* Better handle destroying textures and buffers

Before this commit, explicitly destroying a texture or a buffer (without dropping it)
schedules the asynchronous destruction of its raw resources but does not actually mark
it as destroyed. This can cause some incorrect behavior, for example mapping a buffer
after destroying it does not cause a validation error (and later panics due to the
map callback being dropped without being called).

This Commit adds `Storage::take_and_mark_destroyed` for use in `destroy` methods.
Since it puts the resource in the error state, other methods properly catch that
the resource is no longer usable when attempting to access it and raise validation
errors.

There are other resource types that require similar treatment and will be addressed
in followup work.

* Add a changelog entry

CHANGELOG.md

@@ -274,6 +274,7 @@ By @teoxoy in [#4185](https://github.com/gfx-rs/wgpu/pull/4185)
 - Add `Feature::SHADER_UNUSED_VERTEX_OUTPUT` to allow unused vertex shader outputs. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116).
 - Fix a panic in `surface_configure`. By @nical in [#4220](https://github.com/gfx-rs/wgpu/pull/4220) and [#4227](https://github.com/gfx-rs/wgpu/pull/4227)
 - Pipelines register their implicit layouts in error cases. By @bradwerth in [#4624](https://github.com/gfx-rs/wgpu/pull/4624)
+- Better handle explicit destruction of textures and buffers. By @nical in [#4657](https://github.com/gfx-rs/wgpu/pull/4657)
 
 #### Vulkan
 

tests/tests/gpu.rs

@@ -16,6 +16,7 @@ mod device;
 mod encoder;
 mod external_texture;
 mod instance;
+mod life_cycle;
 mod occlusion_query;
 mod partially_bounded_arrays;
 mod pipeline;

tests/tests/life_cycle.rs

@@ -0,0 +1,63 @@
+use wgpu_test::{gpu_test, FailureCase, GpuTestConfiguration, TestParameters};
+
+#[gpu_test]
+static BUFFER_DESTROY: GpuTestConfiguration = GpuTestConfiguration::new()
+ .parameters(TestParameters::default().expect_fail(FailureCase::always()))
+ .run_sync(|ctx| {
+ let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
+ label: None,
+ size: 256,
+ usage: wgpu::BufferUsages::MAP_WRITE | wgpu::BufferUsages::COPY_SRC,
+ mapped_at_creation: false,
+ });
+
+ buffer.destroy();
+
+ buffer.destroy();
+
+ ctx.device.poll(wgpu::MaintainBase::Wait);
+
+ buffer
+ .slice(..)
+ .map_async(wgpu::MapMode::Write, move |_| {});
+
+ buffer.destroy();
+
+ ctx.device.poll(wgpu::MaintainBase::Wait);
+
+ buffer.destroy();
+
+ buffer.destroy();
+ });
+
+#[gpu_test]
+static TEXTURE_DESTROY: GpuTestConfiguration = GpuTestConfiguration::new().run_sync(|ctx| {
+ let texture = ctx.device.create_texture(&wgpu::TextureDescriptor {
+ label: None,
+ size: wgpu::Extent3d {
+ width: 128,
+ height: 128,
+ depth_or_array_layers: 1,
+ },
+ mip_level_count: 1,
+ sample_count: 1, // multisampling is not supported for clear
+ dimension: wgpu::TextureDimension::D2,
+ format: wgpu::TextureFormat::Rgba8Snorm,
+ usage: wgpu::TextureUsages::COPY_DST | wgpu::TextureUsages::TEXTURE_BINDING,
+ view_formats: &[],
+ });
+
+ texture.destroy();
+
+ texture.destroy();
+
+ ctx.device.poll(wgpu::MaintainBase::Wait);
+
+ texture.destroy();
+
+ ctx.device.poll(wgpu::MaintainBase::Wait);
+
+ texture.destroy();
+
+ texture.destroy();
+});

wgpu-core/src/device/global.rs

@@ -495,8 +495,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
  log::trace!("Buffer::destroy {buffer_id:?}");
  let (mut buffer_guard, _) = hub.buffers.write(&mut token);
- let buffer = buffer_guard
- .get_mut(buffer_id)
+ let mut buffer = buffer_guard
+ .take_and_mark_destroyed(buffer_id)
  .map_err(|_| resource::DestroyError::Invalid)?;
 
  let device = &mut device_guard[buffer.device_id.value];
@@ -506,7 +506,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
  | &BufferMapState::Init { .. }
  | &BufferMapState::Active { .. }
  => {
- self.buffer_unmap_inner(buffer_id, buffer, device)
+ self.buffer_unmap_inner(buffer_id, &mut buffer, device)
  .unwrap_or(None)
  }
  _ => None,
@@ -800,8 +800,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
  let (mut device_guard, mut token) = hub.devices.write(&mut token);
 
  let (mut texture_guard, _) = hub.textures.write(&mut token);
- let texture = texture_guard
- .get_mut(texture_id)
+ let mut texture = texture_guard
+ .take_and_mark_destroyed(texture_id)
  .map_err(|_| resource::DestroyError::Invalid)?;
 
  let device = &mut device_guard[texture.device_id.value];

wgpu-core/src/storage.rs

@@ -169,6 +169,21 @@ impl<T, I: id::TypedId> Storage<T, I> {
  self.insert_impl(index as usize, Element::Error(epoch, label.to_string()))
  }
 
+ pub(crate) fn take_and_mark_destroyed(&mut self, id: I) -> Result<T, InvalidId> {
+ let (index, epoch, _) = id.unzip();
+ match std::mem::replace(
+ &mut self.map[index as usize],
+ Element::Error(epoch, String::new()),
+ ) {
+ Element::Vacant => panic!("Cannot mark a vacant resource destroyed"),
+ Element::Occupied(value, storage_epoch) => {
+ assert_eq!(epoch, storage_epoch);
+ Ok(value)
+ }
+ _ => Err(InvalidId),
+ }
+ }
+
  pub(crate) fn force_replace(&mut self, id: I, value: T) {
  let (index, epoch, _) = id.unzip();
  self.map[index as usize] = Element::Occupied(value, epoch);