Improved exception safety of pipeline state object construction

4 files changed, 92 insertions, 46 deletions

diff --git a/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.hpp b/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.hpp
index eab96f6a..07c8f39e 100644
--- a/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.hpp
+++ b/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.hpp
@@ -45,7 +45,7 @@ namespace Diligent
 class GLProgramResourceCache
 {
 public:
-    GLProgramResourceCache()
+    GLProgramResourceCache() noexcept
     {}
 
     ~GLProgramResourceCache();
diff --git a/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.hpp b/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.hpp
index 0c5692d0..93180774 100644
--- a/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.hpp
+++ b/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.hpp
@@ -114,6 +114,8 @@ private:
     void InitResourceLayouts(const std::vector<GLPipelineShaderStageInfo>& ShaderStages,
                              LinearAllocator&                              MemPool);
 
+    void Destruct();
+
     // Linked GL programs for every shader stage. Every pipeline needs to have its own programs
     // because resource bindings assigned by GLProgramResources::LoadUniforms depend on other
     // shader stages.
diff --git a/Graphics/GraphicsEngineOpenGL/src/GLProgramResourceCache.cpp b/Graphics/GraphicsEngineOpenGL/src/GLProgramResourceCache.cpp
index 95557557..84cdc46e 100644
--- a/Graphics/GraphicsEngineOpenGL/src/GLProgramResourceCache.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/GLProgramResourceCache.cpp
@@ -92,7 +92,9 @@ GLProgramResourceCache::~GLProgramResourceCache()
 
 void GLProgramResourceCache::Destroy(IMemoryAllocator& MemAllocator)
 {
-    VERIFY(IsInitialized(), "Resource cache is not initialized");
+    if (!IsInitialized())
+        return;
+
     VERIFY(m_pdbgMemoryAllocator == &MemAllocator, "The allocator does not match the one used to create resources");
 
     for (Uint32 cb = 0; cb < GetUBCount(); ++cb)
diff --git a/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
index 8bc3802e..8b429094 100644
--- a/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
@@ -40,22 +40,37 @@ namespace Diligent
 template <typename PSOCreateInfoType>
 void PipelineStateGLImpl::Initialize(const PSOCreateInfoType& CreateInfo, const std::vector<GLPipelineShaderStageInfo>& ShaderStages)
 {
-    // Memory must be released if an exception is thrown.
     LinearAllocator MemPool{GetRawAllocator()};
+    VERIFY_EXPR(m_NumShaderStages > 0 && m_NumShaderStages == ShaderStages.size());
+    if (!GetDevice()->GetDeviceCaps().Features.SeparablePrograms)
+        m_NumShaderStages = 1;
 
-    MemPool.AddSpace<GLProgramObj>(GetNumShaderStages());
-    MemPool.AddSpace<GLProgramResources>(GetNumShaderStages());
+    const auto NumPrograms = GetNumShaderStages();
+
+    MemPool.AddSpace<GLProgramObj>(NumPrograms);
+    MemPool.AddSpace<GLProgramResources>(NumPrograms);
     MemPool.AddSpace<SamplerPtr>(m_Desc.ResourceLayout.NumImmutableSamplers);
 
     ReserveSpaceForPipelineDesc(CreateInfo, MemPool);
 
     MemPool.Reserve();
 
-    InitResourceLayouts(ShaderStages, MemPool);
-    InitializePipelineDesc(CreateInfo, MemPool);
+    m_GLPrograms = MemPool.ConstructArray<GLProgramObj>(NumPrograms, false);
 
-    void* Ptr = MemPool.Release();
+    // The memory is now owned by PipelineStateVkImpl and will be freed by Destruct().
+    auto* Ptr = MemPool.ReleaseOwnership();
     VERIFY_EXPR(Ptr == m_GLPrograms);
+    (void)Ptr;
+
+    m_ProgramResources = MemPool.ConstructArray<GLProgramResources>(NumPrograms);
+
+    m_ImmutableSamplers = MemPool.ConstructArray<SamplerPtr>(m_Desc.ResourceLayout.NumImmutableSamplers);
+
+    // It is important to construct all objects before initializing them because if an exception is thrown,
+    // destructors will be called for all objects
+
+    InitResourceLayouts(ShaderStages, MemPool);
+    InitializePipelineDesc(CreateInfo, MemPool);
 }
 
 PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*                    pRefCounters,
@@ -74,26 +89,33 @@ PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*
     m_StaticResourceLayout{*this}
 // clang-format on
 {
-    std::vector<GLPipelineShaderStageInfo> ShaderStages;
-    ExtractShaders<ShaderGLImpl>(CreateInfo, ShaderStages);
+    try
+    {
+        std::vector<GLPipelineShaderStageInfo> ShaderStages;
+        ExtractShaders<ShaderGLImpl>(CreateInfo, ShaderStages);
+
+        RefCntAutoPtr<ShaderGLImpl> pTempPS;
+        if (CreateInfo.pPS == nullptr)
+        {
+            // Some OpenGL implementations fail if fragment shader is not present, so
+            // create a dummy one.
+            ShaderCreateInfo ShaderCI;
+            ShaderCI.SourceLanguage  = SHADER_SOURCE_LANGUAGE_GLSL;
+            ShaderCI.Source          = "void main(){}";
+            ShaderCI.Desc.ShaderType = SHADER_TYPE_PIXEL;
+            ShaderCI.Desc.Name       = "Dummy fragment shader";
+            pDeviceGL->CreateShader(ShaderCI, reinterpret_cast<IShader**>(static_cast<ShaderGLImpl**>(&pTempPS)));
+
+            ShaderStages.emplace_back(SHADER_TYPE_PIXEL, pTempPS);
+            m_ShaderStageTypes[m_NumShaderStages++] = SHADER_TYPE_PIXEL;
+        }
 
-    RefCntAutoPtr<ShaderGLImpl> pTempPS;
-    if (CreateInfo.pPS == nullptr)
+        Initialize(CreateInfo, ShaderStages);
+    }
+    catch (...)
     {
-        // Some OpenGL implementations fail if fragment shader is not present, so
-        // create a dummy one.
-        ShaderCreateInfo ShaderCI;
-        ShaderCI.SourceLanguage  = SHADER_SOURCE_LANGUAGE_GLSL;
-        ShaderCI.Source          = "void main(){}";
-        ShaderCI.Desc.ShaderType = SHADER_TYPE_PIXEL;
-        ShaderCI.Desc.Name       = "Dummy fragment shader";
-        pDeviceGL->CreateShader(ShaderCI, reinterpret_cast<IShader**>(static_cast<ShaderGLImpl**>(&pTempPS)));
-
-        ShaderStages.emplace_back(SHADER_TYPE_PIXEL, pTempPS);
-        m_ShaderStageTypes[m_NumShaderStages++] = SHADER_TYPE_PIXEL;
+        Destruct();
     }
-
-    Initialize(CreateInfo, ShaderStages);
 }
 
 PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*                   pRefCounters,
@@ -112,30 +134,54 @@ PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*                   p
     m_StaticResourceLayout{*this}
 // clang-format on
 {
-    std::vector<GLPipelineShaderStageInfo> ShaderStages;
-    ExtractShaders<ShaderGLImpl>(CreateInfo, ShaderStages);
+    try
+    {
+        std::vector<GLPipelineShaderStageInfo> ShaderStages;
+        ExtractShaders<ShaderGLImpl>(CreateInfo, ShaderStages);
 
-    Initialize(CreateInfo, ShaderStages);
+        Initialize(CreateInfo, ShaderStages);
+    }
+    catch (...)
+    {
+        Destruct();
+    }
 }
 
 PipelineStateGLImpl::~PipelineStateGLImpl()
 {
+    Destruct();
+}
+
+void PipelineStateGLImpl::Destruct()
+{
     auto& RawAllocator = GetRawAllocator();
     m_StaticResourceCache.Destroy(RawAllocator);
     GetDevice()->OnDestroyPSO(this);
 
-    for (Uint32 i = 0; i < GetNumShaderStages(); ++i)
+    if (m_ImmutableSamplers != nullptr)
     {
-        m_GLPrograms[i].~GLProgramObj();
-        m_ProgramResources[i].~GLProgramResources();
+        for (Uint32 i = 0; i < m_Desc.ResourceLayout.NumImmutableSamplers; ++i)
+        {
+            m_ImmutableSamplers[i].~SamplerPtr();
+        }
     }
-    for (Uint32 i = 0; i < m_Desc.ResourceLayout.NumImmutableSamplers; ++i)
+
+    for (Uint32 i = 0; i < GetNumShaderStages(); ++i)
     {
-        m_ImmutableSamplers[i].~SamplerPtr();
+        if (m_GLPrograms != nullptr)
+        {
+            m_GLPrograms[i].~GLProgramObj();
+        }
+        if (m_ProgramResources != nullptr)
+        {
+            m_ProgramResources[i].~GLProgramResources();
+        }
     }
 
-    void* pRawMem = m_GLPrograms;
-    RawAllocator.Free(pRawMem);
+    if (void* pRawMem = m_GLPrograms)
+    {
+        RawAllocator.Free(pRawMem);
+    }
 }
 
 IMPLEMENT_QUERY_INTERFACE(PipelineStateGLImpl, IID_PipelineStateGL, TPipelineStateBase)
@@ -145,8 +191,8 @@ void PipelineStateGLImpl::InitResourceLayouts(const std::vector<GLPipelineShader
                                               LinearAllocator&                              MemPool)
 {
     auto* const pDeviceGL  = GetDevice();
-    const auto& DeviceCaps = pDeviceGL->GetDeviceCaps();
-    VERIFY(DeviceCaps.DevType != RENDER_DEVICE_TYPE_UNDEFINED, "Device caps are not initialized");
+    const auto& deviceCaps = pDeviceGL->GetDeviceCaps();
+    VERIFY(deviceCaps.DevType != RENDER_DEVICE_TYPE_UNDEFINED, "Device caps are not initialized");
 
     auto pImmediateCtx = m_pDevice->GetImmediateContext();
     VERIFY_EXPR(pImmediateCtx);
@@ -157,18 +203,16 @@ void PipelineStateGLImpl::InitResourceLayouts(const std::vector<GLPipelineShader
         m_TotalSamplerBindings       = 0;
         m_TotalImageBindings         = 0;
         m_TotalStorageBufferBindings = 0;
-        if (DeviceCaps.Features.SeparablePrograms)
+        if (deviceCaps.Features.SeparablePrograms)
         {
             // Program pipelines are not shared between GL contexts, so we cannot create
             // it now
             m_ShaderResourceLayoutHash = 0;
-            m_GLPrograms               = MemPool.Allocate<GLProgramObj>(ShaderStages.size());
-            m_ProgramResources         = MemPool.ConstructArray<GLProgramResources>(ShaderStages.size());
             for (size_t i = 0; i < ShaderStages.size(); ++i)
             {
                 auto*       pShaderGL  = ShaderStages[i].pShader;
                 const auto& ShaderDesc = pShaderGL->GetDesc();
-                new (m_GLPrograms + i) GLProgramObj{ShaderGLImpl::LinkProgram(&pShaderGL, 1, true)};
+                m_GLPrograms[i]        = GLProgramObj{ShaderGLImpl::LinkProgram(&pShaderGL, 1, true)};
                 // Load uniforms and assign bindings
                 m_ProgramResources[i].LoadUniforms(ShaderDesc.ShaderType, m_GLPrograms[i], GLState,
                                                    m_TotalUniformBufferBindings,
@@ -191,8 +235,7 @@ void PipelineStateGLImpl::InitResourceLayouts(const std::vector<GLPipelineShader
                 ActiveStages |= Stage.Type;
             }
 
-            m_GLPrograms       = MemPool.Construct<GLProgramObj>(ShaderGLImpl::LinkProgram(Shaders.data(), static_cast<Uint32>(ShaderStages.size()), false));
-            m_ProgramResources = MemPool.Construct<GLProgramResources>();
+            m_GLPrograms[0] = ShaderGLImpl::LinkProgram(Shaders.data(), static_cast<Uint32>(Shaders.size()), false);
 
             m_ProgramResources[0].LoadUniforms(ActiveStages, m_GLPrograms[0], GLState,
                                                m_TotalUniformBufferBindings,
@@ -204,10 +247,9 @@ void PipelineStateGLImpl::InitResourceLayouts(const std::vector<GLPipelineShader
         }
 
         // Initialize master resource layout that keeps all variable types and does not reference a resource cache
-        m_ResourceLayout.Initialize(m_ProgramResources, static_cast<Uint32>(ShaderStages.size()), m_Desc.PipelineType, m_Desc.ResourceLayout, nullptr, 0, nullptr);
+        m_ResourceLayout.Initialize(m_ProgramResources, GetNumShaderStages(), m_Desc.PipelineType, m_Desc.ResourceLayout, nullptr, 0, nullptr);
     }
 
-    m_ImmutableSamplers = MemPool.ConstructArray<SamplerPtr>(m_Desc.ResourceLayout.NumImmutableSamplers);
     for (Uint32 s = 0; s < m_Desc.ResourceLayout.NumImmutableSamplers; ++s)
     {
         pDeviceGL->CreateSampler(m_Desc.ResourceLayout.ImmutableSamplers[s].Desc, &m_ImmutableSamplers[s]);
@@ -216,7 +258,7 @@ void PipelineStateGLImpl::InitResourceLayouts(const std::vector<GLPipelineShader
     {
         // Clone only static variables into static resource layout, assign and initialize static resource cache
         const SHADER_RESOURCE_VARIABLE_TYPE StaticVars[] = {SHADER_RESOURCE_VARIABLE_TYPE_STATIC};
-        m_StaticResourceLayout.Initialize(m_ProgramResources, static_cast<Uint32>(ShaderStages.size()), m_Desc.PipelineType, m_Desc.ResourceLayout, StaticVars, _countof(StaticVars), &m_StaticResourceCache);
+        m_StaticResourceLayout.Initialize(m_ProgramResources, GetNumShaderStages(), m_Desc.PipelineType, m_Desc.ResourceLayout, StaticVars, _countof(StaticVars), &m_StaticResourceCache);
         InitImmutableSamplersInResourceCache(m_StaticResourceLayout, m_StaticResourceCache);
     }
 }