Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[capture] Thread protect for create wrappers and remove wrappers #1189

Closed
locke-lunarg opened this issue Jul 8, 2023 · 1 comment
Closed
Labels
bug Something isn't working capture Issue with capture P1 Prevents an important capture from being replayed

Comments

@locke-lunarg
Copy link
Contributor

locke-lunarg commented Jul 8, 2023

VulkanStateTableBase::mutex_ on InsertEntry and RemoveEntry can't protect wrappers. The thread protect need more wide. Because DestroyBuffer and RemoveWrapper are different functions. CreateBuffer and CreateWrappedHandle could happen between DestoryBuffer and RemoveWrapper.

Process

  1. GetDeviceTable(device)->DestroyBuffer(device, buffer, pAllocator)
  2. CreateBuffer -> Another thread. Create a duplicated handle value.
  3. CreateWrappedHandle -> The new wrapper can't be added to the map because the old one hadn't removed from the map. It prints a warning, like Create a duplicated Handle: 2230487727664. This wrapper can't be written into VulkanStateHandleTable.
  4. DestroyWrappedHandle(buffer)

Now, the map doesn't have the new wrapper. It could get a warning, like GetWrappedId() couldn't find Handle: 2230487727664's wrapper. It might have been destroyed in the following process.

The thread protect should cover between DestroyBuffer and RemoveWrapper, not only RemoveEntry.

CreateBuffer also need protect to ensurn BufferWrapper is alive.

Here is a sample code:
Using vkcube to run CreateBuffer and DestroyBuffer 10k times on different threads.
https://github.com/locke-lunarg/Vulkan-Tools/tree/thread

Thread protect for CreateBuffer and DestroyBuffer. We won't see the warning and crashes. If you remove two std::unique_lock<std::mutex> lock(buffer_wrapper_lock_), you will see the warning or crashes. You might also see InsertEntry Buffer: 1745362285408, time: 1689026056657 is before RemoveEntry Buffer: 1745362285408, time: 1689026056657.
https://github.com/locke-lunarg/gfxreconstruct/tree/test-thread

@locke-lunarg
Copy link
Contributor Author

We could use GFXRECON_FORCE_COMMAND_SERIALIZATION to fix it before we fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working capture Issue with capture P1 Prevents an important capture from being replayed
Projects
None yet
Development

No branches or pull requests

2 participants