From bb10a9187623469df7d90db59c88de13fc9a0c5f Mon Sep 17 00:00:00 2001
From: Austin Eng <enga@chromium.org>
Date: Wed, 7 Aug 2019 21:47:24 +0000
Subject: [PATCH] Fix leak of AttachmentState

Unlike API objects, the AttachmentState object is only used internally
and should have no external references. We should not increment the
reference count on creation because it is Ref'ed internally. This patch
also adds ASSERTs that all Device caches are empty when the Device is
destroyed.

Bug: dawn:154
Change-Id: I5dd46bde03c0be920356444e6b1214ed38e833e8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/9761
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
---
 src/dawn_native/Device.cpp | 30 ++++++++++++++++++------------
 src/dawn_native/Device.h   |  4 ++--
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 8fe480052f..b84d8ddef2 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -77,6 +77,14 @@ namespace dawn_native {
         // Devices must explicitly free the uploader
         ASSERT(mDynamicUploader == nullptr);
         ASSERT(mDeferredCreateBufferMappedAsyncResults.empty());
+
+        ASSERT(mCaches->attachmentStates.empty());
+        ASSERT(mCaches->bindGroupLayouts.empty());
+        ASSERT(mCaches->computePipelines.empty());
+        ASSERT(mCaches->pipelineLayouts.empty());
+        ASSERT(mCaches->renderPipelines.empty());
+        ASSERT(mCaches->samplers.empty());
+        ASSERT(mCaches->shaderModules.empty());
     }
 
     void DeviceBase::HandleError(const char* message) {
@@ -255,35 +263,33 @@ namespace dawn_native {
         ASSERT(removedCount == 1);
     }
 
-    AttachmentState* DeviceBase::GetOrCreateAttachmentState(
+    Ref<AttachmentState> DeviceBase::GetOrCreateAttachmentState(
         const RenderPipelineDescriptor* descriptor) {
         AttachmentStateBlueprint blueprint(descriptor);
 
         auto iter = mCaches->attachmentStates.find(&blueprint);
         if (iter != mCaches->attachmentStates.end()) {
-            AttachmentState* cachedState = static_cast<AttachmentState*>(*iter);
-            cachedState->Reference();
-            return cachedState;
+            return static_cast<AttachmentState*>(*iter);
         }
 
-        AttachmentState* attachmentState = new AttachmentState(this, blueprint);
-        mCaches->attachmentStates.insert(attachmentState);
+        Ref<AttachmentState> attachmentState = new AttachmentState(this, blueprint);
+        attachmentState->Release();
+        mCaches->attachmentStates.insert(attachmentState.Get());
         return attachmentState;
     }
 
-    AttachmentState* DeviceBase::GetOrCreateAttachmentState(
+    Ref<AttachmentState> DeviceBase::GetOrCreateAttachmentState(
         const RenderPassDescriptor* descriptor) {
         AttachmentStateBlueprint blueprint(descriptor);
 
         auto iter = mCaches->attachmentStates.find(&blueprint);
         if (iter != mCaches->attachmentStates.end()) {
-            AttachmentState* cachedState = static_cast<AttachmentState*>(*iter);
-            cachedState->Reference();
-            return cachedState;
+            return static_cast<AttachmentState*>(*iter);
         }
 
-        AttachmentState* attachmentState = new AttachmentState(this, blueprint);
-        mCaches->attachmentStates.insert(attachmentState);
+        Ref<AttachmentState> attachmentState = new AttachmentState(this, blueprint);
+        attachmentState->Release();
+        mCaches->attachmentStates.insert(attachmentState.Get());
         return attachmentState;
     }
 
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index 89353b607d..1d679bc51d 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -115,8 +115,8 @@ namespace dawn_native {
             const ShaderModuleDescriptor* descriptor);
         void UncacheShaderModule(ShaderModuleBase* obj);
 
-        AttachmentState* GetOrCreateAttachmentState(const RenderPipelineDescriptor* descriptor);
-        AttachmentState* GetOrCreateAttachmentState(const RenderPassDescriptor* descriptor);
+        Ref<AttachmentState> GetOrCreateAttachmentState(const RenderPipelineDescriptor* descriptor);
+        Ref<AttachmentState> GetOrCreateAttachmentState(const RenderPassDescriptor* descriptor);
         void UncacheAttachmentState(AttachmentState* obj);
 
         // Dawn API