Improved exception safety of pipeline state object construction

7 files changed, 116 insertions, 77 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp
index 96b34008..d4f2fc36 100644
--- a/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/PipelineStateVkImpl.hpp
@@ -129,13 +129,15 @@ private:
     using TShaderStages = ShaderResourceLayoutVk::TShaderStages;
 
     template <typename PSOCreateInfoType>
-    LinearAllocator InitInternalObjects(const PSOCreateInfoType&                           CreateInfo,
-                                        std::vector<VkPipelineShaderStageCreateInfo>&      vkShaderStages,
-                                        std::vector<VulkanUtilities::ShaderModuleWrapper>& ShaderModules);
+    void InitInternalObjects(const PSOCreateInfoType&                           CreateInfo,
+                             std::vector<VkPipelineShaderStageCreateInfo>&      vkShaderStages,
+                             std::vector<VulkanUtilities::ShaderModuleWrapper>& ShaderModules);
 
     void InitResourceLayouts(const PipelineStateCreateInfo& CreateInfo,
                              TShaderStages&                 ShaderStages);
 
+    void Destruct();
+
     const ShaderResourceLayoutVk& GetStaticShaderResLayout(Uint32 ShaderInd) const
     {
         VERIFY_EXPR(ShaderInd < GetNumShaderStages());
@@ -148,7 +150,7 @@ private:
         return m_StaticResCaches[ShaderInd];
     }
 
-    ShaderVariableManagerVk& GetStaticVarMgr(Uint32 ShaderInd) const
+    const ShaderVariableManagerVk& GetStaticVarMgr(Uint32 ShaderInd) const
     {
         VERIFY_EXPR(ShaderInd < GetNumShaderStages());
         return m_StaticVarsMgrs[ShaderInd];
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp
index a5278cb7..7d77ff48 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.hpp
@@ -75,7 +75,7 @@ public:
     };
 
     // clang-format off
-    ShaderResourceCacheVk(DbgCacheContentType dbgContentType)
+    ShaderResourceCacheVk(DbgCacheContentType dbgContentType) noexcept
 #ifdef DILIGENT_DEBUG
         : m_DbgContentType{dbgContentType}
 #endif
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.hpp
index 0013fe23..9452a390 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceLayoutVk.hpp
@@ -128,7 +128,7 @@ public:
     };
     using TShaderStages = std::vector<ShaderStageInfo>;
 
-    ShaderResourceLayoutVk(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice) :
+    ShaderResourceLayoutVk(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice) noexcept :
         m_LogicalDevice{LogicalDevice}
     {
     }
diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
index 9039e04e..27689c1e 100644
--- a/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
+++ b/Graphics/GraphicsEngineVulkan/include/ShaderVariableVk.hpp
@@ -73,21 +73,25 @@ class ShaderVariableVkImpl;
 class ShaderVariableManagerVk
 {
 public:
-    ShaderVariableManagerVk(IObject&                             Owner,
-                            const ShaderResourceLayoutVk&        SrcLayout,
-                            IMemoryAllocator&                    Allocator,
-                            const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes,
-                            Uint32                               NumAllowedTypes,
-                            ShaderResourceCacheVk&               ResourceCache);
+    ShaderVariableManagerVk(IObject&               Owner,
+                            ShaderResourceCacheVk& ResourceCache) noexcept :
+        m_Owner{Owner},
+        m_ResourceCache{ResourceCache}
+    {}
+
+    void Initialize(const ShaderResourceLayoutVk&        SrcLayout,
+                    IMemoryAllocator&                    Allocator,
+                    const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes,
+                    Uint32                               NumAllowedTypes);
 
     ~ShaderVariableManagerVk();
 
     void DestroyVariables(IMemoryAllocator& Allocator);
 
-    ShaderVariableVkImpl* GetVariable(const Char* Name);
-    ShaderVariableVkImpl* GetVariable(Uint32 Index);
+    ShaderVariableVkImpl* GetVariable(const Char* Name) const;
+    ShaderVariableVkImpl* GetVariable(Uint32 Index) const;
 
-    void BindResources(IResourceMapping* pResourceMapping, Uint32 Flags);
+    void BindResources(IResourceMapping* pResourceMapping, Uint32 Flags) const;
 
     static size_t GetRequiredMemorySize(const ShaderResourceLayoutVk&        Layout,
                                         const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes,
@@ -115,7 +119,7 @@ private:
     Uint32                m_NumVariables = 0;
 
 #ifdef DILIGENT_DEBUG
-    IMemoryAllocator& m_DbgAllocator;
+    IMemoryAllocator* m_pDbgAllocator = nullptr;
 #endif
 };
 
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
index 6cbe5da9..83512870 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineStateVkImpl.cpp
@@ -413,15 +413,12 @@ void PipelineStateVkImpl::InitResourceLayouts(const PipelineStateCreateInfo& Cre
         const auto ShaderType    = StageInfo.Type;
         const auto ShaderTypeInd = GetShaderTypePipelineIndex(ShaderType, m_Desc.PipelineType);
 
-        new (m_ShaderResourceLayouts + s) ShaderResourceLayoutVk{LogicalDevice};
-
         m_ResourceLayoutIndex[ShaderTypeInd] = static_cast<Int8>(s);
 
-        auto* pStaticResLayout = new (m_ShaderResourceLayouts + ShaderStages.size() + s) ShaderResourceLayoutVk{LogicalDevice};
-        auto* pStaticResCache  = new (m_StaticResCaches + s) ShaderResourceCacheVk{ShaderResourceCacheVk::DbgCacheContentType::StaticShaderResources};
-        pStaticResLayout->InitializeStaticResourceLayout(StageInfo.pShader, GetRawAllocator(), m_Desc.ResourceLayout, m_StaticResCaches[s]);
+        auto& StaticResLayout = m_ShaderResourceLayouts[GetNumShaderStages() + s];
+        StaticResLayout.InitializeStaticResourceLayout(StageInfo.pShader, GetRawAllocator(), m_Desc.ResourceLayout, m_StaticResCaches[s]);
 
-        new (m_StaticVarsMgrs + s) ShaderVariableManagerVk{*this, *pStaticResLayout, GetRawAllocator(), nullptr, 0, *pStaticResCache};
+        m_StaticVarsMgrs[s].Initialize(StaticResLayout, GetRawAllocator(), nullptr, 0);
     }
     ShaderResourceLayoutVk::Initialize(pDeviceVk, ShaderStages, m_ShaderResourceLayouts, GetRawAllocator(),
                                        m_Desc.ResourceLayout, m_PipelineLayout,
@@ -464,37 +461,52 @@ void PipelineStateVkImpl::InitResourceLayouts(const PipelineStateCreateInfo& Cre
 }
 
 template <typename PSOCreateInfoType>
-LinearAllocator PipelineStateVkImpl::InitInternalObjects(const PSOCreateInfoType&                           CreateInfo,
-                                                         std::vector<VkPipelineShaderStageCreateInfo>&      vkShaderStages,
-                                                         std::vector<VulkanUtilities::ShaderModuleWrapper>& ShaderModules)
+void PipelineStateVkImpl::InitInternalObjects(const PSOCreateInfoType&                           CreateInfo,
+                                              std::vector<VkPipelineShaderStageCreateInfo>&      vkShaderStages,
+                                              std::vector<VulkanUtilities::ShaderModuleWrapper>& ShaderModules)
 {
     m_ResourceLayoutIndex.fill(-1);
 
     TShaderStages ShaderStages;
     ExtractShaders<ShaderVkImpl>(CreateInfo, ShaderStages);
 
-    // Memory must be released if an exception is thrown.
     LinearAllocator MemPool{GetRawAllocator()};
 
-    MemPool.AddSpace<ShaderResourceLayoutVk>(GetNumShaderStages() * 2);
-    MemPool.AddSpace<ShaderResourceCacheVk>(GetNumShaderStages());
-    MemPool.AddSpace<ShaderVariableManagerVk>(GetNumShaderStages());
+    const auto NumShaderStages = GetNumShaderStages();
+    VERIFY_EXPR(NumShaderStages > 0 && NumShaderStages == ShaderStages.size());
+
+    MemPool.AddSpace<ShaderResourceCacheVk>(NumShaderStages);
+    MemPool.AddSpace<ShaderResourceLayoutVk>(NumShaderStages * 2);
+    MemPool.AddSpace<ShaderVariableManagerVk>(NumShaderStages);
 
     ReserveSpaceForPipelineDesc(CreateInfo, MemPool);
 
     MemPool.Reserve();
 
-    m_ShaderResourceLayouts = MemPool.Allocate<ShaderResourceLayoutVk>(GetNumShaderStages() * 2);
-    m_StaticResCaches       = MemPool.Allocate<ShaderResourceCacheVk>(GetNumShaderStages());
-    m_StaticVarsMgrs        = MemPool.Allocate<ShaderVariableManagerVk>(GetNumShaderStages());
+    const auto& LogicalDevice = GetDevice()->GetLogicalDevice();
+
+    m_StaticResCaches = MemPool.ConstructArray<ShaderResourceCacheVk>(NumShaderStages, ShaderResourceCacheVk::DbgCacheContentType::StaticShaderResources);
+
+    // The memory is now owned by PipelineStateVkImpl and will be freed by Destruct().
+    auto* Ptr = MemPool.ReleaseOwnership();
+    VERIFY_EXPR(Ptr == m_StaticResCaches);
+    (void)Ptr;
+
+    m_ShaderResourceLayouts = MemPool.ConstructArray<ShaderResourceLayoutVk>(NumShaderStages * 2, LogicalDevice);
+
+    m_StaticVarsMgrs = MemPool.Allocate<ShaderVariableManagerVk>(NumShaderStages);
+    for (Uint32 s = 0; s < NumShaderStages; ++s)
+        new (m_StaticVarsMgrs + s) ShaderVariableManagerVk{*this, m_StaticResCaches[s]};
 
     InitializePipelineDesc(CreateInfo, MemPool);
+
+    // It is important to construct all objects before initializing them because if an exception is thrown,
+    // destructors will be called for all objects
+
     InitResourceLayouts(CreateInfo, ShaderStages);
 
     // Create shader modules and initialize shader stages
     InitPipelineShaderStages(GetDevice()->GetLogicalDevice(), ShaderStages, ShaderModules, vkShaderStages);
-
-    return MemPool;
 }
 
 PipelineStateVkImpl::PipelineStateVkImpl(IReferenceCounters*                    pRefCounters,
@@ -503,15 +515,20 @@ PipelineStateVkImpl::PipelineStateVkImpl(IReferenceCounters*
     TPipelineStateBase{pRefCounters, pDeviceVk, CreateInfo.PSODesc},
     m_SRBMemAllocator{GetRawAllocator()}
 {
-    std::vector<VkPipelineShaderStageCreateInfo>      vkShaderStages;
-    std::vector<VulkanUtilities::ShaderModuleWrapper> ShaderModules;
-
-    auto MemPool = InitInternalObjects(CreateInfo, vkShaderStages, ShaderModules);
+    try
+    {
+        std::vector<VkPipelineShaderStageCreateInfo>      vkShaderStages;
+        std::vector<VulkanUtilities::ShaderModuleWrapper> ShaderModules;
 
-    CreateGraphicsPipeline(pDeviceVk, vkShaderStages, m_PipelineLayout, m_Desc, GetGraphicsPipelineDesc(), m_Pipeline, m_pRenderPass);
+        InitInternalObjects(CreateInfo, vkShaderStages, ShaderModules);
 
-    void* Ptr = MemPool.Release();
-    VERIFY_EXPR(Ptr == m_ShaderResourceLayouts);
+        CreateGraphicsPipeline(pDeviceVk, vkShaderStages, m_PipelineLayout, m_Desc, GetGraphicsPipelineDesc(), m_Pipeline, m_pRenderPass);
+    }
+    catch (...)
+    {
+        Destruct();
+        throw;
+    }
 }
 
 
@@ -521,39 +538,59 @@ PipelineStateVkImpl::PipelineStateVkImpl(IReferenceCounters*                   p
     TPipelineStateBase{pRefCounters, pDeviceVk, CreateInfo.PSODesc},
     m_SRBMemAllocator{GetRawAllocator()}
 {
-    std::vector<VkPipelineShaderStageCreateInfo>      vkShaderStages;
-    std::vector<VulkanUtilities::ShaderModuleWrapper> ShaderModules;
-
-    auto MemPool = InitInternalObjects(CreateInfo, vkShaderStages, ShaderModules);
+    try
+    {
+        std::vector<VkPipelineShaderStageCreateInfo>      vkShaderStages;
+        std::vector<VulkanUtilities::ShaderModuleWrapper> ShaderModules;
 
-    CreateComputePipeline(pDeviceVk, vkShaderStages, m_PipelineLayout, m_Desc, m_Pipeline);
+        InitInternalObjects(CreateInfo, vkShaderStages, ShaderModules);
 
-    void* Ptr = MemPool.Release();
-    VERIFY_EXPR(Ptr == m_ShaderResourceLayouts);
+        CreateComputePipeline(pDeviceVk, vkShaderStages, m_PipelineLayout, m_Desc, m_Pipeline);
+    }
+    catch (...)
+    {
+        Destruct();
+        throw;
+    }
 }
 
 
 PipelineStateVkImpl::~PipelineStateVkImpl()
 {
+    Destruct();
+}
+
+void PipelineStateVkImpl::Destruct()
+{
     m_pDevice->SafeReleaseDeviceObject(std::move(m_Pipeline), m_Desc.CommandQueueMask);
     m_PipelineLayout.Release(m_pDevice, m_Desc.CommandQueueMask);
 
     auto& RawAllocator = GetRawAllocator();
-    for (Uint32 s = 0; s < GetNumShaderStages() * 2; ++s)
+    for (Uint32 s = 0; s < GetNumShaderStages(); ++s)
     {
-        m_ShaderResourceLayouts[s].~ShaderResourceLayoutVk();
+        if (m_StaticVarsMgrs != nullptr)
+        {
+            m_StaticVarsMgrs[s].DestroyVariables(GetRawAllocator());
+            m_StaticVarsMgrs[s].~ShaderVariableManagerVk();
+        }
+
+        if (m_ShaderResourceLayouts != nullptr)
+        {
+            m_ShaderResourceLayouts[s].~ShaderResourceLayoutVk();
+            m_ShaderResourceLayouts[GetNumShaderStages() + s].~ShaderResourceLayoutVk();
+        }
+
+        if (m_StaticResCaches != nullptr)
+        {
+            m_StaticResCaches[s].~ShaderResourceCacheVk();
+        }
     }
 
-    for (Uint32 s = 0; s < GetNumShaderStages(); ++s)
+    // All internal objects are allocted in contiguous chunks of memory.
+    if (void* pRawMem = m_StaticResCaches)
     {
-        m_StaticResCaches[s].~ShaderResourceCacheVk();
-        m_StaticVarsMgrs[s].DestroyVariables(GetRawAllocator());
-        m_StaticVarsMgrs[s].~ShaderVariableManagerVk();
+        RawAllocator.Free(pRawMem);
     }
-    // m_ShaderResourceLayouts, m_StaticResCaches and m_StaticVarsMgrs are allocted in
-    // contiguous chunks of memory.
-    void* pRawMem = m_ShaderResourceLayouts;
-    RawAllocator.Free(pRawMem);
 }
 
 IMPLEMENT_QUERY_INTERFACE(PipelineStateVkImpl, IID_PipelineStateVk, TPipelineStateBase)
@@ -746,7 +783,7 @@ IShaderResourceVariable* PipelineStateVkImpl::GetStaticVariableByIndex(SHADER_TY
     if (LayoutInd < 0)
         return nullptr;
 
-    auto& StaticVarMgr = GetStaticVarMgr(LayoutInd);
+    const auto& StaticVarMgr = GetStaticVarMgr(LayoutInd);
     return StaticVarMgr.GetVariable(Index);
 }
 
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
index 91008811..21b216db 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceBindingVkImpl.cpp
@@ -75,7 +75,8 @@ ShaderResourceBindingVkImpl::ShaderResourceBindingVkImpl(IReferenceCounters*  pR
         // Initialize vars manager to reference mutable and dynamic variables
         // Note that the cache has space for all variable types
         const SHADER_RESOURCE_VARIABLE_TYPE VarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC};
-        new (m_pShaderVarMgrs + s) ShaderVariableManagerVk{*this, SrcLayout, VarDataAllocator, VarTypes, _countof(VarTypes), m_ShaderResourceCache};
+        new (m_pShaderVarMgrs + s) ShaderVariableManagerVk{*this, m_ShaderResourceCache};
+        m_pShaderVarMgrs[s].Initialize(SrcLayout, VarDataAllocator, VarTypes, _countof(VarTypes));
     }
 #ifdef DILIGENT_DEBUG
     m_ShaderResourceCache.DbgVerifyResourceInitialization();
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp
index 82d26b44..7e06bd69 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderVariableVk.cpp
@@ -65,20 +65,15 @@ size_t ShaderVariableManagerVk::GetRequiredMemorySize(const ShaderResourceLayout
 }
 
 // Creates shader variable for every resource from SrcLayout whose type is one AllowedVarTypes
-ShaderVariableManagerVk::ShaderVariableManagerVk(IObject&                             Owner,
-                                                 const ShaderResourceLayoutVk&        SrcLayout,
-                                                 IMemoryAllocator&                    Allocator,
-                                                 const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes,
-                                                 Uint32                               NumAllowedTypes,
-                                                 ShaderResourceCacheVk&               ResourceCache) :
-    // clang-format off
-    m_Owner        {Owner        },
-    m_ResourceCache{ResourceCache}
+void ShaderVariableManagerVk::Initialize(const ShaderResourceLayoutVk&        SrcLayout,
+                                         IMemoryAllocator&                    Allocator,
+                                         const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes,
+                                         Uint32                               NumAllowedTypes)
+{
 #ifdef DILIGENT_DEBUG
-  , m_DbgAllocator {Allocator}
+    m_pDbgAllocator = &Allocator;
 #endif
-// clang-format on
-{
+
     const Uint32 AllowedTypeBits = GetAllowedTypeBits(AllowedVarTypes, NumAllowedTypes);
     VERIFY_EXPR(m_NumVariables == 0);
     auto MemSize = GetRequiredMemorySize(SrcLayout, AllowedVarTypes, NumAllowedTypes, m_NumVariables);
@@ -119,10 +114,10 @@ ShaderVariableManagerVk::~ShaderVariableManagerVk()
 
 void ShaderVariableManagerVk::DestroyVariables(IMemoryAllocator& Allocator)
 {
-    VERIFY(&m_DbgAllocator == &Allocator, "Incosistent alloctor");
-
     if (m_pVariables != nullptr)
     {
+        VERIFY(m_pDbgAllocator == &Allocator, "Incosistent alloctor");
+
         for (Uint32 v = 0; v < m_NumVariables; ++v)
             m_pVariables[v].~ShaderVariableVkImpl();
         Allocator.Free(m_pVariables);
@@ -130,7 +125,7 @@ void ShaderVariableManagerVk::DestroyVariables(IMemoryAllocator& Allocator)
     }
 }
 
-ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(const Char* Name)
+ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(const Char* Name) const
 {
     ShaderVariableVkImpl* pVar = nullptr;
     for (Uint32 v = 0; v < m_NumVariables; ++v)
@@ -147,7 +142,7 @@ ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(const Char* Name)
 }
 
 
-ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(Uint32 Index)
+ShaderVariableVkImpl* ShaderVariableManagerVk::GetVariable(Uint32 Index) const
 {
     if (Index >= m_NumVariables)
     {
@@ -178,7 +173,7 @@ Uint32 ShaderVariableManagerVk::GetVariableIndex(const ShaderVariableVkImpl& Var
     }
 }
 
-void ShaderVariableManagerVk::BindResources(IResourceMapping* pResourceMapping, Uint32 Flags)
+void ShaderVariableManagerVk::BindResources(IResourceMapping* pResourceMapping, Uint32 Flags) const
 {
     if (!pResourceMapping)
     {