commit 11267a7e7d25a2771ecd9972210e5273389ac6f8
author Jiawei Shao <jiawei.shao@intel.com> Tue Apr 02 00:44:24 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Apr 02 00:44:24 2024 +0000
tree 7884e8989edc22bd6336ac37157a86f591bd678d
parent 5e19fc5dc123f3248161156bbb691bc01c73048f

Fix dangling pointers in DeviceBase

This patch fixes all the dangling pointers in DeviceBase. In
Chromium the callbacks and userdata in DeviceBase should be cleared
after Device.Destroy() is called and before Dawn wire server is
destroyed, so we should support setting nullptr to callbacks and
userdata even when Device is lost.

Bug: dawn:2349
Change-Id: I5f4e919e44b66181b4fac003ef25e088461d26cc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/181144
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>

src/dawn/native/Device.cpp

diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index af8f738..ec3b63a 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -415,11 +415,13 @@
         dawn::WarningLog() << "Uncaptured error after last external device reference dropped.\n"
                            << message;
     };
+    mUncapturedErrorUserdata = nullptr;
 
     mDeviceLostCallback = [](WGPUDeviceLostReason, char const* message, void*) {
         dawn::WarningLog() << "Device lost after last external device reference dropped.\n"
                            << message;
     };
+    mDeviceLostUserdata = nullptr;
 
     // mAdapter is not set for mock test devices.
     // TODO(crbug.com/dawn/1702): using a mock adapter could avoid the null checking.
@@ -689,6 +691,13 @@
     // this call.
     FlushCallbackTaskQueue();
     auto deviceLock(GetScopedLock());
+    // Clearing the callback and userdata is allowed because in Chromium they should be cleared
+    // after Dawn device is destroyed and before Dawn wire server is destroyed.
+    if (callback == nullptr) {
+        mUncapturedErrorCallback = nullptr;
+        mUncapturedErrorUserdata = nullptr;
+        return;
+    }
     if (IsLost()) {
         return;
     }
@@ -706,6 +715,13 @@
     // this call.
     FlushCallbackTaskQueue();
     auto deviceLock(GetScopedLock());
+    // Clearing the callback and userdata is allowed because in Chromium they should be cleared
+    // after Dawn device is destroyed and before Dawn wire server is destroyed.
+    if (callback == nullptr) {
+        mDeviceLostCallback = nullptr;
+        mDeviceLostUserdata = nullptr;
+        return;
+    }
     if (IsLost()) {
         return;
     }

src/dawn/native/Device.h

diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h
index 69128c6..663ce83 100644
--- a/src/dawn/native/Device.h
+++ b/src/dawn/native/Device.h
@@ -580,17 +580,14 @@
                                                     const Extent3D& copySizePixels) = 0;
 
     wgpu::ErrorCallback mUncapturedErrorCallback = nullptr;
-    // TODO(https://crbug.com/dawn/2349): Investigate DanglingUntriaged in dawn/native.
-    raw_ptr<void, DanglingUntriaged> mUncapturedErrorUserdata = nullptr;
+    raw_ptr<void> mUncapturedErrorUserdata = nullptr;
 
     std::shared_mutex mLoggingMutex;
     wgpu::LoggingCallback mLoggingCallback = nullptr;
-    // TODO(https://crbug.com/dawn/2349): Investigate DanglingUntriaged in dawn/native.
-    raw_ptr<void, DanglingUntriaged> mLoggingUserdata = nullptr;
+    raw_ptr<void> mLoggingUserdata = nullptr;
 
     wgpu::DeviceLostCallback mDeviceLostCallback = nullptr;
-    // TODO(https://crbug.com/dawn/2349): Investigate DanglingUntriaged in dawn/native.
-    raw_ptr<void, DanglingUntriaged> mDeviceLostUserdata = nullptr;
+    raw_ptr<void> mDeviceLostUserdata = nullptr;
 
     std::unique_ptr<ErrorScopeStack> mErrorScopeStack;