Fix race with FencedDeleter and VkShaderModule labels in async creation
FencedDeleter does not need to be used at all since we were deleting
a yet-to-be-used VkShaderModule.
Also, set the VkShaderModule label before cache deduplication since to
avoid a race where the VkShaderModule is in use by another thread.
The other thread may also be setting the label, or using it in a
pipeline creation.
Bug: dawn:1539
Change-Id: I5e3d7ce214c4c089c9cc3272f373aa8233017965
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/102105
Commit-Queue: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 0ec0092..752338e 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -137,12 +137,16 @@
std::lock_guard<std::mutex> lock(mMutex);
auto iter = mTransformedShaderModuleCache.find(key);
if (iter == mTransformedShaderModuleCache.end()) {
- mTransformedShaderModuleCache.emplace(key, std::make_pair(module, std::move(spirv)));
+ bool added = false;
+ std::tie(iter, added) =
+ mTransformedShaderModuleCache.emplace(key, std::make_pair(module, std::move(spirv)));
+ ASSERT(added);
} else {
- mDevice->GetFencedDeleter()->DeleteWhenUnused(module);
+ // No need to use FencedDeleter since this shader module was just created and does
+ // not need to wait for queue operations to complete.
+ // Also, use of fenced deleter here is not thread safe.
+ mDevice->fn.DestroyShaderModule(mDevice->GetVkDevice(), module, nullptr);
}
- // Now the key should exist in the map, so find it again and return it.
- iter = mTransformedShaderModuleCache.find(key);
return ModuleAndSpirv{
iter->second.first,
iter->second.second.Code(),
@@ -371,12 +375,13 @@
if (BlobCache* cache = device->GetBlobCache()) {
cache->EnsureStored(spirv);
}
+ // Set the label on `newHandle` now, and not on `moduleAndSpirv.module` later
+ // since `moduleAndSpirv.module` may be in use by multiple threads.
+ SetDebugName(ToBackend(GetDevice()), newHandle, "Dawn_ShaderModule", GetLabel());
moduleAndSpirv =
mTransformedShaderModuleCache->AddOrGet(cacheKey, newHandle, spirv.Acquire());
}
- SetDebugName(ToBackend(GetDevice()), moduleAndSpirv.module, "Dawn_ShaderModule", GetLabel());
-
return std::move(moduleAndSpirv);
#else
return DAWN_INTERNAL_ERROR("TINT_BUILD_SPV_WRITER is not defined.");
