Fix lock-order-inversion
Device[Vk]::ReduceMemoryUsageImpl() was holding the lock for both
FencedDeleter and ResourceMemoryAllocator at the same time due to
temporary values in std::max() expression. They were acquired in the
opposite order to when ResourceMemoryAllocator enques deletion in
FencedDeleter. Avoid holdin the locks at the same time in
ReduceMemoryUsageImpl() to prevent this.
Bug: 405422523
Change-Id: Ic046888db7234dbf74b15815537972381aa52376
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/232394
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Kyle Charbonneau <kylechar@google.com>
diff --git a/src/dawn/native/vulkan/DeviceVk.cpp b/src/dawn/native/vulkan/DeviceVk.cpp
index 0e82f18..f6112b7 100644
--- a/src/dawn/native/vulkan/DeviceVk.cpp
+++ b/src/dawn/native/vulkan/DeviceVk.cpp
@@ -1041,9 +1041,13 @@
}
bool Device::ReduceMemoryUsageImpl() {
- ExecutionSerial deletionSerial =
- std::max(GetFencedDeleter()->GetLastPendingDeletionSerial(),
- GetResourceMemoryAllocator()->GetLastPendingDeletionSerial());
+ auto GetLastPendingDeletionSerial = [this]() {
+ // Only hold the lock for one of these objects at a time to avoid lock-order-inversion.
+ auto deleterSerial = GetFencedDeleter()->GetLastPendingDeletionSerial();
+ auto allocatorSerial = GetResourceMemoryAllocator()->GetLastPendingDeletionSerial();
+ return std::max(deleterSerial, allocatorSerial);
+ };
+ ExecutionSerial deletionSerial = GetLastPendingDeletionSerial();
if (deletionSerial == kBeginningOfGPUTime) {
// Nothing pending deletion.
@@ -1068,9 +1072,7 @@
DAWN_ASSERT(deletionSerial <= queue->GetLastSubmittedCommandSerial());
// Check again if there is anything left to delete as tick might have deleted objects.
- return std::max(GetFencedDeleter()->GetLastPendingDeletionSerial(),
- GetResourceMemoryAllocator()->GetLastPendingDeletionSerial()) !=
- kBeginningOfGPUTime;
+ return GetLastPendingDeletionSerial() != kBeginningOfGPUTime;
}
void Device::PerformIdleTasksImpl() {
