From 8123ea88d176ae21130d111207750aba9da0d038 Mon Sep 17 00:00:00 2001
From: "Bernhart, Bryan" <bryan.bernhart@intel.com>
Date: Tue, 15 Aug 2023 14:22:47 -0700
Subject: [PATCH] Fix deadlock when creating unowned committed resources.

---
 src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp    | 42 +++++++++++--------
 .../end2end/D3D12ResourceAllocatorTests.cpp   |  4 ++
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp
index f6d70773..86c0487a 100644
--- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp
+++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp
@@ -937,29 +937,35 @@ namespace gpgmm::d3d12 {
             (RESOURCE_ALLOCATOR_CREATE_RESOURCE_PARAMS{allocationDescriptor, resourceDescriptor,
                                                        initialResourceState, pClearValue}));
 
-        std::lock_guard<std::mutex> lock(mMutex);
         ComPtr<IResourceAllocation> allocation;
-        GPGMM_RETURN_IF_FAILED(
-            CreateResourceInternal(allocationDescriptor, resourceDescriptor, initialResourceState,
-                                   pClearValue, &allocation),
-            mDevice);
+        {
+            // Mutex must be destroyed before the allocation gets released. This occurs
+            // when the allocation never calls Detach() below and calls release which
+            // re-enters |this| upon DeallocateMemory().
+            std::lock_guard<std::mutex> lock(mMutex);
+            GPGMM_RETURN_IF_FAILED(
+                CreateResourceInternal(allocationDescriptor, resourceDescriptor,
+                                       initialResourceState, pClearValue, &allocation),
+                mDevice);
 
-        ASSERT(allocation->GetResource() != nullptr);
+            ASSERT(allocation->GetResource() != nullptr);
 
-        if (GPGMM_UNLIKELY(mTrackingAllocator)) {
-            GPGMM_RETURN_IF_FAILED(allocation->SetDebugName(allocationDescriptor.DebugName),
-                                   mDevice);
-            mTrackingAllocator->TrackAllocation(static_cast<ResourceAllocation*>(allocation.Get()));
-        }
+            if (GPGMM_UNLIKELY(mTrackingAllocator)) {
+                GPGMM_RETURN_IF_FAILED(allocation->SetDebugName(allocationDescriptor.DebugName),
+                                       mDevice);
+                mTrackingAllocator->TrackAllocation(
+                    static_cast<ResourceAllocation*>(allocation.Get()));
+            }
 
-        // Update the current usage counters.
-        if (mUseDetailedTimingEvents) {
-            GPGMM_RETURN_IF_FAILED(QueryStatsInternal(nullptr), mDevice);
-        }
+            // Update the current usage counters.
+            if (mUseDetailedTimingEvents) {
+                GPGMM_RETURN_IF_FAILED(QueryStatsInternal(nullptr), mDevice);
+            }
 
-        if (allocationDescriptor.Flags & ALLOCATION_FLAG_ALWAYS_WARN_ON_ALIGNMENT_MISMATCH) {
-            CheckAndReportAllocationMisalignment(
-                *static_cast<ResourceAllocation*>(allocation.Get()));
+            if (allocationDescriptor.Flags & ALLOCATION_FLAG_ALWAYS_WARN_ON_ALIGNMENT_MISMATCH) {
+                CheckAndReportAllocationMisalignment(
+                    *static_cast<ResourceAllocation*>(allocation.Get()));
+            }
         }
 
         if (ppResourceAllocationOut != nullptr) {
diff --git a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp
index 69573db7..94c37c6f 100644
--- a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp
+++ b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp
@@ -898,6 +898,10 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferAlwaysCommitted) {
     ALLOCATION_DESC allocationDesc = {};
     allocationDesc.HeapType = D3D12_HEAP_TYPE_DEFAULT;
 
+    ASSERT_SUCCEEDED(resourceAllocator->CreateResource(
+        allocationDesc, CreateBasicBufferDesc(kBufferOf4MBAllocationSize),
+        D3D12_RESOURCE_STATE_COMMON, nullptr, nullptr));
+
     ComPtr<IResourceAllocation> allocation;
     ASSERT_SUCCEEDED(resourceAllocator->CreateResource(
         allocationDesc, CreateBasicBufferDesc(kBufferOf4MBAllocationSize),