From 2066522311e958e43b9dfb220f57899cd42b1405 Mon Sep 17 00:00:00 2001
From: Austin Eng <enga@chromium.org>
Date: Tue, 24 May 2022 07:04:25 +0000
Subject: [PATCH] Move clearing DeviceBase::mQueue from Destroy() to
 WillDropLastExternalRef

This member should not be cleared after Destroy() since the application
may ask for it if it still has a reference to the device.
Instead, only clear the member after the application has dropped their
last reference to the device.

Bug: chromium:1327865
Change-Id: I282482fec5db11b4c75b91ba5995d7e2599b89a1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/91281
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
---
 src/dawn/native/Device.cpp              |  7 ++++++-
 src/dawn/tests/end2end/DestroyTests.cpp | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index a0afaf52b9..84a9c3a500 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -308,6 +308,10 @@ void DeviceBase::WillDropLastExternalRef() {
     // out all remaining refs.
     Destroy();
 
+    // Drop te device's reference to the queue. Because the application dropped the last external
+    // references, they can no longer get the queue from APIGetQueue().
+    mQueue = nullptr;
+
     // Reset callbacks since after this, since after dropping the last external reference, the
     // application may have freed any device-scope memory needed to run the callback.
     mUncapturedErrorCallback = [](WGPUErrorType, char const* message, void*) {
@@ -454,11 +458,12 @@ void DeviceBase::Destroy() {
     // implementations of DestroyImpl checks that we are disconnected before doing work.
     mState = State::Disconnected;
 
+    // Note: mQueue is not released here since the application may still get it after calling
+    // Destroy() via APIGetQueue.
     mDynamicUploader = nullptr;
     mEmptyBindGroupLayout = nullptr;
     mInternalPipelineStore = nullptr;
     mExternalTexturePlaceholderView = nullptr;
-    mQueue = nullptr;
 
     AssumeCommandsComplete();
 
diff --git a/src/dawn/tests/end2end/DestroyTests.cpp b/src/dawn/tests/end2end/DestroyTests.cpp
index 89450cfafd..7b46a23532 100644
--- a/src/dawn/tests/end2end/DestroyTests.cpp
+++ b/src/dawn/tests/end2end/DestroyTests.cpp
@@ -190,6 +190,20 @@ TEST_P(DestroyTest, DestroyDeviceLingeringBGL) {
     DestroyDevice();
 }
 
+// Regression test for crbug.com/1327865 where the device set the queue to null
+// inside Destroy() such that it could no longer return it to a call to GetQueue().
+TEST_P(DestroyTest, GetQueueAfterDeviceDestroy) {
+    DestroyDevice();
+
+    wgpu::Queue queue = device.GetQueue();
+    ASSERT_DEVICE_ERROR(queue.OnSubmittedWorkDone(
+        0u,
+        [](WGPUQueueWorkDoneStatus status, void* userdata) {
+            EXPECT_EQ(status, WGPUQueueWorkDoneStatus_DeviceLost);
+        },
+        nullptr));
+}
+
 DAWN_INSTANTIATE_TEST(DestroyTest,
                       D3D12Backend(),
                       MetalBackend(),