Destroying resources before the associated command buffer is finished causes vulkan validation errors

[andyleiserson]

Got this failure:

    [2025-06-26T23:56:59Z ERROR wgpu_test::expectations] Validation Error: vkEndCommandBuffer(): was called in VkCommandBuffer 0x7f9731d77530 which is invalid because the bound VkImage 0x160000000016 was destroyed.
        The Vulkan spec states: commandBuffer must be in the recording state (https://vulkan.lunarg.com/doc/view/1.4.313.0/linux/antora/spec/latest/chapters/cmdbuffers.html#VUID-vkEndCommandBuffer-commandBuffer-00059)

with the test in #7853.

The test is doing something that is invalid, but our error handling needs to conform to the spec.

[teoxoy]

Looking at https://registry.khronos.org/vulkan/specs/latest/html/vkspec.html#commandbuffers-lifecycle this seems consistent with the spec.

Looking at some of the vkCmd* commands listed in the "see also" section here, they all seem to have the following requirement:

"commandBuffer must be in the recording state"

This means that after we destroy any resource already recorded in a command buffer, we can only free or reset the command buffer.
cc @jimblandy @cwfitzgerald

[teoxoy]

Resolving this would also resolve the issue we were seeing on Metal since it seems Vulkan and Metal have the same requirements here.

As for D3D12, the only thing I could find is:

Most D3D12 APIs continue to use reference counting following COM conventions. A notable exception to this is the D3D12 graphics command list APIs. All APIs on ID3D12GraphicsCommandList do not hold references to the objects passed into those APIs. This means applications are responsible for ensuring that a command list is never submitted for execution that references a destroyed resource.

from https://learn.microsoft.com/en-us/windows/win32/direct3d12/recording-command-lists-and-bundles#reference-counting

Which implies that it's ok to destroy resources encoded in a command list as long as the command list is not submitted.

Given the situation we should probably make this a wgpu-hal requirement ("all resources encoded in a command buffer must outlive the command buffer") since it's easier to uphold this in wgpu-core where we already have trackers in command buffers holding the resources alive; we would only have to fix .destroy() methods.

[jimblandy]

Since both Vulkan and Direct3D allow destroying resources while unsubmitted command buffers still refer to them, and Metal can be asked politely to hold them alive itself, it seems a pity to require wgpu-core to hold them alive on all platforms. What would be entailed in allowing wgpu-core to destroy hal resources used by unsubmitted command buffers?

Filed #7859 to make sure we document whatever it is we settle on.

[teoxoy]

What would be entailed in allowing wgpu-core to destroy hal resources used by unsubmitted command buffers?

The trouble is that Vulkan doesn't allow calling vkCmd* commands on invalid command buffers, given that command buffers become invalid as soon as a resource that was encoded in them gets destroyed we would need a data structure to remember which command buffers have encoded resource X; wgpu-core's trackers already have this data though.

So, while Vulkan allows destroying resources while unsubmitted command buffers still refer to them, the command buffers become invalid and from that point on, one can only reset them or destroy them which has very similar consequences to encoded resources needing to outlive command buffers.

[teoxoy]

I think we can solve this by deferring actual resource destruction to maintain(), have the Device hold weak refs to CommandBuffers, then in maintain() we go over (pending) command buffers and destroy the HAL command buffer if we see that a resource was also marked for destruction.

In the common case we will only have to destroy the resources marked for destruction since it's highly likely that there will be no (pending) command buffers.

The only downside would be that destruction is delayed but this could be improved with a simple check in resource.destroy(), that checks if there are any command buffers alive, if there aren't, destroy the resource immediately.

[jimblandy]

Okay, I think I see the problem better now: even on Vulkan, if we want to destroy the hal resources early, we must ensure that we do nothing more with any hal command buffers that refer to them. Destroying the hal resources promptly would entail finding all command buffers that refer to them and preventing any further hal operations on them.

I think we cannot make the assumption that there will usually be no pending command buffers. While that's true for today's WebGPU, the recommended usage pattern for the platform APIs, and thus for wgpu, is to record command buffers in multiple threads, and then just submit on the main thread. Eventually WebGPU will support this pattern too.

I would be more comfortable assuming that unsubmitted command buffers rarely refer to resources being destroyed. If they did, that would mean that the application is wasting work recording command buffers that it can't submit. If the application is poorly coded anyway, then less-than ideal performance on our part probably won't be noticed.

[jimblandy]

I'm inclined towards this solution:

Given the situation we should probably make this a wgpu-hal requirement ("all resources encoded in a command buffer must outlive the command buffer") since it's easier to uphold this in wgpu-core where we already have trackers in command buffers holding the resources alive; we would only have to fix .destroy() methods.

[andyleiserson]

I think we cannot make the assumption that there will usually be no pending command buffers.

I would be more comfortable assuming that unsubmitted command buffers rarely refer to resources being destroyed.

I'm not sure that this assumption helps very much in making a solution cheaper. We somehow have to determine whether a resource that is proposed to be destroyed is referenced by any unsubmitted command buffers. Even if we know the answer is likely negative, we still need to do some amount of work to be sure of that. The assumption that there are few or no pending command buffers at a particular time helps, because then we can assume that work proportional to the number of resources referenced by pending command buffers is relatively small.

Trying to capture some options we've discussed and some others I've brainstormed:

Option: @teoxoy's suggestion to destroy resources in maintain. Delays destruction. Relatively simple and cheap if we assume there are few unsubmitted command buffers at the time of maintain.

Option: Hold a set of Weak<CommandBuffer>s in each buffer/texture resource. Immediately invalidate pending command buffers when a resource they use is invalidated. This maintains the ability to destroy a resource immediately. Increased overhead and complexity to maintain the additional state.

Option: Introduce some kind of reference counting that tracks the number of unsubmitted command buffers referencing a resource (i.e. the size of the set that the previous option would maintain). Either leverage the existing Arc reference counting, e.g. assume that if Arc::strong_count is 1, then it is safe to destroy the resource, or introduce some new kind of resource handle that presumably wraps Arc and also keeps track of different reference types.

Option: Walk all the resources referenced by the command encoder to check if they are destroyed before attempting to finalize the encoder (similar to what we do at submit time). This seems like it could be expensive, but it's also pretty easy, so maybe it's worth trying it and assessing the performance impact.

Option: Defer encoding of commands until finish (#7679). We'd do the same work we do today, but the window between encoding and finalization where destruction is a problem mostly goes away. Would need to confirm that locking prevents destruction during the actual finish process.