From 9d911d49576d7409905d00632ec91a2e80d5008f Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Fri, 4 Jun 2021 05:36:46 +0000 Subject: [PATCH] [Vulkan] Free bind group layouts pending deallocation on shut down Bug: 1212385 Change-Id: I093f9e5bb91f697939f1fbb286f284b54f3d7c02 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/52521 Reviewed-by: Corentin Wallez Reviewed-by: Jiawei Shao Commit-Queue: Jiawei Shao Auto-Submit: Austin Eng --- src/dawn_native/vulkan/DeviceVk.cpp | 11 ++++++-- src/tests/end2end/DeviceLostTests.cpp | 36 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index 748b2c490b..9e8ad22134 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/src/dawn_native/vulkan/DeviceVk.cpp @@ -919,10 +919,17 @@ namespace dawn_native { namespace vulkan { } mUnusedFences.clear(); + ExecutionSerial completedSerial = GetCompletedCommandSerial(); + for (Ref& bgl : + mBindGroupLayoutsPendingDeallocation.IterateUpTo(completedSerial)) { + bgl->FinishDeallocation(completedSerial); + } + mBindGroupLayoutsPendingDeallocation.ClearUpTo(completedSerial); + // Releasing the uploader enqueues buffers to be released. // Call Tick() again to clear them before releasing the deleter. - mResourceMemoryAllocator->Tick(GetCompletedCommandSerial()); - mDeleter->Tick(GetCompletedCommandSerial()); + mResourceMemoryAllocator->Tick(completedSerial); + mDeleter->Tick(completedSerial); // Allow recycled memory to be deleted. mResourceMemoryAllocator->DestroyPool(); diff --git a/src/tests/end2end/DeviceLostTests.cpp b/src/tests/end2end/DeviceLostTests.cpp index eb59ca0ce5..b27c499a78 100644 --- a/src/tests/end2end/DeviceLostTests.cpp +++ b/src/tests/end2end/DeviceLostTests.cpp @@ -464,6 +464,42 @@ TEST_P(DeviceLostTest, DeviceLostBeforeCreatePipelineAsyncCallback) { SetCallbackAndLoseForTesting(); } +// This is a regression test for crbug.com/1212385 where Dawn didn't clean up all +// references to bind group layouts such that the cache was non-empty at the end +// of shut down. +TEST_P(DeviceLostTest, FreeBindGroupAfterDeviceLossWithPendingCommands) { + wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::Storage}}); + + wgpu::BufferDescriptor bufferDesc; + bufferDesc.size = sizeof(float); + bufferDesc.usage = wgpu::BufferUsage::Storage; + wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); + + wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, sizeof(float)}}); + + // Advance the pending command serial. We only a need a couple of these to repro the bug, + // but include extra so this does not become a change-detecting test if the specific serial + // value is sensitive. + queue.Submit(0, nullptr); + queue.Submit(0, nullptr); + queue.Submit(0, nullptr); + queue.Submit(0, nullptr); + queue.Submit(0, nullptr); + queue.Submit(0, nullptr); + + SetCallbackAndLoseForTesting(); + + // Releasing the bing group places the bind group layout into a queue in the Vulkan backend + // for recycling of descriptor sets. So, after these release calls there is still one last + // reference to the BGL which wouldn't be freed until the pending serial passes. + // Since the device is lost, destruction will clean up immediately without waiting for the + // serial. The implementation needs to be sure to clear these BGL references. At the end of + // Device shut down, we ASSERT that the BGL cache is empty. + bgl = nullptr; + bg = nullptr; +} + DAWN_INSTANTIATE_TEST(DeviceLostTest, D3D12Backend(), MetalBackend(),