commit 4aca14ccb6b4588d0e12a099d7281d19ef717776
author Natasha Lee <natlee@microsoft.com> Wed Jun 10 14:45:20 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed Jun 10 14:45:20 2020 +0000
tree bd456254fd91b64248191e3ba463261e6daf01ce
parent 527080ff03ce4541a12c2648b6a44e53a1d6384e

Unify TickImpl called during Device shutdown.

Bug: dawn:400
Change-Id: I26ad1a739d123ebc81a2e7d309c8a42f517be06c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22662
Commit-Queue: Natasha Lee <natlee@microsoft.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Device.cpp

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 1f41745..01fc874 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -165,6 +165,9 @@
         mDynamicUploader = nullptr;
         mMapRequestTracker = nullptr;
 
+        // call TickImpl once last time to clean up resources
+        // assert the errors are device loss so we can continue with destruction
+        AssertAndIgnoreDeviceLossError(TickImpl());
         AssumeCommandsComplete();
         // Tell the backend that it can free all the objects now that the GPU timeline is empty.
         ShutDownImpl();
@@ -186,6 +189,7 @@
             // Assert that errors are device losses so that we can continue with destruction.
             // Assume all commands are complete after WaitForIdleForDestruction (because they were)
             AssertAndIgnoreDeviceLossError(WaitForIdleForDestruction());
+            AssertAndIgnoreDeviceLossError(TickImpl());
             AssumeCommandsComplete();
             ASSERT(mFutureCallbackSerial <= mCompletedSerial);
             mState = State::Disconnected;

src/dawn_native/d3d12/DeviceD3D12.cpp

diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 8026bb9..a32b77c 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -459,8 +459,6 @@
         // Wait for all in-flight commands to finish executing
         DAWN_TRY(WaitForSerial(GetLastSubmittedCommandSerial()));
 
-        // Call tick one last time so resources are cleaned up.
-        DAWN_TRY(TickImpl());
         return {};
     }
 
@@ -512,13 +510,15 @@
     void Device::ShutDownImpl() {
         ASSERT(GetState() == State::Disconnected);
 
-        // Immediately forget about all pending commands
+        // Immediately forget about all pending commands for the case where device is lost on its
+        // own and WaitForIdleForDestruction isn't called.
         mPendingCommands.Release();
 
         if (mFenceEvent != nullptr) {
             ::CloseHandle(mFenceEvent);
         }
 
+        // We need to handle clearing up com object refs that were enqeued after TickImpl
         mUsedComObjectRefs.ClearUpTo(GetCompletedCommandSerial());
 
         ASSERT(mUsedComObjectRefs.Empty());

src/dawn_native/metal/DeviceMTL.mm

diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index 08431b9..34b66fc 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -292,8 +292,6 @@
             CheckPassedSerials();
         }
 
-        DAWN_TRY(TickImpl());
-
         return {};
     }
 

src/dawn_native/null/DeviceNull.cpp

diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp
index 59f87ff..fb11d59 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -185,6 +185,7 @@
     }
 
     MaybeError Device::WaitForIdleForDestruction() {
+        mPendingOperations.clear();
         return {};
     }
 

src/dawn_native/opengl/DeviceGL.cpp

diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp
index 3f852b3..6550b48 100644
--- a/src/dawn_native/opengl/DeviceGL.cpp
+++ b/src/dawn_native/opengl/DeviceGL.cpp
@@ -200,7 +200,6 @@
         gl.Finish();
         CheckPassedSerials();
         ASSERT(mFencesInFlight.empty());
-        Tick();
 
         return {};
     }

src/dawn_native/vulkan/DeviceVk.cpp

diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index 9c56c5e..bd3ba34 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -718,6 +718,16 @@
     }
 
     MaybeError Device::WaitForIdleForDestruction() {
+        // Immediately tag the recording context as unused so we don't try to submit it in Tick.
+        // Move the mRecordingContext.used to mUnusedCommands so it can be cleaned up in
+        // ShutDownImpl
+        if (mRecordingContext.used) {
+            CommandPoolAndBuffer commands = {mRecordingContext.commandPool,
+                                             mRecordingContext.commandBuffer};
+            mUnusedCommands.push_back(commands);
+            mRecordingContext = CommandRecordingContext();
+        }
+
         VkResult waitIdleResult = VkResult::WrapUnsafe(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
@@ -782,9 +792,6 @@
         }
         mRecordingContext.signalSemaphores.clear();
 
-        // Assert that errors are device loss so that we can continue with destruction
-        AssertAndIgnoreDeviceLossError(TickImpl());
-
         ASSERT(mCommandsInFlight.Empty());
         for (const CommandPoolAndBuffer& commands : mUnusedCommands) {
             fn.DestroyCommandPool(mVkDevice, commands.pool, nullptr);