Free command encoders' platform resources on drop on _all_ platforms

CHANGELOG.md

@@ -113,6 +113,7 @@ Bottom level categories:
 - Fix panic when creating a surface while no backend is available. By @wumpf [#5166](https://github.com/gfx-rs/wgpu/pull/5166)
 - Correctly compute minimum buffer size for array-typed `storage` and `uniform` vars. By @jimblandy [#5222](https://github.com/gfx-rs/wgpu/pull/5222)
 - Fix timeout when presenting a surface where no work has been done. By @waywardmonkeys in [#5200](https://github.com/gfx-rs/wgpu/pull/5200)
+- Fix an issue where command encoders weren't properly freed if an error occurred during command encoding. By @ErichDonGubler in [#5251](https://github.com/gfx-rs/wgpu/pull/5251).
 
 #### WGL
 

tests/tests/bind_group_layout_dedup.rs

@@ -1,9 +1,6 @@
 use std::num::NonZeroU64;
 
-use wgpu_test::{
- fail, gpu_test, FailureCase, GpuTestConfiguration, TestParameters, TestingContext,
-};
-use wgt::Backends;
+use wgpu_test::{fail, gpu_test, GpuTestConfiguration, TestParameters, TestingContext};
 
 const SHADER_SRC: &str = "
 @group(0) @binding(0)
@@ -307,18 +304,10 @@ fn bgl_dedupe_derived(ctx: TestingContext) {
  ctx.queue.submit(Some(encoder.finish()));
 }
 
-const DX12_VALIDATION_ERROR: &str = "The command allocator cannot be reset because a command list is currently being recorded with the allocator.";
-
 #[gpu_test]
 static SEPARATE_PROGRAMS_HAVE_INCOMPATIBLE_DERIVED_BGLS: GpuTestConfiguration =
  GpuTestConfiguration::new()
- .parameters(
- TestParameters::default()
- .test_features_limits()
- .expect_fail(
- FailureCase::backend(Backends::DX12).validation_error(DX12_VALIDATION_ERROR),
- ),
- )
+ .parameters(TestParameters::default().test_features_limits())
  .run_sync(separate_programs_have_incompatible_derived_bgls);
 
 fn separate_programs_have_incompatible_derived_bgls(ctx: TestingContext) {
@@ -376,13 +365,7 @@ fn separate_programs_have_incompatible_derived_bgls(ctx: TestingContext) {
 #[gpu_test]
 static DERIVED_BGLS_INCOMPATIBLE_WITH_REGULAR_BGLS: GpuTestConfiguration =
  GpuTestConfiguration::new()
- .parameters(
- TestParameters::default()
- .test_features_limits()
- .expect_fail(
- FailureCase::backend(Backends::DX12).validation_error(DX12_VALIDATION_ERROR),
- ),
- )
+ .parameters(TestParameters::default().test_features_limits())
  .run_sync(derived_bgls_incompatible_with_regular_bgls);
 
 fn derived_bgls_incompatible_with_regular_bgls(ctx: TestingContext) {

tests/tests/encoder.rs

@@ -21,16 +21,9 @@ static DROP_QUEUE_BEFORE_CREATING_COMMAND_ENCODER: GpuTestConfiguration =
  device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
  });
 
-// This test crashes on DX12 with the exception:
-//
-// ID3D12CommandAllocator::Reset: The command allocator cannot be reset because a
-// command list is currently being recorded with the allocator. [ EXECUTION ERROR
-// #543: COMMAND_ALLOCATOR_CANNOT_RESET]
-//
-// For now, we mark the test as failing on DX12.
 #[gpu_test]
 static DROP_ENCODER_AFTER_ERROR: GpuTestConfiguration = GpuTestConfiguration::new()
- .parameters(TestParameters::default().expect_fail(FailureCase::backend(wgpu::Backends::DX12)))
+ .parameters(TestParameters::default())
  .run_sync(|ctx| {
  let mut encoder = ctx
  .device

wgpu-core/src/command/mod.rs

@@ -75,7 +75,7 @@ impl<A: HalApi> CommandEncoder<A> {
  Ok(())
  }
 
- fn discard(&mut self) {
+ pub(crate) fn discard(&mut self) {
  if self.is_open {
  self.is_open = false;
  unsafe { self.raw.discard_encoding() };
@@ -112,7 +112,7 @@ pub(crate) struct DestroyedBufferError(pub id::BufferId);
 pub(crate) struct DestroyedTextureError(pub id::TextureId);
 
 pub struct CommandBufferMutable<A: HalApi> {
- encoder: CommandEncoder<A>,
+ pub(crate) encoder: CommandEncoder<A>,
  status: CommandEncoderStatus,
  pub(crate) trackers: Tracker<A>,
  buffer_memory_init_actions: Vec<BufferInitTrackerAction<A>>,

wgpu-core/src/device/global.rs

@@ -1377,6 +1377,7 @@ impl Global {
  .command_buffers
  .unregister(command_encoder_id.transmute())
  {
+ cmd_buf.data.lock().as_mut().unwrap().encoder.discard();
  cmd_buf
  .device
  .untrack(&cmd_buf.data.lock().as_ref().unwrap().trackers);

wgpu-hal/src/dx12/command.rs

@@ -56,6 +56,13 @@ impl super::Temp {
  }
 }
 
+impl Drop for super::CommandEncoder {
+ fn drop(&mut self) {
+ use crate::CommandEncoder;
+ unsafe { self.discard_encoding() }
+ }
+}
+
 impl super::CommandEncoder {
  unsafe fn begin_pass(&mut self, kind: super::PassKind, label: crate::Label) {
  let list = self.list.as_ref().unwrap();

wgpu-hal/src/dx12/device.rs

@@ -663,11 +663,7 @@ impl crate::Device<super::Api> for super::Device {
  end_of_pass_timer_query: None,
  })
  }
- unsafe fn destroy_command_encoder(&self, encoder: super::CommandEncoder) {
- if let Some(list) = encoder.list {
- list.close();
- }
- }
+ unsafe fn destroy_command_encoder(&self, _encoder: super::CommandEncoder) {}
 
  unsafe fn create_bind_group_layout(
  &self,

wgpu-hal/src/gles/command.rs

@@ -93,6 +93,13 @@ impl super::CommandBuffer {
  }
 }
 
+impl Drop for super::CommandEncoder {
+ fn drop(&mut self) {
+ use crate::CommandEncoder;
+ unsafe { self.discard_encoding() }
+ }
+}
+
 impl super::CommandEncoder {
  fn rebind_stencil_func(&mut self) {
  fn make(s: &super::StencilSide, face: u32) -> C {