From 600a26d50a04d82cade2f52c79a2d7ad9e71b89b Mon Sep 17 00:00:00 2001
From: Rafael Cintron <rafael.cintron@microsoft.com>
Date: Fri, 8 Nov 2019 21:47:00 +0000
Subject: [PATCH] Fix ResourceHeapAllocation Memory Leak

ResourceAllocatorManager::DeallocateMemory was correctly invalidating the
passed in allocation object. However, since the subclass
ResourceHeapAllocation class was not overriding the Invalidate method and
clearing out the D3D12Resource pointer, the resource ended up being tied
to the lifetime of the Texture object instead of being released on Destroy.

In Chromium, this bug was particularly egregious as it meant swap chain
texture cleanup was at the whims of the Javascript garbage collector.

Bug: dawn:242
Change-Id: Ia5856c61c8d3b92a2247a9aaa5f91c5de0a99dcb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/13200
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
Reviewed-by: Austin Eng <enga@chromium.org>
---
 src/dawn_native/ResourceMemoryAllocation.h              | 4 ++--
 src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp | 2 ++
 src/dawn_native/d3d12/ResourceHeapAllocationD3D12.cpp   | 5 +++++
 src/dawn_native/d3d12/ResourceHeapAllocationD3D12.h     | 4 +++-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/dawn_native/ResourceMemoryAllocation.h b/src/dawn_native/ResourceMemoryAllocation.h
index c4de0301bf..a66129ac64 100644
--- a/src/dawn_native/ResourceMemoryAllocation.h
+++ b/src/dawn_native/ResourceMemoryAllocation.h
@@ -54,14 +54,14 @@ namespace dawn_native {
                                  uint64_t offset,
                                  ResourceHeapBase* resourceHeap,
                                  uint8_t* mappedPointer = nullptr);
-        ~ResourceMemoryAllocation() = default;
+        virtual ~ResourceMemoryAllocation() = default;
 
         ResourceHeapBase* GetResourceHeap() const;
         uint64_t GetOffset() const;
         uint8_t* GetMappedPointer() const;
         AllocationInfo GetInfo() const;
 
-        void Invalidate();
+        virtual void Invalidate();
 
       private:
         AllocationInfo mInfo;
diff --git a/src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp b/src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp
index 895afe06fd..5200a145c3 100644
--- a/src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp
+++ b/src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp
@@ -168,6 +168,8 @@ namespace dawn_native { namespace d3d12 {
         // Invalidate the allocation immediately in case one accidentally
         // calls DeallocateMemory again using the same allocation.
         allocation.Invalidate();
+
+        ASSERT(allocation.GetD3D12Resource().Get() == nullptr);
     }
 
     void ResourceAllocatorManager::FreeMemory(ResourceHeapAllocation& allocation) {
diff --git a/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.cpp b/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.cpp
index 158e8900d1..bf805cb8d0 100644
--- a/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.cpp
+++ b/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.cpp
@@ -23,6 +23,11 @@ namespace dawn_native { namespace d3d12 {
         : ResourceMemoryAllocation(info, offset, nullptr), mResource(std::move(resource)) {
     }
 
+    void ResourceHeapAllocation::Invalidate() {
+        ResourceMemoryAllocation::Invalidate();
+        mResource.Reset();
+    }
+
     ComPtr<ID3D12Resource> ResourceHeapAllocation::GetD3D12Resource() const {
         return mResource;
     }
diff --git a/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.h b/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.h
index e168e0bb72..d764a9675f 100644
--- a/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.h
+++ b/src/dawn_native/d3d12/ResourceHeapAllocationD3D12.h
@@ -26,7 +26,9 @@ namespace dawn_native { namespace d3d12 {
         ResourceHeapAllocation(const AllocationInfo& info,
                                uint64_t offset,
                                ComPtr<ID3D12Resource> resource);
-        ~ResourceHeapAllocation() = default;
+        ~ResourceHeapAllocation() override = default;
+
+        void Invalidate() override;
 
         ComPtr<ID3D12Resource> GetD3D12Resource() const;
         D3D12_GPU_VIRTUAL_ADDRESS GetGPUPointer() const;