Fix multiple device leaks in dawn_end2end_tests and dawn_unittests

Adds ForTesting APIs to the instance to track the number of devices.

Bug: dawn:1164
Change-Id: Ib743afb1e86ef16740d49613f43f9e2f009232bc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/90583
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

include/dawn/native/DawnNative.h

@@ -170,6 +170,8 @@ class DAWN_NATIVE_EXPORT Instance {
     // TODO(dawn:1374) Deprecate this once it is passed via the descriptor.
     void SetPlatform(dawn::platform::Platform* platform);
 
+    uint64_t GetDeviceCountForTesting() const;
+
     // Returns the underlying WGPUInstance object.
     WGPUInstance Get() const;
 

src/dawn/native/DawnNative.cpp

@@ -244,6 +244,10 @@ void Instance::SetPlatform(dawn::platform::Platform* platform) {
     mImpl->SetPlatform(platform);
 }
 
+uint64_t Instance::GetDeviceCountForTesting() const {
+    return mImpl->GetDeviceCountForTesting();
+}
+
 WGPUInstance Instance::Get() const {
     return ToAPI(mImpl);
 }

src/dawn/native/Device.cpp

@@ -171,6 +171,7 @@ ResultOrError<Ref<PipelineLayoutBase>> ValidateLayoutAndGetRenderPipelineDescrip
 
 DeviceBase::DeviceBase(AdapterBase* adapter, const DeviceDescriptor* descriptor)
     : mInstance(adapter->GetInstance()), mAdapter(adapter), mNextPipelineCompatibilityToken(1) {
+    mInstance->IncrementDeviceCountForTesting();
     ASSERT(descriptor != nullptr);
 
     AdapterProperties adapterProperties;
@@ -220,6 +221,10 @@ DeviceBase::~DeviceBase() {
     // We need to explicitly release the Queue before we complete the destructor so that the
     // Queue does not get destroyed after the Device.
     mQueue = nullptr;
+    // mInstance is not set for mock test devices.
+    if (mInstance != nullptr) {
+        mInstance->DecrementDeviceCountForTesting();
+    }
 }
 
 MaybeError DeviceBase::Initialize(Ref<QueueBase> defaultQueue) {

src/dawn/native/Instance.cpp

@@ -437,6 +437,18 @@ BlobCache* InstanceBase::GetBlobCache() {
     return mBlobCache.get();
 }
 
+uint64_t InstanceBase::GetDeviceCountForTesting() const {
+    return mDeviceCountForTesting.load();
+}
+
+void InstanceBase::IncrementDeviceCountForTesting() {
+    mDeviceCountForTesting++;
+}
+
+void InstanceBase::DecrementDeviceCountForTesting() {
+    mDeviceCountForTesting--;
+}
+
 const std::vector<std::string>& InstanceBase::GetRuntimeSearchPaths() const {
     return mRuntimeSearchPaths;
 }

src/dawn/native/Instance.h

@@ -84,6 +84,10 @@ class InstanceBase final : public RefCounted {
     dawn::platform::Platform* GetPlatform();
     BlobCache* GetBlobCache();
 
+    uint64_t GetDeviceCountForTesting() const;
+    void IncrementDeviceCountForTesting();
+    void DecrementDeviceCountForTesting();
+
     const std::vector<std::string>& GetRuntimeSearchPaths() const;
 
     // Get backend-independent libraries that need to be loaded dynamically.
@@ -130,6 +134,8 @@ class InstanceBase final : public RefCounted {
 #if defined(DAWN_USE_X11)
     std::unique_ptr<XlibXcbFunctions> mXlibXcbFunctions;
 #endif  // defined(DAWN_USE_X11)
+
+    std::atomic_uint64_t mDeviceCountForTesting{0};
 };
 
 }  // namespace dawn::native

src/dawn/tests/BUILD.gn

@@ -491,12 +491,6 @@ source_set("end2end_tests_sources") {
     "end2end/ViewportTests.cpp",
   ]
 
-  # Validation tests that need OS windows live in end2end tests.
-  sources += [
-    "unittests/validation/ValidationTest.cpp",
-    "unittests/validation/ValidationTest.h",
-  ]
-
   libs = []
 
   if (dawn_enable_d3d12) {

src/dawn/tests/DawnNativeTest.cpp

@@ -62,7 +62,7 @@ void DawnNativeTest::SetUp() {
 
     ASSERT(foundNullAdapter);
 
-    device = wgpu::Device(CreateTestDevice());
+    device = wgpu::Device::Acquire(CreateTestDevice());
     device.SetUncapturedErrorCallback(DawnNativeTest::OnDeviceError, nullptr);
 }
 

src/dawn/tests/DawnTest.cpp

@@ -723,6 +723,9 @@ DawnTestBase::~DawnTestBase() {
         mBackendAdapter.ResetInternalDeviceForTesting();
     }
     mWireHelper.reset();
+
+    // Check that all devices were destructed.
+    EXPECT_EQ(gTestEnv->GetInstance()->GetDeviceCountForTesting(), 0u);
 }
 
 bool DawnTestBase::IsD3D12() const {

src/dawn/tests/unittests/FeatureTests.cpp

@@ -84,5 +84,6 @@ TEST_F(FeatureTests, GetEnabledFeatures) {
         wgpu::FeatureName enabledFeature;
         deviceBase->APIEnumerateFeatures(&enabledFeature);
         EXPECT_EQ(enabledFeature, featureName);
+        deviceBase->APIRelease();
     }
 }

src/dawn/tests/unittests/validation/DeviceValidationTests.cpp

@@ -23,8 +23,8 @@ using ::testing::HasSubstr;
 class RequestDeviceValidationTest : public ValidationTest {
   protected:
     void SetUp() override {
-        DAWN_SKIP_TEST_IF(UsesWire());
         ValidationTest::SetUp();
+        DAWN_SKIP_TEST_IF(UsesWire());
     }
 
     static void ExpectRequestDeviceSuccess(WGPURequestDeviceStatus status,

src/dawn/tests/unittests/validation/LabelTests.cpp

@@ -325,7 +325,7 @@ TEST_F(LabelTest, Queue) {
     // The label should be empty if one was not set.
     {
         wgpu::DeviceDescriptor descriptor;
-        wgpu::Device labelDevice = adapter.CreateDevice(&descriptor);
+        wgpu::Device labelDevice = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
         std::string readbackLabel =
             dawn::native::GetObjectLabelForTesting(labelDevice.GetQueue().Get());
         ASSERT_TRUE(readbackLabel.empty());
@@ -334,7 +334,7 @@ TEST_F(LabelTest, Queue) {
     // Test setting a label through API
     {
         wgpu::DeviceDescriptor descriptor;
-        wgpu::Device labelDevice = adapter.CreateDevice(&descriptor);
+        wgpu::Device labelDevice = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
         labelDevice.GetQueue().SetLabel(label.c_str());
         std::string readbackLabel =
             dawn::native::GetObjectLabelForTesting(labelDevice.GetQueue().Get());
@@ -345,7 +345,7 @@ TEST_F(LabelTest, Queue) {
     {
         wgpu::DeviceDescriptor descriptor;
         descriptor.defaultQueue.label = label.c_str();
-        wgpu::Device labelDevice = adapter.CreateDevice(&descriptor);
+        wgpu::Device labelDevice = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
         std::string readbackLabel =
             dawn::native::GetObjectLabelForTesting(labelDevice.GetQueue().Get());
         ASSERT_EQ(label, readbackLabel);

src/dawn/tests/unittests/validation/ToggleValidationTests.cpp

@@ -18,7 +18,12 @@
 
 namespace {
 
-class ToggleValidationTest : public ValidationTest {};
+class ToggleValidationTest : public ValidationTest {
+    void SetUp() override {
+        ValidationTest::SetUp();
+        DAWN_SKIP_TEST_IF(UsesWire());
+    }
+};
 
 // Tests querying the detail of a toggle from dawn::native::InstanceBase works correctly.
 TEST_F(ToggleValidationTest, QueryToggleInfo) {
@@ -51,8 +56,8 @@ TEST_F(ToggleValidationTest, OverrideToggleUsage) {
         togglesDesc.forceEnabledToggles = &kValidToggleName;
         togglesDesc.forceEnabledTogglesCount = 1;
 
-        WGPUDevice deviceWithToggle = adapter.CreateDevice(&descriptor);
+        wgpu::Device deviceWithToggle = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
-        std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle);
+        std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle.Get());
         bool validToggleExists = false;
         for (const char* toggle : toggleNames) {
             if (strcmp(toggle, kValidToggleName) == 0) {
@@ -71,8 +76,8 @@ TEST_F(ToggleValidationTest, OverrideToggleUsage) {
         togglesDesc.forceEnabledToggles = &kInvalidToggleName;
         togglesDesc.forceEnabledTogglesCount = 1;
 
-        WGPUDevice deviceWithToggle = adapter.CreateDevice(&descriptor);
+        wgpu::Device deviceWithToggle = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
-        std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle);
+        std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle.Get());
         bool InvalidToggleExists = false;
         for (const char* toggle : toggleNames) {
             if (strcmp(toggle, kInvalidToggleName) == 0) {
@@ -91,8 +96,8 @@ TEST_F(ToggleValidationTest, TurnOffVsyncWithToggle) {
     togglesDesc.forceEnabledToggles = &kValidToggleName;
     togglesDesc.forceEnabledTogglesCount = 1;
 
-    WGPUDevice deviceWithToggle = adapter.CreateDevice(&descriptor);
+    wgpu::Device deviceWithToggle = wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
-    std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle);
+    std::vector<const char*> toggleNames = dawn::native::GetTogglesUsed(deviceWithToggle.Get());
     bool validToggleExists = false;
     for (const char* toggle : toggleNames) {
         if (strcmp(toggle, kValidToggleName) == 0) {

src/dawn/tests/unittests/validation/ValidationTest.cpp

@@ -118,6 +118,9 @@ ValidationTest::~ValidationTest() {
     // will call a nullptr
     device = wgpu::Device();
     mWireHelper.reset();
+
+    // Check that all devices were destructed.
+    EXPECT_EQ(instance->GetDeviceCountForTesting(), 0u);
 }
 
 void ValidationTest::TearDown() {

src/dawn/tests/white_box/VulkanImageWrappingTests_DmaBuf.cpp

@@ -118,7 +118,8 @@ class VulkanImageWrappingTestBackendDmaBuf : public VulkanImageWrappingTestBacke
         descriptorDmaBuf.stride = textureDmaBuf->stride;
         descriptorDmaBuf.drmModifier = textureDmaBuf->drmModifier;
 
-        return dawn::native::vulkan::WrapVulkanImage(device.Get(), &descriptorDmaBuf);
+        return wgpu::Texture::Acquire(
+            dawn::native::vulkan::WrapVulkanImage(device.Get(), &descriptorDmaBuf));
     }
 
     bool ExportImage(const wgpu::Texture& texture,

src/dawn/tests/white_box/VulkanImageWrappingTests_OpaqueFD.cpp

@@ -141,7 +141,8 @@ class VulkanImageWrappingTestBackendOpaqueFD : public VulkanImageWrappingTestBac
         descriptorOpaqueFD.memoryTypeIndex = textureOpaqueFD->memoryTypeIndex;
         descriptorOpaqueFD.waitFDs = std::move(waitFDs);
 
-        return dawn::native::vulkan::WrapVulkanImage(device.Get(), &descriptorOpaqueFD);
+        return wgpu::Texture::Acquire(
+            dawn::native::vulkan::WrapVulkanImage(device.Get(), &descriptorOpaqueFD));
     }
 
     bool ExportImage(const wgpu::Texture& texture,