From 4e9ee36f9e542ad6b563f5dd1f002157ca153023 Mon Sep 17 00:00:00 2001
From: Egor Yusov <egor.yusov@gmail.com>
Date: Tue, 24 Jul 2018 09:35:51 -0700
Subject: Fixed issue with Device context destruction in Vulkan backend

---
 .../include/DescriptorPoolManager.h                |  4 +--
 .../src/DeviceContextVkImpl.cpp                    | 34 ++++++++++++++--------
 .../src/VulkanUtilities/VulkanUploadHeap.cpp       |  1 +
 3 files changed, 25 insertions(+), 14 deletions(-)

(limited to 'Graphics/GraphicsEngineVulkan')

diff --git a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
index 9f029f24..af4f8dd7 100644
--- a/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
+++ b/Graphics/GraphicsEngineVulkan/include/DescriptorPoolManager.h
@@ -139,8 +139,8 @@ private:
     const uint32_t m_MaxSets;
 
     std::mutex m_Mutex;
-    std::shared_ptr<const VulkanUtilities::VulkanLogicalDevice> m_LogicalDevice;
-    std::deque< std::unique_ptr<VulkanUtilities::VulkanDescriptorPool> > m_DescriptorPools;
+    std::shared_ptr<const VulkanUtilities::VulkanLogicalDevice>                       m_LogicalDevice;
+    std::deque< std::unique_ptr<VulkanUtilities::VulkanDescriptorPool> >              m_DescriptorPools;
     std::vector< std::pair<VkDescriptorSet, VulkanUtilities::VulkanDescriptorPool*> > m_ReleasedAllocations;
 
     // When adding new members, do not forget to update move ctor!
diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
index beadb78d..1d9c4e05 100644
--- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
@@ -120,22 +120,32 @@ namespace Diligent
 
     DeviceContextVkImpl::~DeviceContextVkImpl()
     {
-        auto *pDeviceVkImpl = m_pDevice.RawPtr<RenderDeviceVkImpl>();
-        if (m_bIsDeferred)
-        {
-            DisposeCurrentCmdBuffer(pDeviceVkImpl->GetNextFenceValue());
-            // There must be no resources in the stale resource list. All outstanding resources (if any) must be in the
-            // release queue and must be now released
-            ReleaseStaleContextResources(m_NextCmdBuffNumber, pDeviceVkImpl->GetNextFenceValue(), pDeviceVkImpl->GetCompletedFenceValue());
-        }
-        else
-        {
-            if (m_State.NumCommands != 0)
-                LOG_WARNING_MESSAGE("Flusing outstanding commands from the device context being destroyed. This may result in synchronization errors");
+        auto* pDeviceVkImpl = m_pDevice.RawPtr<RenderDeviceVkImpl>();
+        if (m_State.NumCommands != 0)
+            LOG_ERROR_MESSAGE(m_bIsDeferred ? 
+                                "There are outstanding commands in the deferred context being destroyed, which indicates that FinishCommandList() has not been called." :
+                                "There are outstanding commands in the immediate context being destroyed, which indicates the context has not been Flush()'ed.",
+                              " This is unexpected and may result in synchronization errors");
 
+        if (!m_bIsDeferred)
+        {
+            // There should be no outstanding commands, but we need to call Flush to discard all stale
+            // context resources to actually destroy them in the next call to ReleaseStaleContextResources()
             Flush();
+            // We must now wait for GPU to finish so that we can safely destroy all context resources
+            pDeviceVkImpl->IdleGPU(true);
         }
 
+        DisposeCurrentCmdBuffer(pDeviceVkImpl->GetNextFenceValue());
+
+        // There must be no resources in the stale resource list. For immediate context, all stale resources must have been
+        // moved to the release queue by Flush(). For deferred contexts, this should have happened in the last FinishCommandList()
+        // call.
+        VERIFY(m_ReleaseQueue.GetStaleResourceCount() == 0, "All stale resources must have been discarded at this point");
+        ReleaseStaleContextResources(m_NextCmdBuffNumber, pDeviceVkImpl->GetNextFenceValue(), pDeviceVkImpl->GetCompletedFenceValue());
+        // Since we idled the GPU, all stale resources must have been destroyed now
+        VERIFY(m_ReleaseQueue.GetPendingReleaseResourceCount() == 0, "All stale resources must have been destroyed at this point");
+
         auto VkCmdPool = m_CmdPool.Release();
         pDeviceVkImpl->SafeReleaseVkObject(std::move(VkCmdPool));
     }
diff --git a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanUploadHeap.cpp b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanUploadHeap.cpp
index 57b4075d..ad6e9fbc 100644
--- a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanUploadHeap.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanUploadHeap.cpp
@@ -90,6 +90,7 @@ void VulkanUploadHeap::OnNewPageCreated(VulkanMemoryPage& NewPage)
     VERIFY(MemoryTypeIndex == m_StagingBufferMemoryTypeIndex, "Incosistent memory type");
     auto err = m_LogicalDevice.BindBufferMemory(NewBuffer, NewPage.GetVkMemory(), 0);
     CHECK_VK_ERROR_AND_THROW(err, "Failed to bind buffer memory");
+    VERIFY(m_Buffers.find(&NewPage) == m_Buffers.end(), "Buffer corresponding to this page already exists");
     m_Buffers.emplace(&NewPage, std::move(NewBuffer));
 }
 
-- 
cgit v1.2.3