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>
This commit is contained in:
Austin Eng 2022-09-14 18:13:17 +00:00 committed by Dawn LUCI CQ
parent e09da52837
commit 945e2642d6
1 changed files with 11 additions and 6 deletions

View File

@ -137,12 +137,16 @@ ShaderModule::ModuleAndSpirv ShaderModule::ConcurrentTransformedShaderModuleCach
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 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
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.");