From 4b7ca6bf96a7e397be459bba6ca3d90127afdb48 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Fri, 17 Jul 2020 15:26:56 +0000 Subject: [PATCH] Call vkFreeCommandBuffers before destroying the vkCommandPool This should not be necessary as the command pool should own the memory if it was created without the creation flag VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT. However, some drivers leak memory if vkFreeCommandBuffers is not called. Bug: chromium:1082181 Change-Id: Ia437fc17b2a304a248b9227b43cfff1868f9f10e Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/25062 Commit-Queue: Austin Eng Reviewed-by: Stephen White --- src/dawn_native/vulkan/DeviceVk.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 7729e8a1dd..53538a8ee8 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -852,7 +852,15 @@ namespace dawn_native { namespace vulkan { // Immediately tag the recording context as unused so we don't try to submit it in Tick. mRecordingContext.used = false; - fn.DestroyCommandPool(mVkDevice, mRecordingContext.commandPool, nullptr); + if (mRecordingContext.commandPool != VK_NULL_HANDLE) { + // The VkCommandBuffer memory should be wholly owned by the pool and freed when it is + // destroyed, but that's not the case in some drivers and the leak memory. + // So we call FreeCommandBuffers before DestroyCommandPool to be safe. + // TODO(enga): Only do this on a known list of bad drivers. + fn.FreeCommandBuffers(mVkDevice, mRecordingContext.commandPool, 1, + &mRecordingContext.commandBuffer); + fn.DestroyCommandPool(mVkDevice, mRecordingContext.commandPool, nullptr); + } for (VkSemaphore semaphore : mRecordingContext.waitSemaphores) { fn.DestroySemaphore(mVkDevice, semaphore, nullptr); @@ -866,6 +874,11 @@ namespace dawn_native { namespace vulkan { ASSERT(mCommandsInFlight.Empty()); for (const CommandPoolAndBuffer& commands : mUnusedCommands) { + // The VkCommandBuffer memory should be wholly owned by the pool and freed when it is + // destroyed, but that's not the case in some drivers and the leak memory. + // So we call FreeCommandBuffers before DestroyCommandPool to be safe. + // TODO(enga): Only do this on a known list of bad drivers. + fn.FreeCommandBuffers(mVkDevice, commands.pool, 1, &commands.commandBuffer); fn.DestroyCommandPool(mVkDevice, commands.pool, nullptr); } mUnusedCommands.clear(); @@ -899,4 +912,4 @@ namespace dawn_native { namespace vulkan { mVkDevice = VK_NULL_HANDLE; } -}} // namespace dawn_native::vulkan \ No newline at end of file +}} // namespace dawn_native::vulkan