From d0c07f112eb2038891b62d529d1d841e06673b67 Mon Sep 17 00:00:00 2001 From: Jiajie Hu Date: Mon, 18 Nov 2019 08:15:18 +0000 Subject: [PATCH] Vulkan: Fix release of semaphores in the recording context This seems to be a regression introduced by CL:12022, where the previously recorded semaphores are leaked while resetting the recording context. The Vulkan validation layers will complain about this when running the Linux-only image wrapping tests. Also, the semaphores are queued for deletion with the right serial now. Bug: dawn:19, dawn:150 Change-Id: I50fd46ca9de9199b29be2f85d5e9bd7394a10f1a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/13420 Reviewed-by: Corentin Wallez Reviewed-by: Kai Ninomiya Commit-Queue: Jiajie Hu --- src/dawn_native/vulkan/DeviceVk.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 7f895bd474..677ae87f27 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -123,6 +123,9 @@ namespace dawn_native { namespace vulkan { } mUnusedCommands.clear(); + // TODO(jiajie.hu@intel.com): In rare cases, a DAWN_TRY() failure may leave semaphores + // untagged for deletion. But for most of the time when everything goes well, these + // assertions can be helpful in catching bugs. ASSERT(mRecordingContext.waitSemaphores.empty()); ASSERT(mRecordingContext.signalSemaphores.empty()); @@ -311,6 +314,15 @@ namespace dawn_native { namespace vulkan { DAWN_TRY_ASSIGN(fence, GetUnusedFence()); DAWN_TRY(CheckVkSuccess(fn.QueueSubmit(mQueue, 1, &submitInfo, fence), "vkQueueSubmit")); + // Enqueue the semaphores before incrementing the serial, so that they can be deleted as + // soon as the current submission is finished. + for (VkSemaphore semaphore : mRecordingContext.waitSemaphores) { + mDeleter->DeleteWhenUnused(semaphore); + } + for (VkSemaphore semaphore : mRecordingContext.signalSemaphores) { + mDeleter->DeleteWhenUnused(semaphore); + } + mLastSubmittedSerial++; mFencesInFlight.emplace(fence, mLastSubmittedSerial); @@ -320,14 +332,6 @@ namespace dawn_native { namespace vulkan { mRecordingContext = CommandRecordingContext(); DAWN_TRY(PrepareRecordingContext()); - for (VkSemaphore semaphore : mRecordingContext.waitSemaphores) { - mDeleter->DeleteWhenUnused(semaphore); - } - - for (VkSemaphore semaphore : mRecordingContext.signalSemaphores) { - mDeleter->DeleteWhenUnused(semaphore); - } - return {}; }