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>
This commit is contained in:
Austin Eng 2022-05-24 07:04:25 +00:00 committed by Dawn LUCI CQ
parent 4e34192f9e
commit 2066522311
2 changed files with 20 additions and 1 deletions

View File

@ -308,6 +308,10 @@ void DeviceBase::WillDropLastExternalRef() {
// out all remaining refs. // out all remaining refs.
Destroy(); 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 // 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. // application may have freed any device-scope memory needed to run the callback.
mUncapturedErrorCallback = [](WGPUErrorType, char const* message, void*) { mUncapturedErrorCallback = [](WGPUErrorType, char const* message, void*) {
@ -454,11 +458,12 @@ void DeviceBase::Destroy() {
// implementations of DestroyImpl checks that we are disconnected before doing work. // implementations of DestroyImpl checks that we are disconnected before doing work.
mState = State::Disconnected; mState = State::Disconnected;
// Note: mQueue is not released here since the application may still get it after calling
// Destroy() via APIGetQueue.
mDynamicUploader = nullptr; mDynamicUploader = nullptr;
mEmptyBindGroupLayout = nullptr; mEmptyBindGroupLayout = nullptr;
mInternalPipelineStore = nullptr; mInternalPipelineStore = nullptr;
mExternalTexturePlaceholderView = nullptr; mExternalTexturePlaceholderView = nullptr;
mQueue = nullptr;
AssumeCommandsComplete(); AssumeCommandsComplete();

View File

@ -190,6 +190,20 @@ TEST_P(DestroyTest, DestroyDeviceLingeringBGL) {
DestroyDevice(); 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, DAWN_INSTANTIATE_TEST(DestroyTest,
D3D12Backend(), D3D12Backend(),
MetalBackend(), MetalBackend(),