From 7840e75bf7c6ba548c41a9969b9ceeabb53dc026 Mon Sep 17 00:00:00 2001
From: Luna <26054772+villuna@users.noreply.github.com>
Date: Tue, 23 Apr 2024 05:39:08 +1000
Subject: [PATCH] Add check to ensure
 `vulkan::CommandEncoder::discard_encoding` is not called multiple times in a
 row (#5557)

Document that `wgpu_hal::CommandEncoder::discard_encoding` must not be called multiple times.

Assert in `wgpu_hal::vulkan::CommandEncoder::discard_encoding` that encoding is actually in progress.

Fixes #5255.
---
 CHANGELOG.md                   | 1 +
 wgpu-hal/src/lib.rs            | 6 +++++-
 wgpu-hal/src/vulkan/command.rs | 5 +++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e7538ed4b2..7b9fdf7783 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -183,6 +183,7 @@ Bottom level categories:
 #### Vulkan
 
 - Set object labels when the DEBUG flag is set, even if the VALIDATION flag is disabled. By @DJMcNab in [#5345](https://github.com/gfx-rs/wgpu/pull/5345).
+- Add safety check to `wgpu_hal::vulkan::CommandEncoder` to make sure `discard_encoding` is not called in the closed state. By @villuna in [#5557](https://github.com/gfx-rs/wgpu/pull/5557)
 
 #### Metal
 
diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs
index 1aff081606..8fffca015d 100644
--- a/wgpu-hal/src/lib.rs
+++ b/wgpu-hal/src/lib.rs
@@ -645,7 +645,7 @@ pub trait CommandEncoder: WasmNotSendSync + fmt::Debug {
     /// This `CommandEncoder` must be in the "closed" state.
     unsafe fn begin_encoding(&mut self, label: Label) -> Result<(), DeviceError>;
 
-    /// Discard the command list under construction, if any.
+    /// Discard the command list under construction.
     ///
     /// If an error has occurred while recording commands, this
     /// is the only safe thing to do with the encoder.
@@ -655,6 +655,10 @@ pub trait CommandEncoder: WasmNotSendSync + fmt::Debug {
     /// # Safety
     ///
     /// This `CommandEncoder` must be in the "recording" state.
+    ///
+    /// Callers must not assume that implementations of this
+    /// function are idempotent, and thus should not call it
+    /// multiple times in a row.
     unsafe fn discard_encoding(&mut self);
 
     /// Return a fresh [`CommandBuffer`] holding the recorded commands.
diff --git a/wgpu-hal/src/vulkan/command.rs b/wgpu-hal/src/vulkan/command.rs
index 43a2471954..ceb44dfbe6 100644
--- a/wgpu-hal/src/vulkan/command.rs
+++ b/wgpu-hal/src/vulkan/command.rs
@@ -104,6 +104,11 @@ impl crate::CommandEncoder for super::CommandEncoder {
     }
 
     unsafe fn discard_encoding(&mut self) {
+        // Safe use requires this is not called in the "closed" state, so the buffer
+        // shouldn't be null. Assert this to make sure we're not pushing null
+        // buffers to the discard pile.
+        assert_ne!(self.active, vk::CommandBuffer::null());
+
         self.discarded.push(self.active);
         self.active = vk::CommandBuffer::null();
     }