KhronosGroup / KhronosGroup/Vulkan-ValidationLayers

Suspected race condition between `CommandBuffer::NotifyInvalidate` and `CommandBuffer::ResetCBState`

Open
#8,074 2 comments 0 reactions 0 assignees View on GitHub
Bug Threading
Dominant language
C++
Stars
1k
Forks
504
Avg merge
11h 35m
Merged PRs (30d)
224

Description

**Environment:**
- OS: Linux
- GPU and driver version: Tested on Nvidia driver 535.129.03
- SDK or header version if building from repo: Tested on 04e24fb800f17905619c106e3ab15f32ad829748
- Options enabled (synchronization, best practices, etc.): None

I'm hitting what I suspect is a race condition between `CommandBuffer::NotifyInvalidate` and `CommandBuffer::ResetCBState`. I spuriously get errors like the following:

```
Validation Error: [ VUID-vkQueueSubmit-pCommandBuffers-00070 ] Object 0: handle = 0x9e500000009e5, type = VK_OBJECT_TYPE_IMAGE_VIEW; Object 1: handle = 0x52c000000052c, type = VK_OBJECT_TYPE_DESCRIPTOR_SET; Object 2: handle = 0x308b791bd2d0, type = VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0xaeecdc0b | vkQueueSubmit(): pSubmits[0].pCommandBuffers[5] was called in VkCommandBuffer 0x308b791bd2d0[] which is invalid because bound VkImageView 0x9e500000009e5[] was destroyed. The Vulkan spec states: Each element of the pCommandBuffers member of each element of pSubmits must be in the pending or executable state (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkQueueSubmit-pCommandBuffers-00070)
```

In my setup, both command buffer and descriptor pool handles are reused between frames, but are updated before the next call to `vkQueueSubmit`. The problem appeared when I started reusing descriptor sets, instead of freeing and re-allocating them every time.

What happens is that after a command buffer has executed, any resources it uses are either returned to be reused or outright destroyed on a cleanup thread. This includes the command buffer itself, the descriptor set and any images and buffers. Once the command buffer has been marked as ready for reuse, it may immediately be picked up by another thread for recording. Since a command buffer (and its pools) can only be modified from one thread at a time, it is the thread that does the writing that also resets the command buffer.

I've tried for a couple of days to track down exactly what is going wrong but am unable to figure it out. I am, however, able to greatly increase the occurence of this error by adding a usleep(1000) to `CommandBuffer::NotifyInvalidate` (see details section).

In a run of ~3300 tests, all running for multiple frames, this change increases the number of occurrences from 31 to 41357. I would not expect that `NotifyInvalidate` being slow should alter its behavior. There's also a comment in it that mentions a possible race condition.

I've tried to create a reproduction case, but have not been able to. I may try again when I have some more time. I hope this report is still useful.

**Valid Usage ID**
Mainly:
- VUID-vkQueueSubmit-pCommandBuffers-00070

Sometimes:
- VUID-vkCmdDispatch-None-08114
- VUID-vkCmdPipelineBarrier-commandBuffer-recording (noticed after adding the `usleep(1000)`)
- VUID-vkEndCommandBuffer-commandBuffer-00059 (noticed after adding the `usleep(1000)`)

**Additional context**

Modified NotifyInvalidate

```c++
void CommandBuffer::NotifyInvalidate(const StateObject::NodeList &invalid_nodes, bool unlink) {
{
auto guard = WriteLock();
assert(!invalid_nodes.empty());
// Save all of the vulkan handles between the command buffer and the now invalid node
LogObjectList log_list;
for (auto &obj : invalid_nodes) {
log_list.add(obj->Handle());
}

bool found_invalid = false;
for (auto &obj : invalid_nodes) {
// Only record a broken binding if one of the nodes in the invalid chain is still
// being tracked by the command buffer. This is to try to avoid race conditions
// caused by separate CommandBuffer and StateObject::parent_nodes locking.
if (object_bindings.erase(obj)) {
obj->RemoveParent(this);
found_invalid = true;
/* added -> */ usleep(1000);
}
switch (obj->Type()) {
case kVulkanObjectTypeCommandBuffer:
if (unlink) {
linkedCommandBuffers.erase(static_cast(obj.get()));
}
break;
case kVulkanObjectTypeImage:
if (unlink) {
image_layout_map.erase(obj->Handle().Cast());
}
break;
default:
break;
}
}
if (found_invalid) {
if (state == CbState::Recording) {
state = CbState::InvalidIncomplete;
} else if (state == CbState::Recorded) {
state = CbState::InvalidComplete;
}
broken_bindings.emplace(invalid_nodes[0]->Handle(), log_list);
}
}
StateObject::NotifyInvalidate(invalid_nodes, unlink);
}
```

Contributor guide

Open the contributing guide

Research direction

Start by reading CommandBuffer::NotifyInvalidate and CommandBuffer::ResetCBState, focusing on their locking and command-buffer state transitions. Reproduce the issue with descriptor-set reuse and the added usleep(1000) stress condition; done means the reported validation errors no longer occur during repeated multi-frame runs without introducing the other command-buffer errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.