Refactor Device destructors to WaitForIdleForDestruction + Destroy
To help with Device Loss, this splits Device backend destructors
to WaitForIdleForDestruction and Destroy.
WaitForIdleForDestruction waits for GPU to finish, checks errors and gets
ready for destruction.
Destroy is used to clean up and release resources used by device,
does not wait for GPU or check errors.
Bug: dawn:68
Change-Id: I054fd735e8d5b289365604209f38e616c723a4e7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14560
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Commit-Queue: Natasha Lee <natlee@microsoft.com>
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 664b81d..9c136be 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -94,6 +94,15 @@
ASSERT(mCaches->shaderModules.empty());
}
+ void DeviceBase::BaseDestructor() {
+ MaybeError err = WaitForIdleForDestruction();
+ if (err.IsError()) {
+ // Assert that errors are device loss so that we can continue with destruction
+ ASSERT(err.AcquireError()->GetType() == wgpu::ErrorType::DeviceLost);
+ }
+ Destroy();
+ }
+
void DeviceBase::HandleError(wgpu::ErrorType type, const char* message) {
mCurrentErrorScope->HandleError(type, message);
}
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index e887fa7..7f22df5 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -192,6 +192,7 @@
protected:
void SetToggle(Toggle toggle, bool isEnabled);
void ApplyToggleOverrides(const DeviceDescriptor* deviceDescriptor);
+ void BaseDestructor();
std::unique_ptr<DynamicUploader> mDynamicUploader;
@@ -251,6 +252,16 @@
void ConsumeError(ErrorData* error);
+ // Destroy is used to clean up and release resources used by device, does not wait for GPU
+ // or check errors.
+ virtual void Destroy() = 0;
+
+ // WaitForIdleForDestruction waits for GPU to finish, checks errors and gets ready for
+ // destruction. This is only used when properly destructing the device. For a real
+ // device loss, this function doesn't need to be called since the driver already closed all
+ // resources.
+ virtual MaybeError WaitForIdleForDestruction() = 0;
+
AdapterBase* mAdapter = nullptr;
Ref<ErrorScope> mRootErrorScope;
diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 204e55c..352fe00 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -17,6 +17,7 @@
#include "common/Assert.h"
#include "dawn_native/BackendConnection.h"
#include "dawn_native/DynamicUploader.h"
+#include "dawn_native/ErrorData.h"
#include "dawn_native/d3d12/AdapterD3D12.h"
#include "dawn_native/d3d12/BackendD3D12.h"
#include "dawn_native/d3d12/BindGroupD3D12.h"
@@ -104,34 +105,7 @@
}
Device::~Device() {
- // Immediately forget about all pending commands
- mPendingCommands.Release();
-
- ConsumedError(NextSerial());
- // Wait for all in-flight commands to finish executing
- ConsumedError(WaitForSerial(mLastSubmittedSerial));
-
- // Call tick one last time so resources are cleaned up. Ignore the return value so we can
- // continue shutting down in an orderly fashion.
- ConsumedError(TickImpl());
-
- // Free services explicitly so that they can free D3D12 resources before destruction of the
- // device.
- mDynamicUploader = nullptr;
-
- // GPU is no longer executing commands. Existing objects do not get freed until the device
- // is destroyed. To ensure objects are always released, force the completed serial to be
- // MAX.
- mCompletedSerial = std::numeric_limits<Serial>::max();
-
- if (mFenceEvent != nullptr) {
- ::CloseHandle(mFenceEvent);
- }
-
- mUsedComObjectRefs.ClearUpTo(mCompletedSerial);
-
- ASSERT(mUsedComObjectRefs.Empty());
- ASSERT(!mPendingCommands.IsOpen());
+ BaseDestructor();
}
ComPtr<ID3D12Device> Device::GetD3D12Device() const {
@@ -417,4 +391,38 @@
SetToggle(Toggle::UseD3D12RenderPass, GetDeviceInfo().supportsRenderPass);
}
+ MaybeError Device::WaitForIdleForDestruction() {
+ DAWN_TRY(NextSerial());
+ // Wait for all in-flight commands to finish executing
+ DAWN_TRY(WaitForSerial(mLastSubmittedSerial));
+
+ // Call tick one last time so resources are cleaned up.
+ DAWN_TRY(TickImpl());
+
+ return {};
+ }
+
+ void Device::Destroy() {
+ // Immediately forget about all pending commands
+ mPendingCommands.Release();
+
+ // Free services explicitly so that they can free D3D12 resources before destruction of the
+ // device.
+ mDynamicUploader = nullptr;
+
+ // GPU is no longer executing commands. Existing objects do not get freed until the device
+ // is destroyed. To ensure objects are always released, force the completed serial to be
+ // MAX.
+ mCompletedSerial = std::numeric_limits<Serial>::max();
+
+ if (mFenceEvent != nullptr) {
+ ::CloseHandle(mFenceEvent);
+ }
+
+ mUsedComObjectRefs.ClearUpTo(mCompletedSerial);
+
+ ASSERT(mUsedComObjectRefs.Empty());
+ ASSERT(!mPendingCommands.IsOpen());
+ }
+
}} // namespace dawn_native::d3d12
diff --git a/src/dawn_native/d3d12/DeviceD3D12.h b/src/dawn_native/d3d12/DeviceD3D12.h
index 2740e03..a29adc9 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.h
+++ b/src/dawn_native/d3d12/DeviceD3D12.h
@@ -127,6 +127,9 @@
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
+ void Destroy() override;
+ MaybeError WaitForIdleForDestruction() override;
+
Serial mCompletedSerial = 0;
Serial mLastSubmittedSerial = 0;
ComPtr<ID3D12Fence> mFence;
diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h
index 4424dc8..2219ab6 100644
--- a/src/dawn_native/metal/DeviceMTL.h
+++ b/src/dawn_native/metal/DeviceMTL.h
@@ -90,6 +90,8 @@
const TextureViewDescriptor* descriptor) override;
void InitTogglesFromDriver();
+ void Destroy() override;
+ MaybeError WaitForIdleForDestruction() override;
id<MTLDevice> mMtlDevice = nil;
id<MTLCommandQueue> mCommandQueue = nil;
diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index 0249df1..6cd73a9 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -18,6 +18,7 @@
#include "dawn_native/BindGroup.h"
#include "dawn_native/BindGroupLayout.h"
#include "dawn_native/DynamicUploader.h"
+#include "dawn_native/ErrorData.h"
#include "dawn_native/metal/BufferMTL.h"
#include "dawn_native/metal/CommandBufferMTL.h"
#include "dawn_native/metal/ComputePipelineMTL.h"
@@ -53,27 +54,7 @@
}
Device::~Device() {
- // Wait for all commands to be finished so we can free resources SubmitPendingCommandBuffer
- // may not increment the pendingCommandSerial if there are no pending commands, so we can't
- // store the pendingSerial before SubmitPendingCommandBuffer then wait for it to be passed.
- // Instead we submit and wait for the serial before the next pendingCommandSerial.
- SubmitPendingCommandBuffer();
- while (GetCompletedCommandSerial() != mLastSubmittedSerial) {
- usleep(100);
- }
- Tick();
-
- [mPendingCommands release];
- mPendingCommands = nil;
-
- mMapTracker = nullptr;
- mDynamicUploader = nullptr;
-
- [mCommandQueue release];
- mCommandQueue = nil;
-
- [mMtlDevice release];
- mMtlDevice = nil;
+ BaseDestructor();
}
void Device::InitTogglesFromDriver() {
@@ -291,4 +272,32 @@
[mLastSubmittedCommands waitUntilScheduled];
}
+ MaybeError Device::WaitForIdleForDestruction() {
+ [mPendingCommands release];
+ mPendingCommands = nil;
+
+ // Wait for all commands to be finished so we can free resources
+ while (GetCompletedCommandSerial() != mLastSubmittedSerial) {
+ usleep(100);
+ }
+ Tick();
+ return {};
+ }
+
+ void Device::Destroy() {
+ if (mPendingCommands != nil) {
+ [mPendingCommands release];
+ mPendingCommands = nil;
+ }
+
+ mMapTracker = nullptr;
+ mDynamicUploader = nullptr;
+
+ [mCommandQueue release];
+ mCommandQueue = nil;
+
+ [mMtlDevice release];
+ mMtlDevice = nil;
+ }
+
}} // namespace dawn_native::metal
diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp
index 872d486..d721751 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -17,6 +17,7 @@
#include "dawn_native/BackendConnection.h"
#include "dawn_native/Commands.h"
#include "dawn_native/DynamicUploader.h"
+#include "dawn_native/ErrorData.h"
#include "dawn_native/Instance.h"
#include <spirv_cross.hpp>
@@ -85,10 +86,7 @@
}
Device::~Device() {
- mDynamicUploader = nullptr;
-
- mPendingOperations.clear();
- ASSERT(mMemoryUsage == 0);
+ BaseDestructor();
}
ResultOrError<BindGroupBase*> Device::CreateBindGroupImpl(
@@ -167,6 +165,17 @@
return std::move(stagingBuffer);
}
+ void Device::Destroy() {
+ mDynamicUploader = nullptr;
+
+ mPendingOperations.clear();
+ ASSERT(mMemoryUsage == 0);
+ }
+
+ MaybeError Device::WaitForIdleForDestruction() {
+ return {};
+ }
+
MaybeError Device::CopyFromStagingToBuffer(StagingBufferBase* source,
uint64_t sourceOffset,
BufferBase* destination,
diff --git a/src/dawn_native/null/DeviceNull.h b/src/dawn_native/null/DeviceNull.h
index 82b37bc..c44e027 100644
--- a/src/dawn_native/null/DeviceNull.h
+++ b/src/dawn_native/null/DeviceNull.h
@@ -130,6 +130,9 @@
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
+ void Destroy() override;
+ MaybeError WaitForIdleForDestruction() override;
+
Serial mCompletedSerial = 0;
Serial mLastSubmittedSerial = 0;
std::vector<std::unique_ptr<PendingOperation>> mPendingOperations;
diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp
index 9c252ef..d44bd52 100644
--- a/src/dawn_native/opengl/DeviceGL.cpp
+++ b/src/dawn_native/opengl/DeviceGL.cpp
@@ -18,6 +18,7 @@
#include "dawn_native/BindGroup.h"
#include "dawn_native/BindGroupLayout.h"
#include "dawn_native/DynamicUploader.h"
+#include "dawn_native/ErrorData.h"
#include "dawn_native/opengl/BufferGL.h"
#include "dawn_native/opengl/CommandBufferGL.h"
#include "dawn_native/opengl/ComputePipelineGL.h"
@@ -42,17 +43,7 @@
}
Device::~Device() {
- CheckPassedFences();
- ASSERT(mFencesInFlight.empty());
-
- // Some operations might have been started since the last submit and waiting
- // on a serial that doesn't have a corresponding fence enqueued. Force all
- // operations to look as if they were completed (because they were).
- mCompletedSerial = mLastSubmittedSerial + 1;
-
- mDynamicUploader = nullptr;
-
- Tick();
+ BaseDestructor();
}
const GLFormat& Device::GetGLFormat(const Format& format) {
@@ -170,4 +161,21 @@
return DAWN_UNIMPLEMENTED_ERROR("Device unable to copy from staging buffer.");
}
+ void Device::Destroy() {
+ // Some operations might have been started since the last submit and waiting
+ // on a serial that doesn't have a corresponding fence enqueued. Force all
+ // operations to look as if they were completed (because they were).
+ mCompletedSerial = mLastSubmittedSerial + 1;
+
+ mDynamicUploader = nullptr;
+ }
+
+ MaybeError Device::WaitForIdleForDestruction() {
+ gl.Finish();
+ CheckPassedFences();
+ ASSERT(mFencesInFlight.empty());
+ Tick();
+ return {};
+ }
+
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/opengl/DeviceGL.h b/src/dawn_native/opengl/DeviceGL.h
index 757f27c..aa97708 100644
--- a/src/dawn_native/opengl/DeviceGL.h
+++ b/src/dawn_native/opengl/DeviceGL.h
@@ -86,6 +86,8 @@
const TextureViewDescriptor* descriptor) override;
void CheckPassedFences();
+ void Destroy() override;
+ MaybeError WaitForIdleForDestruction() override;
Serial mCompletedSerial = 0;
Serial mLastSubmittedSerial = 0;
diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index 232d9b8..1b99814 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -87,77 +87,7 @@
}
Device::~Device() {
- // 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);
-
- VkResult waitIdleResult = fn.QueueWaitIdle(mQueue);
- // Ignore the result of QueueWaitIdle: it can return OOM which we can't really do anything
- // about, Device lost, which means workloads running on the GPU are no longer accessible
- // (so they are as good as waited on) or success.
- DAWN_UNUSED(waitIdleResult);
-
- CheckPassedFences();
-
- // Make sure all fences are complete by explicitly waiting on them all
- while (!mFencesInFlight.empty()) {
- VkFence fence = mFencesInFlight.front().first;
- Serial fenceSerial = mFencesInFlight.front().second;
- ASSERT(fenceSerial > mCompletedSerial);
-
- VkResult result = VK_TIMEOUT;
- do {
- result = fn.WaitForFences(mVkDevice, 1, &fence, true, UINT64_MAX);
- } while (result == VK_TIMEOUT);
- fn.DestroyFence(mVkDevice, fence, nullptr);
-
- mFencesInFlight.pop();
- mCompletedSerial = fenceSerial;
- }
-
- // Some operations might have been started since the last submit and waiting
- // on a serial that doesn't have a corresponding fence enqueued. Force all
- // operations to look as if they were completed (because they were).
- mCompletedSerial = mLastSubmittedSerial + 1;
- Tick();
-
- ASSERT(mCommandsInFlight.Empty());
- for (const CommandPoolAndBuffer& commands : mUnusedCommands) {
- fn.DestroyCommandPool(mVkDevice, commands.pool, nullptr);
- }
- 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());
-
- for (VkFence fence : mUnusedFences) {
- fn.DestroyFence(mVkDevice, fence, nullptr);
- }
- mUnusedFences.clear();
-
- // Free services explicitly so that they can free Vulkan objects before vkDestroyDevice
- mDynamicUploader = nullptr;
- mDescriptorSetService = nullptr;
-
- // Releasing the uploader enqueues buffers to be released.
- // Call Tick() again to clear them before releasing the deleter.
- mDeleter->Tick(mCompletedSerial);
-
- mDeleter = nullptr;
- mMapRequestTracker = nullptr;
-
- // The VkRenderPasses in the cache can be destroyed immediately since all commands referring
- // to them are guaranteed to be finished executing.
- mRenderPassCache = nullptr;
-
- // VkQueues are destroyed when the VkDevice is destroyed
- if (mVkDevice != VK_NULL_HANDLE) {
- fn.DestroyDevice(mVkDevice, nullptr);
- mVkDevice = VK_NULL_HANDLE;
- }
+ BaseDestructor();
}
ResultOrError<BindGroupBase*> Device::CreateBindGroupImpl(
@@ -768,4 +698,81 @@
return mResourceMemoryAllocator.get();
}
+ MaybeError Device::WaitForIdleForDestruction() {
+ VkResult waitIdleResult = fn.QueueWaitIdle(mQueue);
+ // Ignore the result of QueueWaitIdle: it can return OOM which we can't really do anything
+ // about, Device lost, which means workloads running on the GPU are no longer accessible
+ // (so they are as good as waited on) or success.
+ DAWN_UNUSED(waitIdleResult);
+
+ CheckPassedFences();
+
+ // Make sure all fences are complete by explicitly waiting on them all
+ while (!mFencesInFlight.empty()) {
+ VkFence fence = mFencesInFlight.front().first;
+ Serial fenceSerial = mFencesInFlight.front().second;
+ ASSERT(fenceSerial > mCompletedSerial);
+
+ VkResult result = VK_TIMEOUT;
+ do {
+ result = fn.WaitForFences(mVkDevice, 1, &fence, true, UINT64_MAX);
+ } while (result == VK_TIMEOUT);
+ fn.DestroyFence(mVkDevice, fence, nullptr);
+
+ mFencesInFlight.pop();
+ mCompletedSerial = fenceSerial;
+ }
+ return {};
+ }
+
+ void Device::Destroy() {
+ // 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);
+
+ // Some operations might have been started since the last submit and waiting
+ // on a serial that doesn't have a corresponding fence enqueued. Force all
+ // operations to look as if they were completed (because they were).
+ mCompletedSerial = mLastSubmittedSerial + 1;
+ Tick();
+
+ ASSERT(mCommandsInFlight.Empty());
+ for (const CommandPoolAndBuffer& commands : mUnusedCommands) {
+ fn.DestroyCommandPool(mVkDevice, commands.pool, nullptr);
+ }
+ 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());
+
+ for (VkFence fence : mUnusedFences) {
+ fn.DestroyFence(mVkDevice, fence, nullptr);
+ }
+ mUnusedFences.clear();
+
+ // Free services explicitly so that they can free Vulkan objects before vkDestroyDevice
+ mDynamicUploader = nullptr;
+ mDescriptorSetService = nullptr;
+
+ // Releasing the uploader enqueues buffers to be released.
+ // Call Tick() again to clear them before releasing the deleter.
+ mDeleter->Tick(mCompletedSerial);
+
+ mDeleter = nullptr;
+ mMapRequestTracker = nullptr;
+
+ // The VkRenderPasses in the cache can be destroyed immediately since all commands referring
+ // to them are guaranteed to be finished executing.
+ mRenderPassCache = nullptr;
+
+ // VkQueues are destroyed when the VkDevice is destroyed
+ if (mVkDevice != VK_NULL_HANDLE) {
+ fn.DestroyDevice(mVkDevice, nullptr);
+ mVkDevice = VK_NULL_HANDLE;
+ }
+ }
+
}} // namespace dawn_native::vulkan
diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h
index 13eb152..79c9d65 100644
--- a/src/dawn_native/vulkan/DeviceVk.h
+++ b/src/dawn_native/vulkan/DeviceVk.h
@@ -130,6 +130,9 @@
void InitTogglesFromDriver();
void ApplyDepth24PlusS8Toggle();
+ void Destroy() override;
+ MaybeError WaitForIdleForDestruction() override;
+
// To make it easier to use fn it is a public const member. However
// the Device is allowed to mutate them through these private methods.
VulkanFunctions* GetMutableFunctions();
