From a1285d6d85afc99e8f997a5a5d1bbfd179f789f7 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 19:21:32 -0700 Subject: [PATCH 01/36] GPUUploadManager: wake upload schedulers when pages return to pool --- .../include/GPUUploadManagerImpl.hpp | 6 ++--- .../src/GPUUploadManagerImpl.cpp | 22 +++++++++++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp index 1b92e2ab29..36c591198e 100644 --- a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp +++ b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp @@ -348,14 +348,14 @@ class GPUUploadManagerImpl final : public ObjectBase bool TryEnqueuePage(Page* P); void ProcessPagesToRelease(IDeviceContext* pContext); void AddFreePages(IDeviceContext* pContext); - void AddFreePage(Page* pPage) { m_FreePages.Push(pPage); } + void ReturnFreePage(Page* pPage); bool ScheduleUpdate(IDeviceContext* pContext, Uint32 UpdateSize, const void* pUpdateInfo, bool ScheduleUpdate(Page::Writer& Writer, const void* pUpdateInfo)); void ReleaseStagingBuffers(IDeviceContext* pContext); - void SignalPageRotated() { m_PageRotatedSignal.Tick(); } + void SignalPageRotated() { m_PagePoolChangedSignal.Tick(); } void SignalStop(); Uint32 GetPageSize() const { return m_PageSize; } @@ -370,7 +370,7 @@ class GPUUploadManagerImpl final : public ObjectBase std::atomic m_pCurrentPage{nullptr}; - Threading::TickSignal m_PageRotatedSignal; + Threading::TickSignal m_PagePoolChangedSignal; std::unordered_map> m_Pages; std::map m_PageSizeToCount; diff --git a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp index 8e2585bc05..6a41961e4e 100644 --- a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp +++ b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp @@ -913,7 +913,7 @@ bool GPUUploadManagerImpl::Page::TryEnqueue() void GPUUploadManagerImpl::Page::Recycle() { - m_pStream->AddFreePage(this); + m_pStream->ReturnFreePage(this); } void GPUUploadManagerImpl::Page::ReleaseStagingBuffer(IDeviceContext* pContext) @@ -1172,7 +1172,15 @@ void GPUUploadManagerImpl::UploadStream::ReleaseStagingBuffers(IDeviceContext* p void GPUUploadManagerImpl::UploadStream::SignalStop() { - m_PageRotatedSignal.RequestStop(); + m_PagePoolChangedSignal.RequestStop(); +} + +void GPUUploadManagerImpl::UploadStream::ReturnFreePage(Page* pPage) +{ + // Publish the page before waking schedulers so a waiter that observes the tick + // can immediately acquire the returned page. + m_FreePages.Push(pPage); + m_PagePoolChangedSignal.Tick(); } bool GPUUploadManagerImpl::TryBeginScheduleUpdate() noexcept @@ -1368,7 +1376,7 @@ bool GPUUploadManagerImpl::UploadStream::ScheduleUpdate(IDeviceContext* pContext bool UpdateScheduled = false; auto UpdatePendingSizeAndTryRotate = [&](Page* P) { - Uint64 PageEpoch = m_PageRotatedSignal.CurrentEpoch(); + Uint64 PageEpoch = m_PagePoolChangedSignal.CurrentEpoch(); // Note that for texture pages, UpdateSize is the texture dimension. if (!TryRotatePage(pContext, P, UpdateSize)) { @@ -1379,7 +1387,7 @@ bool GPUUploadManagerImpl::UploadStream::ScheduleUpdate(IDeviceContext* pContext m_TotalPendingUpdateSize.fetch_add(UpdateSize, std::memory_order_acq_rel); IsFirstAttempt = false; } - AbortUpdate = m_PageRotatedSignal.WaitNext(PageEpoch) == 0; + AbortUpdate = m_PagePoolChangedSignal.WaitNext(PageEpoch) == 0; } }; @@ -1621,7 +1629,7 @@ bool GPUUploadManagerImpl::UploadStream::TryRotatePage(IDeviceContext* pContext, // free list only if sealing observes no active writers; otherwise the // last writer will recycle the empty page through TryEnqueuePage(). if (Fresh->TrySeal() == Page::SealStatus::Ready) - m_FreePages.Push(Fresh); + ReturnFreePage(Fresh); return true; // Rotation happened by someone else } @@ -1629,7 +1637,7 @@ bool GPUUploadManagerImpl::UploadStream::TryRotatePage(IDeviceContext* pContext, if (ExpectedCurrent != nullptr && ExpectedCurrent->TrySeal() == Page::SealStatus::Ready) TryEnqueuePage(ExpectedCurrent); - m_PageRotatedSignal.Tick(); + m_PagePoolChangedSignal.Tick(); return true; } @@ -1646,7 +1654,7 @@ bool GPUUploadManagerImpl::UploadStream::TryEnqueuePage(Page* P) else { P->Reset(nullptr); - m_FreePages.Push(P); + ReturnFreePage(P); } return true; } From 21d9116827e9f870b365db04a046620eeb1584db Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 19:25:45 -0700 Subject: [PATCH 02/36] Fix GPU upload manager test constants --- Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp b/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp index d367d56bff..6abff0c4ee 100644 --- a/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp +++ b/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp @@ -794,8 +794,8 @@ TEST(GPUUploadManagerTest, StopReleasesBlockedBufferUpdates) RefCntAutoPtr pBuffer = CreateUploadTestBuffer(pDevice, BufferData.size()); ASSERT_NE(pBuffer, nullptr); - constexpr size_t kNumThreads = 4; - constexpr size_t kNumUpdates = kNumThreads * 2; + static constexpr size_t kNumThreads = 4; + static constexpr size_t kNumUpdates = kNumThreads * 2; std::vector Threads; std::atomic NumUpdatesRunning{0}; std::atomic NumCopyCallbacks{0}; @@ -918,8 +918,8 @@ TEST(GPUUploadManagerTest, StopReleasesBlockedTextureUpdates) CreateGPUUploadManager(CreateInfo, &pUploadManager); ASSERT_TRUE(pUploadManager != nullptr); - constexpr size_t kNumThreads = 4; - constexpr size_t kNumUpdates = kNumThreads * 2; + static constexpr size_t kNumThreads = 4; + static constexpr size_t kNumUpdates = kNumThreads * 2; std::vector Threads; std::atomic NumUpdatesRunning{0}; std::atomic NumCopyCallbacks{0}; From 36c6d1badd6ffcf7c1ecf57d6e39a28cc22862b8 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 19:44:49 -0700 Subject: [PATCH 03/36] GPUUploadManager: handle page creation failures --- .../include/GPUUploadManagerImpl.hpp | 4 +- .../src/GPUUploadManagerImpl.cpp | 58 ++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp index 36c591198e..b489e616a1 100644 --- a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp +++ b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp @@ -149,7 +149,8 @@ class GPUUploadManagerImpl final : public ObjectBase void Seal(); void ExecutePendingOps(IDeviceContext* pContext, Uint64 FenceValue); - void Reset(IDeviceContext* pContext); + bool Reset(IDeviceContext* pContext); + bool IsValid() const; // Tries to set the page as enqueued for execution. // Returns true if the page was not previously enqueued, false otherwise. @@ -212,6 +213,7 @@ class GPUUploadManagerImpl final : public ObjectBase void* Map(IDeviceContext* pContext); void Unmap(IDeviceContext* pContext); void Reset(); + bool IsValid() const { return pTex != nullptr; } DynamicAtlasManager::Region Allocate(Uint32 Width, Uint32 Height); diff --git a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp index 6a41961e4e..e8e7e5637d 100644 --- a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp +++ b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp @@ -359,7 +359,10 @@ GPUUploadManagerImpl::Page::StagingTextureAtlas::StagingTextureAtlas(IRenderDevi TexDesc.CPUAccessFlags = CPU_ACCESS_WRITE; pDevice->CreateTexture(TexDesc, nullptr, &pTex); - VERIFY_EXPR(pTex); + if (!pTex) + { + LOG_ERROR_MESSAGE("Failed to create GPU upload manager staging texture '", Name, "'"); + } } GPUUploadManagerImpl::Page::StagingTextureAtlas::~StagingTextureAtlas() @@ -369,6 +372,9 @@ GPUUploadManagerImpl::Page::StagingTextureAtlas::~StagingTextureAtlas() void* GPUUploadManagerImpl::Page::StagingTextureAtlas::Map(IDeviceContext* pContext) { + if (!pTex) + return nullptr; + pContext->TransitionResourceState({pTex, RESOURCE_STATE_UNKNOWN, RESOURCE_STATE_COPY_SOURCE, STATE_TRANSITION_FLAG_UPDATE_STATE}); MappedTextureSubresource MappedData; pContext->MapTextureSubresource(pTex, 0, 0, MAP_WRITE, MAP_FLAG_NONE, nullptr, MappedData); @@ -426,7 +432,10 @@ GPUUploadManagerImpl::Page::Page(UploadStream* pStream, IRenderDevice* pDevice, Desc.Usage = USAGE_STAGING; Desc.CPUAccessFlags = CPU_ACCESS_WRITE; pDevice->CreateBuffer(Desc, nullptr, &m_pStagingBuffer); - VERIFY_EXPR(m_pStagingBuffer != nullptr); + if (!m_pStagingBuffer) + { + LOG_ERROR_MESSAGE("Failed to create GPU upload manager staging buffer '", Name, "'"); + } } GPUUploadManagerImpl::Page::Page(UploadStream* pStream, IRenderDevice* pDevice, Uint32 Size, TEXTURE_FORMAT Format) : @@ -445,6 +454,11 @@ GPUUploadManagerImpl::Page::Page(UploadStream* pStream, IRenderDevice* pDevice, { } +bool GPUUploadManagerImpl::Page::IsValid() const +{ + return (m_pStagingBuffer != nullptr) || (m_pStagingAtlas != nullptr && m_pStagingAtlas->IsValid()); +} + GPUUploadManagerImpl::Page::~Page() { for (PendingOp Op; m_PendingOps.Dequeue(Op);) @@ -868,7 +882,7 @@ void GPUUploadManagerImpl::Page::ExecutePendingOps(IDeviceContext* pContext, Uin m_FenceValue = FenceValue; } -void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext) +bool GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext) { VERIFY(DbgGetWriterCount() == 0, "All writers must finish before resetting the page"); VERIFY(m_PendingOps.IsEmpty(), "All pending operations must be executed before resetting the page"); @@ -898,8 +912,14 @@ void GPUUploadManagerImpl::Page::Reset(IDeviceContext* pContext) m_pData = m_pStagingAtlas->Map(pContext); } - VERIFY_EXPR(m_pData != nullptr); + if (m_pData == nullptr) + { + LOG_ERROR_MESSAGE("Failed to map GPU upload manager staging page"); + return false; + } } + + return true; } bool GPUUploadManagerImpl::Page::TryEnqueue() @@ -1151,7 +1171,10 @@ GPUUploadManagerImpl::GPUUploadManagerImpl(IReferenceCounters* pRefCounters, con Desc.Name = "GPU upload manager fence"; Desc.Type = FENCE_TYPE_CPU_WAIT_ONLY; m_pDevice->CreateFence(Desc, &m_pFence); - VERIFY_EXPR(m_pFence != nullptr); + if (!m_pFence) + { + LOG_ERROR_AND_THROW("Failed to create GPU upload manager fence"); + } if (m_DeviceType == RENDER_DEVICE_TYPE_D3D11) { @@ -1380,6 +1403,12 @@ bool GPUUploadManagerImpl::UploadStream::ScheduleUpdate(IDeviceContext* pContext // Note that for texture pages, UpdateSize is the texture dimension. if (!TryRotatePage(pContext, P, UpdateSize)) { + if (pContext != nullptr) + { + AbortUpdate = true; + return; + } + // Atomically update the max pending update size to ensure the next page is large enough AtomicMax(m_MaxPendingUpdateSize, UpdateSize, std::memory_order_acq_rel); if (IsFirstAttempt) @@ -1578,11 +1607,18 @@ GPUUploadManagerImpl::Page* GPUUploadManagerImpl::UploadStream::CreatePage(IDevi std::make_unique(this, m_Mgr.m_pDevice, PageSize, m_Format) : std::make_unique(this, m_Mgr.m_pDevice, PageSize); + if (!NewPage->IsValid()) + return nullptr; + Page* P = NewPage.get(); if (pContext != nullptr) { - P->Reset(pContext); + if (!P->Reset(pContext)) + { + return nullptr; + } } + m_Pages.emplace(P, std::move(NewPage)); m_PageSizeToCount[PageSize]++; m_PeakPageCount = std::max(m_PeakPageCount, static_cast(m_Pages.size())); @@ -1674,8 +1710,14 @@ void GPUUploadManagerImpl::ReclaimCompletedPages(IDeviceContext* pContext) Page* P = m_InFlightPages[i]; if (P->GetFenceValue() <= CompletedFenceValue) { - P->Reset(pContext); - m_TmpPages.push_back(P); + if (P->Reset(pContext)) + { + m_TmpPages.push_back(P); + } + else + { + m_InFlightPages[NewInFlightPageCount++] = P; + } } else { From b8958715a2c3447c99688ecb12ab3413e102b8af Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 21:01:29 -0700 Subject: [PATCH 04/36] GPUUploadManager: fix interface identity Add an IID and QueryInterface mapping for IGPUUploadManager, and correct the C interface vtable to use IObject as the base interface. Document that all GPU upload callbacks must be non-throwing, and cover the IID path in include/API tests. --- .../GraphicsTools/include/GPUUploadManagerImpl.hpp | 2 ++ Graphics/GraphicsTools/interface/GPUUploadManager.h | 13 ++++++++++++- .../src/GPUUploadManagerTest.cpp | 6 ++++++ .../GraphicsTools/GPUUploadManagerH_test.c | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp index b489e616a1..70129ce207 100644 --- a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp +++ b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp @@ -59,6 +59,8 @@ class GPUUploadManagerImpl final : public ObjectBase GPUUploadManagerImpl(IReferenceCounters* pRefCounters, const GPUUploadManagerCreateInfo& CI); ~GPUUploadManagerImpl(); + IMPLEMENT_QUERY_INTERFACE_IN_PLACE(IID_GPUUploadManager, TBase); + virtual void DILIGENT_CALL_TYPE RenderThreadUpdate(IDeviceContext* pContext) override final; virtual bool DILIGENT_CALL_TYPE ScheduleBufferUpdate(const ScheduleBufferUpdateInfo& UpdateInfo) override final; diff --git a/Graphics/GraphicsTools/interface/GPUUploadManager.h b/Graphics/GraphicsTools/interface/GPUUploadManager.h index b631eeffff..0fbee64620 100644 --- a/Graphics/GraphicsTools/interface/GPUUploadManager.h +++ b/Graphics/GraphicsTools/interface/GPUUploadManager.h @@ -82,6 +82,7 @@ typedef struct GPUUploadManagerCreateInfo GPUUploadManagerCreateInfo; /// \warning Reentrancy / thread-safety: /// The callback is executed from inside IGPUUploadManager::ScheduleBufferUpdate(). /// The callback MUST NOT call back into the same IGPUUploadManager instance. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. typedef void (*WriteStagingBufferDataCallbackType)(void* pDstData, Uint32 NumBytes, void* pUserData); @@ -105,6 +106,7 @@ typedef void (*WriteStagingBufferDataCallbackType)(void* pDstData, Uint32 NumByt /// perform actions that may synchronously trigger RenderThreadUpdate() or otherwise /// re-enter the manager, as this may lead to deadlocks, unbounded recursion, or /// inconsistent internal state. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. /// /// If follow-up work is required, the callback should only enqueue work to be /// processed later (e.g. push a task into a user-owned queue) and return promptly. @@ -143,6 +145,7 @@ typedef void (*GPUBufferUploadEnqueuedCallbackType)(IBuffer* pDstBuffer, /// perform actions that may synchronously trigger RenderThreadUpdate() or otherwise /// re-enter the manager, as this may lead to deadlocks, unbounded recursion, or /// inconsistent internal state. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. /// /// If follow-up work is required, the callback should only enqueue work to be /// processed later (e.g. push a task into a user-owned queue) and return promptly. @@ -280,6 +283,7 @@ typedef struct ScheduleBufferUpdateInfo ScheduleBufferUpdateInfo; /// \warning Reentrancy / thread-safety: /// The callback is executed from inside IGPUUploadManager::ScheduleTextureUpdate(). /// The callback MUST NOT call back into the same IGPUUploadManager instance. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. typedef void (*WriteStagingTextureDataCallbackType)(void* pDstData, Uint32 Stride, Uint32 DepthStride, @@ -307,6 +311,7 @@ typedef void (*WriteStagingTextureDataCallbackType)(void* pDstData, /// perform actions that may synchronously trigger RenderThreadUpdate() or otherwise /// re-enter the manager, as this may lead to deadlocks, unbounded recursion, or /// inconsistent internal state. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. /// /// If follow-up work is required, the callback should only enqueue work to be /// processed later (e.g. push a task into a user-owned queue) and return promptly. @@ -348,6 +353,7 @@ typedef void (*GPUTextureUploadEnqueuedCallbackType)(ITexture* pDstTexture, /// perform actions that may synchronously trigger RenderThreadUpdate() or otherwise /// re-enter the manager, as this may lead to deadlocks, unbounded recursion, or /// inconsistent internal state. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. /// /// If follow-up work is required, the callback should only enqueue work to be /// processed later (e.g. push a task into a user-owned queue) and return promptly. @@ -382,6 +388,7 @@ typedef void (*CopyStagingTextureCallbackType)(IDeviceContext* pCont /// If scheduling is abandoned, it may be executed with a null context pointer /// from inside ScheduleTextureUpdate() or the manager stop/destruction path. /// The callback MUST NOT call back into the same IGPUUploadManager instance. +/// The callback MUST NOT throw exceptions. It must handle any errors internally. typedef void (*CopyStagingD3D11TextureCallbackType)(IDeviceContext* pContext, Uint32 DstMipLevel, Uint32 DstSlice, @@ -547,11 +554,15 @@ typedef struct GPUUploadManagerStats GPUUploadManagerStats; // clang-format off +// {1C5CF903-9E24-4B2C-9D63-FE63D49BE1F6} +static DILIGENT_CONSTEXPR INTERFACE_ID IID_GPUUploadManager = + { 0x1c5cf903, 0x9e24, 0x4b2c, { 0x9d, 0x63, 0xfe, 0x63, 0xd4, 0x9b, 0xe1, 0xf6 } }; + #define DILIGENT_INTERFACE_NAME IGPUUploadManager #include "../../../Primitives/interface/DefineInterfaceHelperMacros.h" #define IGPUUploadManagerInclusiveMethods \ - IDeviceObjectInclusiveMethods; \ + IObjectInclusiveMethods; \ IGPUUploadManagerMethods GPUUploadManager /// Asynchronous GPU upload manager diff --git a/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp b/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp index 6abff0c4ee..d61fc0613e 100644 --- a/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp +++ b/Tests/DiligentCoreAPITest/src/GPUUploadManagerTest.cpp @@ -87,6 +87,12 @@ TEST(GPUUploadManagerTest, Creation) GPUUploadManagerCreateInfo CreateInfo{pDevice, pContext}; CreateGPUUploadManager(CreateInfo, &pUploadManager); ASSERT_TRUE(pUploadManager != nullptr); + + RefCntAutoPtr pObject{pUploadManager, IID_Unknown}; + ASSERT_TRUE(pObject != nullptr); + + RefCntAutoPtr pQueriedManager{pObject, IID_GPUUploadManager}; + EXPECT_EQ(pQueriedManager.RawPtr(), pUploadManager.RawPtr()); } void VerifyBufferContents(IBuffer* pBuffer, const std::vector& ExpectedData) diff --git a/Tests/IncludeTest/GraphicsTools/GPUUploadManagerH_test.c b/Tests/IncludeTest/GraphicsTools/GPUUploadManagerH_test.c index 424c0beafd..28ca728956 100644 --- a/Tests/IncludeTest/GraphicsTools/GPUUploadManagerH_test.c +++ b/Tests/IncludeTest/GraphicsTools/GPUUploadManagerH_test.c @@ -28,6 +28,9 @@ void TestRenderStateCacheCInterface() { + const INTERFACE_ID* pIID = &IID_GPUUploadManager; + (void)pIID; + GPUUploadManagerCreateInfo CI; CI.pDevice = NULL; CI.pContext = NULL; From 714594b8cd2d01f2662dcc73fbe5715d24fb6d69 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 21:25:53 -0700 Subject: [PATCH 05/36] GPUUploadManager: require non-null context for Stop --- .../include/GPUUploadManagerImpl.hpp | 1 + .../interface/GPUUploadManager.h | 18 +++++---- .../src/GPUUploadManagerImpl.cpp | 38 ++++++++++++------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp index 70129ce207..29e148a109 100644 --- a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp +++ b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp @@ -448,6 +448,7 @@ class GPUUploadManagerImpl final : public ObjectBase bool TryBeginScheduleUpdate() noexcept; void EndScheduleUpdate() noexcept; bool SetStopping() noexcept; + void StopInternal(IDeviceContext* pContext); static constexpr Uint32 SCHEDULE_STOP_BIT = 0x80000000u; static constexpr Uint32 SCHEDULE_COUNT_MASK = ~SCHEDULE_STOP_BIT; diff --git a/Graphics/GraphicsTools/interface/GPUUploadManager.h b/Graphics/GraphicsTools/interface/GPUUploadManager.h index 0fbee64620..2d4e99dc39 100644 --- a/Graphics/GraphicsTools/interface/GPUUploadManager.h +++ b/Graphics/GraphicsTools/interface/GPUUploadManager.h @@ -656,8 +656,7 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// Permanently stops the upload manager. /// - /// \param [in] pContext - Device context used to release staging resources. If null, the manager - /// uses the context provided at creation or during RenderThreadUpdate(), if any. + /// \param [in] pContext - Device context used to release staging resources. Must not be null. /// /// The method wakes any threads blocked in ScheduleBufferUpdate() or ScheduleTextureUpdate(). /// After this call, the manager must not be used for new upload scheduling, render-thread @@ -668,10 +667,11 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// will invoke their callbacks with null handles during manager teardown. RenderThreadUpdate() /// and GetStats() calls after Stop() are misuse. /// - /// Stop() waits for admitted scheduling calls to return and releases staging resources using pContext - /// or the manager's stored context. The context must be the same context used by RenderThreadUpdate(), - /// if any, and must not be used concurrently while Stop() is executing. Internal stream/page objects - /// remain alive until the manager is destroyed. + /// Stop() waits for admitted scheduling calls to return and releases staging resources using pContext. + /// If the manager context has not been set yet, pContext becomes the manager context. + /// Otherwise, pContext must be the same as the manager context. The context must not be used + /// concurrently while Stop() is executing. Internal stream/page objects remain alive until the + /// manager is destroyed. /// /// The intended use is to call Stop() once, normally from the render thread. /// Multiple and parallel Stop() calls are allowed, but only the first call performs @@ -683,8 +683,10 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// ScheduleTextureUpdate(). The manager must remain alive until these calls have returned. /// Stop() must not be called concurrently with RenderThreadUpdate() or GetStats(). /// - /// If Stop() is not called explicitly, it is called by the manager destructor using the stored - /// context from whichever thread releases the last reference. + /// If Stop() is not called explicitly, the manager destructor performs the same internal stop + /// sequence using the stored manager context from whichever thread releases the last reference. + /// If no context has ever been provided to the manager, destructor cleanup proceeds without a + /// context; in this case no staging resources have been mapped through the manager. VIRTUAL void METHOD(Stop)(THIS_ IDeviceContext* pContext) PURE; }; diff --git a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp index e8e7e5637d..ff87462521 100644 --- a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp +++ b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp @@ -1266,23 +1266,32 @@ GPUUploadManagerImpl::ScheduleUpdateGuard::~ScheduleUpdateGuard() void GPUUploadManagerImpl::Stop(IDeviceContext* pContext) { - if (!SetStopping()) + if (pContext == nullptr) + { + LOG_ERROR_MESSAGE("A valid context must be provided to Stop()"); return; + } - m_Stopping.store(true, std::memory_order_release); - - if (pContext != nullptr) + if (!m_pContext) { - if (!m_pContext) - m_pContext = pContext; - else - DEV_CHECK_ERR(pContext == m_pContext, "The context provided to Stop must be the same as the one used to create the GPUUploadManagerImpl"); + m_pContext = pContext; } - else + else if (pContext != m_pContext) { - pContext = m_pContext; + LOG_ERROR_MESSAGE("The context provided to Stop() must be the same as the one already used by the GPUUploadManagerImpl"); + return; } + StopInternal(pContext); +} + +void GPUUploadManagerImpl::StopInternal(IDeviceContext* pContext) +{ + if (!SetStopping()) + return; + + m_Stopping.store(true, std::memory_order_release); + if (m_pTextureStreams) { m_pTextureStreams->SetStopping(); @@ -1305,15 +1314,18 @@ void GPUUploadManagerImpl::Stop(IDeviceContext* pContext) // the destructor so the application controls the thread/phase where the manager's device // context is touched. Keep streams and pages alive until destruction, because pending // operations still own callback payloads that must be released during teardown. - for (UploadStreamUniquePtr& Stream : m_Streams) + if (pContext != nullptr) { - Stream->ReleaseStagingBuffers(pContext); + for (UploadStreamUniquePtr& Stream : m_Streams) + { + Stream->ReleaseStagingBuffers(pContext); + } } } GPUUploadManagerImpl::~GPUUploadManagerImpl() { - Stop(m_pContext); + StopInternal(m_pContext); // Pending page pointers are owned by the streams below. The manager is terminally // destroyed, so discard the queue nodes before destroying the pages. From 789205b0f13375634d389098f9f2ade6caf3d6b3 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 21:40:48 -0700 Subject: [PATCH 06/36] GPUUploadManager: validate context-owning calls --- .../include/GPUUploadManagerImpl.hpp | 2 + .../interface/GPUUploadManager.h | 29 ++++++++---- .../src/GPUUploadManagerImpl.cpp | 44 ++++++++++--------- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp index 29e148a109..e01a315e70 100644 --- a/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp +++ b/Graphics/GraphicsTools/include/GPUUploadManagerImpl.hpp @@ -448,6 +448,8 @@ class GPUUploadManagerImpl final : public ObjectBase bool TryBeginScheduleUpdate() noexcept; void EndScheduleUpdate() noexcept; bool SetStopping() noexcept; + // Only call from context-owning paths: RenderThreadUpdate(), Stop(), or Schedule*Update() with non-null pContext. + bool SetOrValidateContext(IDeviceContext* pContext, const char* MethodName); void StopInternal(IDeviceContext* pContext); static constexpr Uint32 SCHEDULE_STOP_BIT = 0x80000000u; diff --git a/Graphics/GraphicsTools/interface/GPUUploadManager.h b/Graphics/GraphicsTools/interface/GPUUploadManager.h index 2d4e99dc39..2ceb5b3c07 100644 --- a/Graphics/GraphicsTools/interface/GPUUploadManager.h +++ b/Graphics/GraphicsTools/interface/GPUUploadManager.h @@ -160,6 +160,8 @@ typedef void (*CopyStagingBufferCallbackType)(IDeviceContext* pContext, struct ScheduleBufferUpdateInfo { /// If calling ScheduleBufferUpdate() from the render thread, a pointer to the device context. + /// If no manager context has been set yet, this context becomes the manager context. + /// Otherwise, it must be the same as the manager context. /// If calling ScheduleBufferUpdate() from a worker thread, this parameter must be null. IDeviceContext* pContext DEFAULT_INITIALIZER(nullptr); @@ -403,6 +405,8 @@ typedef void (*CopyStagingD3D11TextureCallbackType)(IDeviceContext* pContext, struct ScheduleTextureUpdateInfo { /// If calling ScheduleTextureUpdate() from the render thread, a pointer to the device context. + /// If no manager context has been set yet, this context becomes the manager context. + /// Otherwise, it must be the same as the manager context. /// If calling ScheduleTextureUpdate() from a worker thread, this parameter must be null. IDeviceContext* pContext DEFAULT_INITIALIZER(nullptr); @@ -574,6 +578,9 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// from worker threads, but only one thread is allowed to call RenderThreadUpdate() at a time. /// RenderThreadUpdate() must not be called concurrently with Stop(), and the device context /// used by the manager must not be used concurrently by other threads. + /// pContext must not be null. If no context was provided at creation, the first context + /// passed to RenderThreadUpdate() becomes the manager context. Otherwise, it must be + /// the same as the manager context. /// /// The method must be called periodically to process pending updates. If the method is not called, /// ScheduleBufferUpdate() or ScheduleTextureUpdate() may block indefinitely when there are no free @@ -589,8 +596,10 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// due to invalid parameters, a stopped manager, or an internal scheduling failure. /// If false is returned, any abandoned-scheduling callback is invoked before the method returns. /// - /// The method is thread-safe and can be called from multiple threads simultaneously with other calls to ScheduleBufferUpdate() - /// and RenderThreadUpdate(). + /// The method is thread-safe for worker-thread calls that use a null pContext. These calls can be made from multiple threads + /// simultaneously with other worker-thread ScheduleBufferUpdate() calls and RenderThreadUpdate(). + /// Calls that provide a non-null pContext use the device context and must be externally serialized with RenderThreadUpdate(), + /// Stop(), GetStats(), and other use of the same context. /// The caller must keep the upload manager alive for the entire duration of this call. Worker threads should /// hold their own strong reference to the manager if the manager may be stopped or released concurrently. /// Calls admitted before Stop() may complete; calls admitted after Stop() are ignored and return false. @@ -606,8 +615,9 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// call RenderThreadUpdate() to process pending buffer updates. If RenderThreadUpdate() is not called, the method may block indefinitely /// when there are no free pages available for new updates. /// - /// If the method is called from the render thread, the pContext parameter must be a pointer to the device context used to create the - /// GPU upload manager. If the method is called from the render thread with null pContext, it may never return. + /// If the method is called from the render thread, the pContext parameter must be a pointer + /// to the manager context. If no manager context has been set yet, pContext becomes the manager + /// context. If the method is called from the render thread with null pContext, it may never return. VIRTUAL bool METHOD(ScheduleBufferUpdate)(THIS_ const ScheduleBufferUpdateInfo REF UpdateInfo) PURE; @@ -620,8 +630,10 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// due to invalid parameters, a stopped manager, or an internal scheduling failure. /// If false is returned, any abandoned-scheduling callback is invoked before the method returns. /// - /// The method is thread-safe and can be called from multiple threads simultaneously with other calls to ScheduleTextureUpdate() - /// and RenderThreadUpdate(). + /// The method is thread-safe for worker-thread calls that use a null pContext. These calls can be made from multiple threads + /// simultaneously with other worker-thread ScheduleTextureUpdate() calls and RenderThreadUpdate(). + /// Calls that provide a non-null pContext use the device context and must be externally serialized with RenderThreadUpdate(), + /// Stop(), GetStats(), and other use of the same context. /// The caller must keep the upload manager alive for the entire duration of this call. Worker threads should /// hold their own strong reference to the manager if the manager may be stopped or released concurrently. /// Calls admitted before Stop() may complete; calls admitted after Stop() are ignored and return false. @@ -637,8 +649,9 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// call RenderThreadUpdate() to process pending texture updates. If RenderThreadUpdate() is not called, the method may block indefinitely /// when there are no free pages available for new updates. /// - /// If the method is called from the render thread, the pContext parameter must be a pointer to the device context used to create the - /// GPU upload manager. If the method is called from the render thread with null pContext, it may never return. + /// If the method is called from the render thread, the pContext parameter must be a pointer + /// to the manager context. If no manager context has been set yet, pContext becomes the manager + /// context. If the method is called from the render thread with null pContext, it may never return. VIRTUAL bool METHOD(ScheduleTextureUpdate)(THIS_ const ScheduleTextureUpdateInfo REF UpdateInfo) PURE; diff --git a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp index ff87462521..eb13d1790b 100644 --- a/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp +++ b/Graphics/GraphicsTools/src/GPUUploadManagerImpl.cpp @@ -1264,24 +1264,34 @@ GPUUploadManagerImpl::ScheduleUpdateGuard::~ScheduleUpdateGuard() m_pMgr->EndScheduleUpdate(); } -void GPUUploadManagerImpl::Stop(IDeviceContext* pContext) +bool GPUUploadManagerImpl::SetOrValidateContext(IDeviceContext* pContext, const char* MethodName) { if (pContext == nullptr) { - LOG_ERROR_MESSAGE("A valid context must be provided to Stop()"); - return; + LOG_ERROR_MESSAGE("A valid context must be provided to ", MethodName, "()"); + return false; } if (!m_pContext) { m_pContext = pContext; + return true; } - else if (pContext != m_pContext) + + if (pContext != m_pContext) { - LOG_ERROR_MESSAGE("The context provided to Stop() must be the same as the one already used by the GPUUploadManagerImpl"); - return; + LOG_ERROR_MESSAGE("The context provided to ", MethodName, "() must be the same as the one already used by the GPUUploadManagerImpl"); + return false; } + return true; +} + +void GPUUploadManagerImpl::Stop(IDeviceContext* pContext) +{ + if (!SetOrValidateContext(pContext, "Stop")) + return; + StopInternal(pContext); } @@ -1336,22 +1346,14 @@ GPUUploadManagerImpl::~GPUUploadManagerImpl() void GPUUploadManagerImpl::RenderThreadUpdate(IDeviceContext* pContext) { - DEV_CHECK_ERR(pContext != nullptr, "A valid context must be provided to RenderThreadUpdate"); if (m_Stopping.load(std::memory_order_acquire)) { DEV_ERROR("GPU upload manager has been stopped"); return; } - if (!m_pContext) - { - // If no context was provided at creation, we can accept any context in RenderThreadUpdate, but it must be the same across calls. - m_pContext = pContext; - } - else - { - DEV_CHECK_ERR(pContext == m_pContext, "The context provided to RenderThreadUpdate must be the same as the one used to create the GPUUploadManagerImpl"); - } + if (!SetOrValidateContext(pContext, "RenderThreadUpdate")) + return; if (m_pTextureStreams) { @@ -1402,10 +1404,6 @@ bool GPUUploadManagerImpl::UploadStream::ScheduleUpdate(IDeviceContext* pContext const void* pUpdateInfo, bool ScheduleUpdate(Page::Writer& Writer, const void* pUpdateInfo)) { - DEV_CHECK_ERR(pContext == nullptr || pContext == m_Mgr.m_pContext, - "If a context is provided to ScheduleBufferUpdate/ScheduleTextureUpdate, it must be the same as the " - "one used to create the GPUUploadManagerImpl"); - bool IsFirstAttempt = true; bool AbortUpdate = false; bool UpdateScheduled = false; @@ -1498,6 +1496,9 @@ bool GPUUploadManagerImpl::ScheduleBufferUpdate(const ScheduleBufferUpdateInfo& return false; } + if (UpdateInfo.pContext != nullptr && !SetOrValidateContext(UpdateInfo.pContext, "ScheduleBufferUpdate")) + return false; + if (!ValidateBufferUpdate(UpdateInfo)) return false; @@ -1535,6 +1536,9 @@ bool GPUUploadManagerImpl::ScheduleTextureUpdate(const ScheduleTextureUpdateInfo return false; } + if (UpdateInfo.pContext != nullptr && !SetOrValidateContext(UpdateInfo.pContext, "ScheduleTextureUpdate")) + return false; + const bool HasCopyCallback = UseD3D11TextureCallback ? UpdateInfo.CopyD3D11Texture != nullptr : From e3dd13be89c26638b02d0595fe3047025893b042 Mon Sep 17 00:00:00 2001 From: assiduous Date: Tue, 23 Jun 2026 22:08:57 -0700 Subject: [PATCH 07/36] GPUUploadManager: clarify Stop threading contract --- Graphics/GraphicsTools/interface/GPUUploadManager.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Graphics/GraphicsTools/interface/GPUUploadManager.h b/Graphics/GraphicsTools/interface/GPUUploadManager.h index 2ceb5b3c07..254b8a86e3 100644 --- a/Graphics/GraphicsTools/interface/GPUUploadManager.h +++ b/Graphics/GraphicsTools/interface/GPUUploadManager.h @@ -686,15 +686,16 @@ DILIGENT_BEGIN_INTERFACE(IGPUUploadManager, IObject) /// concurrently while Stop() is executing. Internal stream/page objects remain alive until the /// manager is destroyed. /// - /// The intended use is to call Stop() once, normally from the render thread. - /// Multiple and parallel Stop() calls are allowed, but only the first call performs - /// the stop, wait, and staging-resource release. Any subsequent Stop() call from - /// any thread is a no-op and returns immediately, potentially before the first - /// Stop() call has completed. + /// Stop() is a render-thread/context-owning call. The first Stop() call must be made + /// from the render thread and must not race with RenderThreadUpdate(), GetStats(), + /// another Stop() call, or ScheduleBufferUpdate()/ScheduleTextureUpdate() calls that + /// provide a non-null device context. After Stop() has completed, subsequent Stop() + /// calls are no-ops and return immediately. /// /// The method may be called while worker threads are inside ScheduleBufferUpdate() or /// ScheduleTextureUpdate(). The manager must remain alive until these calls have returned. - /// Stop() must not be called concurrently with RenderThreadUpdate() or GetStats(). + /// The first Stop() call must not be made while other render-thread/context-owning + /// upload-manager calls are running. /// /// If Stop() is not called explicitly, the manager destructor performs the same internal stop /// sequence using the stored manager context from whichever thread releases the last reference. From 8d60343894d2e6f23e5076ad7231de93b9384031 Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 24 Jun 2026 18:27:07 -0700 Subject: [PATCH 08/36] Add DILIGENT_ENABLE_ASAN build option and Linux CI coverage --- .github/workflows/build-linux.yml | 9 ++++++++- CMakeLists.txt | 25 ++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index eb1650bde6..783bc58895 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -61,6 +61,13 @@ jobs: cc: "clang-18" cxx: "clang++-18" + - name: "Clang-ASAN" + build_type: "RelWithDebInfo" + cmake_generator: "Ninja" + cmake_args: "-DDILIGENT_BUILD_CORE_TESTS=ON -DDILIGENT_ENABLE_ASAN=ON" + cc: "clang-18" + cxx: "clang++-18" + - name: "Clang-NO_GLSLANG" build_type: "Debug" cmake_generator: "Ninja" @@ -150,7 +157,7 @@ jobs: - name: Upload artifact uses: actions/upload-artifact@v7 - if: ${{ success() && matrix.build_type != 'Debug' }} + if: ${{ success() && matrix.build_type != 'Debug' && matrix.name != 'Clang-ASAN' }} with: name: DiligentCore-Linux-${{ matrix.name }}-x64-${{ matrix.build_type }} path: | diff --git a/CMakeLists.txt b/CMakeLists.txt index 947c391e67..c3087e9a2d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -316,6 +316,7 @@ endif() option(DILIGENT_NO_ARCHIVER "Do not build archiver" OFF) option(DILIGENT_NO_SUPER_RESOLUTION "Do not build super resolution" OFF) +option(DILIGENT_ENABLE_ASAN "Enable AddressSanitizer" OFF) option(DILIGENT_EMSCRIPTEN_STRIP_DEBUG_INFO "Strip debug information from WebAsm binaries" OFF) @@ -392,6 +393,21 @@ endif() target_link_libraries(Diligent-BuildSettings INTERFACE Diligent-PublicBuildSettings) +if(DILIGENT_ENABLE_ASAN) + message(STATUS "DILIGENT_ENABLE_ASAN is ON: AddressSanitizer is enabled") + if(MSVC) + target_compile_options(Diligent-BuildSettings INTERFACE /fsanitize=address) + elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") + target_compile_options(Diligent-BuildSettings INTERFACE + -fsanitize=address + -fno-omit-frame-pointer + ) + target_link_libraries(Diligent-BuildSettings INTERFACE -fsanitize=address) + else() + message(FATAL_ERROR "DILIGENT_ENABLE_ASAN is not supported by ${CMAKE_CXX_COMPILER_ID}") + endif() +endif() + foreach(DBG_CONFIG ${DEBUG_CONFIGURATIONS}) target_compile_definitions(Diligent-BuildSettings INTERFACE "$<$:_DEBUG;DEBUG>") endforeach() @@ -413,12 +429,15 @@ if(MSVC) endif() # Enable whole program optimization - set(DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS /GL) + set(DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS "") + if(NOT DILIGENT_ENABLE_ASAN) + list(APPEND DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS /GL) + endif() if("${TARGET_CPU}" STREQUAL "x86_64") # Enable AVX2 - set(DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS ${DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS} /arch:AVX2) + list(APPEND DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS /arch:AVX2) endif() - set(DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS ${DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS} CACHE STRING "Additional MSVC compile options for release configurations") + set(DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS "${DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS}" CACHE STRING "Additional MSVC compile options for release configurations") if (DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS) message("Additional MSVC compile options for release configurations: " ${DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS}) endif() From dfe7fbff7681e2c23286faa4229e1d8649e76c50 Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 24 Jun 2026 18:45:26 -0700 Subject: [PATCH 09/36] CI: enable GPU tests with ASAN on Linux --- .github/workflows/build-linux.yml | 8 ++++---- Tests/DiligentCoreAPITest/src/DrawCommandTest.cpp | 2 +- .../DiligentCoreAPITest/src/ShaderVariableAccessTest.cpp | 2 ++ .../src/Linux/TestingEnvironmentLinux.cpp | 3 ++- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 783bc58895..3a19ffcea9 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -130,26 +130,26 @@ jobs: - name: DiligentCoreAPITest VK # NB: it is essential to include failure() to override the default status check of success() # that is automatically applied to if conditions that don't contain a status check function. - if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC') }} + if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 with: mode: vk_sw - name: DiligentCoreAPITest VK Compatibility - if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC') }} + if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 with: mode: vk_sw vk-compatibility: true - name: DiligentCoreAPITest GL - if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC') }} + if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 with: mode: gl - name: DiligentCoreAPITest GL with Non-Separable Programs - if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC') }} + if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 with: mode: gl diff --git a/Tests/DiligentCoreAPITest/src/DrawCommandTest.cpp b/Tests/DiligentCoreAPITest/src/DrawCommandTest.cpp index 2008455797..5d609b3e24 100644 --- a/Tests/DiligentCoreAPITest/src/DrawCommandTest.cpp +++ b/Tests/DiligentCoreAPITest/src/DrawCommandTest.cpp @@ -2943,7 +2943,7 @@ TEST_F(DrawCommandTest, DynamicUniformBufferUpdatesWithSignatures) } const Uint32 Indices[] = {0, 1, 2, 3, 4, 5}; - RefCntAutoPtr pIndexBuffer = CreateIndexBuffer(Indices, sizeof(Indices)); + RefCntAutoPtr pIndexBuffer = CreateIndexBuffer(Indices, _countof(Indices)); TestDynamicBufferUpdatesWithSignaturesAttribs Attribs{ pVS, diff --git a/Tests/DiligentCoreAPITest/src/ShaderVariableAccessTest.cpp b/Tests/DiligentCoreAPITest/src/ShaderVariableAccessTest.cpp index ab62da79a9..bf4e39619d 100644 --- a/Tests/DiligentCoreAPITest/src/ShaderVariableAccessTest.cpp +++ b/Tests/DiligentCoreAPITest/src/ShaderVariableAccessTest.cpp @@ -157,12 +157,14 @@ TEST(ShaderResourceLayout, VariableAccess) constexpr auto RTVFormat = TEX_FORMAT_RGBA8_UNORM; constexpr auto DSVFormat = TEX_FORMAT_D32_FLOAT; + TexDesc.Name = "Render target"; TexDesc.Format = RTVFormat; TexDesc.BindFlags = BIND_RENDER_TARGET | BIND_SHADER_RESOURCE; RefCntAutoPtr pRenderTarget; pDevice->CreateTexture(TexDesc, nullptr, &pRenderTarget); auto* pRTV = pRenderTarget->GetDefaultView(TEXTURE_VIEW_RENDER_TARGET); + TexDesc.Name = "Depth texture"; TexDesc.Format = DSVFormat; TexDesc.BindFlags = BIND_DEPTH_STENCIL; RefCntAutoPtr pDepthTex; diff --git a/Tests/GPUTestFramework/src/Linux/TestingEnvironmentLinux.cpp b/Tests/GPUTestFramework/src/Linux/TestingEnvironmentLinux.cpp index 7c9d4089bb..676fbd652f 100644 --- a/Tests/GPUTestFramework/src/Linux/TestingEnvironmentLinux.cpp +++ b/Tests/GPUTestFramework/src/Linux/TestingEnvironmentLinux.cpp @@ -136,11 +136,12 @@ NativeWindow GPUTestingEnvironment::CreateNativeWindow() // clang-format on GLXContext ctx = glXCreateContextAttribsARB(display, fbc[0], NULL, 1, context_attribs); + XFree(vi); + XFree(fbc); if (!ctx) { LOG_ERROR_AND_THROW("Failed to create GL context."); } - XFree(fbc); glXMakeCurrent(display, win, ctx); From b0bc1fd74c89875fbac8fe901d202770c6d1de04 Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 24 Jun 2026 20:31:01 -0700 Subject: [PATCH 10/36] Update CI actions to v18 and use ASAN input --- .github/workflows/build-android.yml | 2 +- .github/workflows/build-apple.yml | 8 ++++---- .github/workflows/build-emscripten.yml | 6 +++--- .github/workflows/build-linux.yml | 20 +++++++++++--------- .github/workflows/build-windows.yml | 16 ++++++++-------- .github/workflows/codeql.yml | 6 +++--- .github/workflows/msvc_analysis.yml | 6 +++--- 7 files changed, 33 insertions(+), 31 deletions(-) diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index aa22a226ec..7a68e0a158 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -25,7 +25,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: Android diff --git a/.github/workflows/build-apple.yml b/.github/workflows/build-apple.yml index 05783ced9c..c7c16e25cf 100644 --- a/.github/workflows/build-apple.yml +++ b/.github/workflows/build-apple.yml @@ -57,7 +57,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: ${{ matrix.platform }} @@ -71,20 +71,20 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} - name: Build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 with: target: install - name: DiligentCoreTest if: ${{ success() && matrix.platform == 'MacOS' }} - uses: DiligentGraphics/github-action/run-core-tests@v14 + uses: DiligentGraphics/github-action/run-core-tests@v18 - name: Upload artifact uses: actions/upload-artifact@v7 diff --git a/.github/workflows/build-emscripten.yml b/.github/workflows/build-emscripten.yml index ab2b36f91e..f35b1fa34f 100644 --- a/.github/workflows/build-emscripten.yml +++ b/.github/workflows/build-emscripten.yml @@ -38,17 +38,17 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: Web - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} - name: Build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 3a19ffcea9..636c0be238 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -64,7 +64,8 @@ jobs: - name: "Clang-ASAN" build_type: "RelWithDebInfo" cmake_generator: "Ninja" - cmake_args: "-DDILIGENT_BUILD_CORE_TESTS=ON -DDILIGENT_ENABLE_ASAN=ON" + cmake_args: "-DDILIGENT_BUILD_CORE_TESTS=ON" + enable_asan: "true" cc: "clang-18" cxx: "clang++-18" @@ -92,7 +93,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: Linux cmake-generator: ${{ matrix.cmake_generator }} @@ -107,50 +108,51 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: cc: ${{ matrix.cc }} cxx: ${{ matrix.cxx }} generator: ${{ matrix.cmake_generator }} build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} + enable-asan: ${{ matrix.enable_asan }} - name: Build id: build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 with: target: install - name: DiligentCoreTest if: success() - uses: DiligentGraphics/github-action/run-core-tests@v14 + uses: DiligentGraphics/github-action/run-core-tests@v18 - name: DiligentCoreAPITest VK # NB: it is essential to include failure() to override the default status check of success() # that is automatically applied to if conditions that don't contain a status check function. if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: vk_sw - name: DiligentCoreAPITest VK Compatibility if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: vk_sw vk-compatibility: true - name: DiligentCoreAPITest GL if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: gl - name: DiligentCoreAPITest GL with Non-Separable Programs if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: gl non-separable-progs: true diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 1064eb4f5a..953d3c3321 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -77,7 +77,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} @@ -93,7 +93,7 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: generator: ${{ matrix.cmake_generator }} vs-arch: ${{ matrix.toolset }} @@ -103,31 +103,31 @@ jobs: - name: Build id: build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 with: target: install - name: DiligentCoreTest if: ${{ success() && matrix.name != 'UWP'}} - uses: DiligentGraphics/github-action/run-core-tests@v14 + uses: DiligentGraphics/github-action/run-core-tests@v18 - name: DiligentCoreAPITest D3D11 # NB: it is essential to include failure() to override the default status check of success() # that is automatically applied to if conditions that don't contain a status check function. if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: d3d11_sw - name: DiligentCoreAPITest D3D12 if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: d3d12_sw - name: DiligentCoreAPITest D3D12 DXC if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: d3d12_sw use-dxc: true @@ -136,7 +136,7 @@ jobs: - name: DiligentCoreAPITest WebGPU if: ${{ (success() || failure() && steps.build.outcome == 'success') && matrix.name == 'Win10' }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v14 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 with: mode: wgpu diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 8be322ff4e..df33615bab 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -41,14 +41,14 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: cc: ${{ matrix.cc }} cxx: ${{ matrix.cxx }} @@ -65,7 +65,7 @@ jobs: - name: Build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v4 diff --git a/.github/workflows/msvc_analysis.yml b/.github/workflows/msvc_analysis.yml index 0681f0b520..6127d9008e 100644 --- a/.github/workflows/msvc_analysis.yml +++ b/.github/workflows/msvc_analysis.yml @@ -32,14 +32,14 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v14 + uses: DiligentGraphics/github-action/setup-build-env@v18 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v14 + uses: DiligentGraphics/github-action/configure-cmake@v18 with: generator: ${{ matrix.cmake_generator }} vs-arch: ${{ matrix.toolset }} @@ -48,7 +48,7 @@ jobs: - name: Build if: success() - uses: DiligentGraphics/github-action/build@v14 + uses: DiligentGraphics/github-action/build@v18 - name: Run MSVC Code Analysis uses: DiligentGraphics/msvc-code-analysis-action@main From aac6dd6e6b4fae7ff7386fb6a22f4250832c3cfe Mon Sep 17 00:00:00 2001 From: assiduous Date: Wed, 24 Jun 2026 23:35:31 -0700 Subject: [PATCH 11/36] Propagate ASAN flags to Abseil third-party build --- CMakeLists.txt | 20 ++++++++++++++------ ThirdParty/abseil-cpp/CMakeLists.txt | 4 ++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c3087e9a2d..4c9549fdf6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -393,19 +393,27 @@ endif() target_link_libraries(Diligent-BuildSettings INTERFACE Diligent-PublicBuildSettings) +set(DILIGENT_ASAN_COMPILE_OPTIONS "" CACHE INTERNAL "Diligent AddressSanitizer compile options") +set(DILIGENT_ASAN_LINK_OPTIONS "" CACHE INTERNAL "Diligent AddressSanitizer link options") + if(DILIGENT_ENABLE_ASAN) message(STATUS "DILIGENT_ENABLE_ASAN is ON: AddressSanitizer is enabled") if(MSVC) - target_compile_options(Diligent-BuildSettings INTERFACE /fsanitize=address) + set(DILIGENT_ASAN_COMPILE_OPTIONS /fsanitize=address CACHE INTERNAL "Diligent AddressSanitizer compile options") elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") - target_compile_options(Diligent-BuildSettings INTERFACE - -fsanitize=address - -fno-omit-frame-pointer - ) - target_link_libraries(Diligent-BuildSettings INTERFACE -fsanitize=address) + set(DILIGENT_ASAN_COMPILE_OPTIONS -fsanitize=address -fno-omit-frame-pointer CACHE INTERNAL "Diligent AddressSanitizer compile options") + set(DILIGENT_ASAN_LINK_OPTIONS -fsanitize=address CACHE INTERNAL "Diligent AddressSanitizer link options") else() message(FATAL_ERROR "DILIGENT_ENABLE_ASAN is not supported by ${CMAKE_CXX_COMPILER_ID}") endif() + + message(STATUS " DILIGENT_ASAN_COMPILE_OPTIONS: ${DILIGENT_ASAN_COMPILE_OPTIONS}") + message(STATUS " DILIGENT_ASAN_LINK_OPTIONS: ${DILIGENT_ASAN_LINK_OPTIONS}") + + target_compile_options(Diligent-BuildSettings INTERFACE ${DILIGENT_ASAN_COMPILE_OPTIONS}) + if(DILIGENT_ASAN_LINK_OPTIONS) + target_link_libraries(Diligent-BuildSettings INTERFACE ${DILIGENT_ASAN_LINK_OPTIONS}) + endif() endif() foreach(DBG_CONFIG ${DEBUG_CONFIGURATIONS}) diff --git a/ThirdParty/abseil-cpp/CMakeLists.txt b/ThirdParty/abseil-cpp/CMakeLists.txt index 4cabaf99f1..ef21f9f45d 100644 --- a/ThirdParty/abseil-cpp/CMakeLists.txt +++ b/ThirdParty/abseil-cpp/CMakeLists.txt @@ -12,5 +12,9 @@ set(CMAKE_CXX_EXTENSIONS OFF) set(ABSL_PROPAGATE_CXX_STD ON CACHE BOOL "" FORCE) +if(DILIGENT_ENABLE_ASAN) + add_compile_options(${DILIGENT_ASAN_COMPILE_OPTIONS}) +endif() + FetchContent_MakeAvailable(abseil-cpp) install(FILES "${abseil-cpp_SOURCE_DIR}/LICENSE" DESTINATION "Licenses/ThirdParty/${DILIGENT_CORE_DIR}" RENAME abseil-cpp-License.txt) From 40c57b8ac3b539c90a0c364e038061f8cb74b4ac Mon Sep 17 00:00:00 2001 From: assiduous Date: Thu, 25 Jun 2026 09:38:45 -0700 Subject: [PATCH 12/36] Add generic sanitizer option and update CI actions --- .github/workflows/build-android.yml | 2 +- .github/workflows/build-apple.yml | 8 ++-- .github/workflows/build-emscripten.yml | 6 +-- .github/workflows/build-linux.yml | 20 +++++----- .github/workflows/build-windows.yml | 16 ++++---- .github/workflows/codeql.yml | 6 +-- .github/workflows/msvc_analysis.yml | 6 +-- CMakeLists.txt | 54 ++++++++++++++++++-------- ThirdParty/abseil-cpp/CMakeLists.txt | 4 +- 9 files changed, 71 insertions(+), 51 deletions(-) diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index 7a68e0a158..168953fffe 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -25,7 +25,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: Android diff --git a/.github/workflows/build-apple.yml b/.github/workflows/build-apple.yml index c7c16e25cf..d729d945dc 100644 --- a/.github/workflows/build-apple.yml +++ b/.github/workflows/build-apple.yml @@ -57,7 +57,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: ${{ matrix.platform }} @@ -71,20 +71,20 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} - name: Build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 with: target: install - name: DiligentCoreTest if: ${{ success() && matrix.platform == 'MacOS' }} - uses: DiligentGraphics/github-action/run-core-tests@v18 + uses: DiligentGraphics/github-action/run-core-tests@v19 - name: Upload artifact uses: actions/upload-artifact@v7 diff --git a/.github/workflows/build-emscripten.yml b/.github/workflows/build-emscripten.yml index f35b1fa34f..19bfb38a22 100644 --- a/.github/workflows/build-emscripten.yml +++ b/.github/workflows/build-emscripten.yml @@ -38,17 +38,17 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: Web - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} - name: Build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 636c0be238..ca317aff4b 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -65,7 +65,7 @@ jobs: build_type: "RelWithDebInfo" cmake_generator: "Ninja" cmake_args: "-DDILIGENT_BUILD_CORE_TESTS=ON" - enable_asan: "true" + sanitizer: "address" cc: "clang-18" cxx: "clang++-18" @@ -93,7 +93,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: Linux cmake-generator: ${{ matrix.cmake_generator }} @@ -108,51 +108,51 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: cc: ${{ matrix.cc }} cxx: ${{ matrix.cxx }} generator: ${{ matrix.cmake_generator }} build-type: ${{ matrix.build_type }} cmake-args: ${{ matrix.cmake_args }} - enable-asan: ${{ matrix.enable_asan }} + sanitizer: ${{ matrix.sanitizer }} - name: Build id: build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 with: target: install - name: DiligentCoreTest if: success() - uses: DiligentGraphics/github-action/run-core-tests@v18 + uses: DiligentGraphics/github-action/run-core-tests@v19 - name: DiligentCoreAPITest VK # NB: it is essential to include failure() to override the default status check of success() # that is automatically applied to if conditions that don't contain a status check function. if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: vk_sw - name: DiligentCoreAPITest VK Compatibility if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: vk_sw vk-compatibility: true - name: DiligentCoreAPITest GL if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: gl - name: DiligentCoreAPITest GL with Non-Separable Programs if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Clang' || matrix.name == 'GCC' || matrix.name == 'Clang-ASAN') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: gl non-separable-progs: true diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 953d3c3321..d03e98a3af 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -77,7 +77,7 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} @@ -93,7 +93,7 @@ jobs: - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: generator: ${{ matrix.cmake_generator }} vs-arch: ${{ matrix.toolset }} @@ -103,31 +103,31 @@ jobs: - name: Build id: build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 with: target: install - name: DiligentCoreTest if: ${{ success() && matrix.name != 'UWP'}} - uses: DiligentGraphics/github-action/run-core-tests@v18 + uses: DiligentGraphics/github-action/run-core-tests@v19 - name: DiligentCoreAPITest D3D11 # NB: it is essential to include failure() to override the default status check of success() # that is automatically applied to if conditions that don't contain a status check function. if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: d3d11_sw - name: DiligentCoreAPITest D3D12 if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: d3d12_sw - name: DiligentCoreAPITest D3D12 DXC if: ${{ (success() || failure() && steps.build.outcome == 'success') && (matrix.name == 'Win10' || matrix.name == 'Win10-Ninja') }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: d3d12_sw use-dxc: true @@ -136,7 +136,7 @@ jobs: - name: DiligentCoreAPITest WebGPU if: ${{ (success() || failure() && steps.build.outcome == 'success') && matrix.name == 'Win10' }} - uses: DiligentGraphics/github-action/run-core-gpu-tests@v18 + uses: DiligentGraphics/github-action/run-core-gpu-tests@v19 with: mode: wgpu diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index df33615bab..bd5de169fb 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -41,14 +41,14 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: cc: ${{ matrix.cc }} cxx: ${{ matrix.cxx }} @@ -65,7 +65,7 @@ jobs: - name: Build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v4 diff --git a/.github/workflows/msvc_analysis.yml b/.github/workflows/msvc_analysis.yml index 6127d9008e..989ecd4a06 100644 --- a/.github/workflows/msvc_analysis.yml +++ b/.github/workflows/msvc_analysis.yml @@ -32,14 +32,14 @@ jobs: - name: Set up build environment if: success() - uses: DiligentGraphics/github-action/setup-build-env@v18 + uses: DiligentGraphics/github-action/setup-build-env@v19 with: platform: ${{ matrix.platform }} cmake-generator: ${{ matrix.cmake_generator }} - name: Configure CMake if: success() - uses: DiligentGraphics/github-action/configure-cmake@v18 + uses: DiligentGraphics/github-action/configure-cmake@v19 with: generator: ${{ matrix.cmake_generator }} vs-arch: ${{ matrix.toolset }} @@ -48,7 +48,7 @@ jobs: - name: Build if: success() - uses: DiligentGraphics/github-action/build@v18 + uses: DiligentGraphics/github-action/build@v19 - name: Run MSVC Code Analysis uses: DiligentGraphics/msvc-code-analysis-action@main diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c9549fdf6..c5c74dc3b8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -316,7 +316,8 @@ endif() option(DILIGENT_NO_ARCHIVER "Do not build archiver" OFF) option(DILIGENT_NO_SUPER_RESOLUTION "Do not build super resolution" OFF) -option(DILIGENT_ENABLE_ASAN "Enable AddressSanitizer" OFF) +set(DILIGENT_SANITIZER "" CACHE STRING "Enable sanitizer: address or thread") +set_property(CACHE DILIGENT_SANITIZER PROPERTY STRINGS "" address thread) option(DILIGENT_EMSCRIPTEN_STRIP_DEBUG_INFO "Strip debug information from WebAsm binaries" OFF) @@ -393,26 +394,45 @@ endif() target_link_libraries(Diligent-BuildSettings INTERFACE Diligent-PublicBuildSettings) -set(DILIGENT_ASAN_COMPILE_OPTIONS "" CACHE INTERNAL "Diligent AddressSanitizer compile options") -set(DILIGENT_ASAN_LINK_OPTIONS "" CACHE INTERNAL "Diligent AddressSanitizer link options") +string(TOLOWER "${DILIGENT_SANITIZER}" DILIGENT_SANITIZER_NORMALIZED) +if(NOT "${DILIGENT_SANITIZER}" STREQUAL "${DILIGENT_SANITIZER_NORMALIZED}") + set(DILIGENT_SANITIZER "${DILIGENT_SANITIZER_NORMALIZED}" CACHE STRING "Enable sanitizer: address or thread" FORCE) +endif() + +set(DILIGENT_SANITIZER_COMPILE_OPTIONS "" CACHE INTERNAL "Diligent sanitizer compile options") +set(DILIGENT_SANITIZER_LINK_OPTIONS "" CACHE INTERNAL "Diligent sanitizer link options") -if(DILIGENT_ENABLE_ASAN) - message(STATUS "DILIGENT_ENABLE_ASAN is ON: AddressSanitizer is enabled") - if(MSVC) - set(DILIGENT_ASAN_COMPILE_OPTIONS /fsanitize=address CACHE INTERNAL "Diligent AddressSanitizer compile options") - elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") - set(DILIGENT_ASAN_COMPILE_OPTIONS -fsanitize=address -fno-omit-frame-pointer CACHE INTERNAL "Diligent AddressSanitizer compile options") - set(DILIGENT_ASAN_LINK_OPTIONS -fsanitize=address CACHE INTERNAL "Diligent AddressSanitizer link options") +if(DILIGENT_SANITIZER) + if(DILIGENT_SANITIZER STREQUAL "address") + message(STATUS "DILIGENT_SANITIZER is 'address': AddressSanitizer is enabled") + if(MSVC) + set(DILIGENT_SANITIZER_COMPILE_OPTIONS /fsanitize=address CACHE INTERNAL "Diligent sanitizer compile options") + elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") + set(DILIGENT_SANITIZER_COMPILE_OPTIONS -fsanitize=address -fno-omit-frame-pointer CACHE INTERNAL "Diligent sanitizer compile options") + set(DILIGENT_SANITIZER_LINK_OPTIONS -fsanitize=address CACHE INTERNAL "Diligent sanitizer link options") + else() + message(FATAL_ERROR "DILIGENT_SANITIZER=address is not supported by ${CMAKE_CXX_COMPILER_ID}") + endif() + elseif(DILIGENT_SANITIZER STREQUAL "thread") + message(STATUS "DILIGENT_SANITIZER is 'thread': ThreadSanitizer is enabled") + if(MSVC) + message(FATAL_ERROR "DILIGENT_SANITIZER=thread is not supported by MSVC") + elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") + set(DILIGENT_SANITIZER_COMPILE_OPTIONS -fsanitize=thread -fno-omit-frame-pointer CACHE INTERNAL "Diligent sanitizer compile options") + set(DILIGENT_SANITIZER_LINK_OPTIONS -fsanitize=thread CACHE INTERNAL "Diligent sanitizer link options") + else() + message(FATAL_ERROR "DILIGENT_SANITIZER=thread is not supported by ${CMAKE_CXX_COMPILER_ID}") + endif() else() - message(FATAL_ERROR "DILIGENT_ENABLE_ASAN is not supported by ${CMAKE_CXX_COMPILER_ID}") + message(FATAL_ERROR "Unsupported DILIGENT_SANITIZER value: '${DILIGENT_SANITIZER}'. Supported values: address, thread.") endif() - message(STATUS " DILIGENT_ASAN_COMPILE_OPTIONS: ${DILIGENT_ASAN_COMPILE_OPTIONS}") - message(STATUS " DILIGENT_ASAN_LINK_OPTIONS: ${DILIGENT_ASAN_LINK_OPTIONS}") + message(STATUS " DILIGENT_SANITIZER_COMPILE_OPTIONS: ${DILIGENT_SANITIZER_COMPILE_OPTIONS}") + message(STATUS " DILIGENT_SANITIZER_LINK_OPTIONS: ${DILIGENT_SANITIZER_LINK_OPTIONS}") - target_compile_options(Diligent-BuildSettings INTERFACE ${DILIGENT_ASAN_COMPILE_OPTIONS}) - if(DILIGENT_ASAN_LINK_OPTIONS) - target_link_libraries(Diligent-BuildSettings INTERFACE ${DILIGENT_ASAN_LINK_OPTIONS}) + target_compile_options(Diligent-BuildSettings INTERFACE ${DILIGENT_SANITIZER_COMPILE_OPTIONS}) + if(DILIGENT_SANITIZER_LINK_OPTIONS) + target_link_libraries(Diligent-BuildSettings INTERFACE ${DILIGENT_SANITIZER_LINK_OPTIONS}) endif() endif() @@ -438,7 +458,7 @@ if(MSVC) # Enable whole program optimization set(DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS "") - if(NOT DILIGENT_ENABLE_ASAN) + if(NOT DILIGENT_SANITIZER) list(APPEND DEFAULT_DILIGENT_MSVC_RELEASE_COMPILE_OPTIONS /GL) endif() if("${TARGET_CPU}" STREQUAL "x86_64") diff --git a/ThirdParty/abseil-cpp/CMakeLists.txt b/ThirdParty/abseil-cpp/CMakeLists.txt index ef21f9f45d..ec10f14967 100644 --- a/ThirdParty/abseil-cpp/CMakeLists.txt +++ b/ThirdParty/abseil-cpp/CMakeLists.txt @@ -12,8 +12,8 @@ set(CMAKE_CXX_EXTENSIONS OFF) set(ABSL_PROPAGATE_CXX_STD ON CACHE BOOL "" FORCE) -if(DILIGENT_ENABLE_ASAN) - add_compile_options(${DILIGENT_ASAN_COMPILE_OPTIONS}) +if(DILIGENT_SANITIZER_COMPILE_OPTIONS) + add_compile_options(${DILIGENT_SANITIZER_COMPILE_OPTIONS}) endif() FetchContent_MakeAvailable(abseil-cpp) From 8070b9a26f0ddc4af5483293c8636e1ef703151e Mon Sep 17 00:00:00 2001 From: assiduous Date: Thu, 25 Jun 2026 20:02:50 -0700 Subject: [PATCH 13/36] Fix TSAN races in object registry and once logging --- Common/interface/ObjectsRegistry.hpp | 4 ++- Primitives/interface/Errors.hpp | 37 ++++++++++++++-------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/Common/interface/ObjectsRegistry.hpp b/Common/interface/ObjectsRegistry.hpp index de1ad453c9..555430df75 100644 --- a/Common/interface/ObjectsRegistry.hpp +++ b/Common/interface/ObjectsRegistry.hpp @@ -1,4 +1,4 @@ -/* Copyright 2023-2025 Diligent Graphics LLC +/* Copyright 2023-2026 Diligent Graphics LLC * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, @@ -294,11 +294,13 @@ class ObjectsRegistry StrongPtrType Lock() { + std::lock_guard Guard{m_CreateObjectMtx}; return _LockWeakPtr(m_wpObject); } bool IsExpired() { + std::lock_guard Guard{m_CreateObjectMtx}; return _IsWeakPtrExpired(m_wpObject); } diff --git a/Primitives/interface/Errors.hpp b/Primitives/interface/Errors.hpp index fec2c8870a..07d5f461e2 100644 --- a/Primitives/interface/Errors.hpp +++ b/Primitives/interface/Errors.hpp @@ -1,5 +1,5 @@ /* - * Copyright 2019-2025 Diligent Graphics LLC + * Copyright 2019-2026 Diligent Graphics LLC * Copyright 2015-2019 Egor Yusov * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -31,6 +31,7 @@ #include #include #include +#include #include "DebugOutput.h" #include "FormatString.hpp" @@ -87,15 +88,14 @@ void LogError(bool IsFatal, const char* Function, const char* FullFilePath, int Diligent::LogError(/*IsFatal=*/true, __FUNCTION__, __FILE__, __LINE__, ##__VA_ARGS__); \ } while (false) -#define LOG_ERROR_ONCE(...) \ - do \ - { \ - static bool IsFirstTime = true; \ - if (IsFirstTime) \ - { \ - LOG_ERROR(##__VA_ARGS__); \ - IsFirstTime = false; \ - } \ +#define LOG_ERROR_ONCE(...) \ + do \ + { \ + static std::atomic_bool IsFirstTime{true}; \ + if (IsFirstTime.exchange(false, std::memory_order_relaxed)) \ + { \ + LOG_ERROR(##__VA_ARGS__); \ + } \ } while (false) @@ -148,15 +148,14 @@ void LogError(bool IsFatal, const char* Function, const char* FullFilePath, int # define LOG_DVP_INFO_MESSAGE(...) #endif -#define LOG_DEBUG_MESSAGE_ONCE(Severity, ...) \ - do \ - { \ - static bool IsFirstTime = true; \ - if (IsFirstTime) \ - { \ - LOG_DEBUG_MESSAGE(Severity, ##__VA_ARGS__); \ - IsFirstTime = false; \ - } \ +#define LOG_DEBUG_MESSAGE_ONCE(Severity, ...) \ + do \ + { \ + static std::atomic_bool IsFirstTime{true}; \ + if (IsFirstTime.exchange(false, std::memory_order_relaxed)) \ + { \ + LOG_DEBUG_MESSAGE(Severity, ##__VA_ARGS__); \ + } \ } while (false) #define LOG_FATAL_ERROR_MESSAGE_ONCE(...) LOG_DEBUG_MESSAGE_ONCE(Diligent::DEBUG_MESSAGE_SEVERITY_FATAL_ERROR, ##__VA_ARGS__) From 3d62de3baa84a9480adb7509ea1fcd5d54405526 Mon Sep 17 00:00:00 2001 From: assiduous Date: Thu, 25 Jun 2026 21:38:57 -0700 Subject: [PATCH 14/36] ObjectsRegistry: fix create-once race during purge --- Common/interface/ObjectsRegistry.hpp | 85 ++++++++++---- .../src/Common/ObjectsRegistryTest.cpp | 110 ++++++++++++++++++ 2 files changed, 171 insertions(+), 24 deletions(-) diff --git a/Common/interface/ObjectsRegistry.hpp b/Common/interface/ObjectsRegistry.hpp index 555430df75..bb29591532 100644 --- a/Common/interface/ObjectsRegistry.hpp +++ b/Common/interface/ObjectsRegistry.hpp @@ -22,6 +22,7 @@ #include #include #include +#include #include "../../Platforms/Basic/interface/DebugUtilities.hpp" #include "RefCntAutoPtr.hpp" @@ -29,48 +30,53 @@ namespace Diligent { -template -struct _StrongPtrHelper; +namespace Details +{ + +template +struct StrongPtrHelper; -// _StrongPtrHelper specialization for RefCntAutoPtr +// StrongPtrHelper specialization for RefCntAutoPtr template -struct _StrongPtrHelper> +struct StrongPtrHelper> { using WeakPtrType = RefCntWeakPtr; }; -// _StrongPtrHelper specialization for std::shared_ptr +// StrongPtrHelper specialization for std::shared_ptr template -struct _StrongPtrHelper> +struct StrongPtrHelper> { using WeakPtrType = std::weak_ptr; }; template -auto _LockWeakPtr(RefCntWeakPtr& pWeakPtr) +auto LockWeakPtr(RefCntWeakPtr& pWeakPtr) { return pWeakPtr.Lock(); } template -auto _LockWeakPtr(std::weak_ptr& pWeakPtr) +auto LockWeakPtr(std::weak_ptr& pWeakPtr) { return pWeakPtr.lock(); } template -auto _IsWeakPtrExpired(RefCntWeakPtr& pWeakPtr) +auto IsWeakPtrExpired(RefCntWeakPtr& pWeakPtr) { return !pWeakPtr.IsValid(); } template -auto _IsWeakPtrExpired(std::weak_ptr& pWeakPtr) +auto IsWeakPtrExpired(std::weak_ptr& pWeakPtr) { return pWeakPtr.expired(); } +} // namespace Details + /// A thread-safe and exception-safe object registry that works with std::shared_ptr or RefCntAutoPtr. /// The registry keeps weak pointers to the objects and returns strong pointers if the requested object exits. /// An application should keep strong pointers to the objects to keep them alive. @@ -114,12 +120,22 @@ template ::WeakPtrType; + using WeakPtrType = typename Details::StrongPtrHelper::WeakPtrType; explicit ObjectsRegistry(Uint32 NumRequestsToPurge = 1024) noexcept : m_NumRequestsToPurge{NumRequestsToPurge} {} +#ifdef DILIGENT_OBJECTS_REGISTRY_TEST_HOOKS + using BeforeGetObjectCallbackType = void (*)(void* pUserData); + + void SetBeforeGetObjectCallback(BeforeGetObjectCallbackType Callback, void* pUserData = nullptr) + { + m_BeforeGetObjectCallback = Callback; + m_pBeforeGetObjectCallbackCtx = pUserData; + } +#endif + /// Finds the object in the registry and returns strong pointer to it (std::shared_ptr or RefCntAutoPtr). /// If the object is not found, it is atomically created using the provided initializer. /// @@ -132,9 +148,6 @@ class ObjectsRegistry /// CreateObject function may throw in case of an error. /// /// It is guaranteed, that the Object will only be initialized once, even if multiple threads call Get() simultaneously. - /// However, if another thread runs an overloaded Get() without the initializer function with the same key, it may - /// remove the entry from the registry, and the object will be initialized multiple times. - /// This is OK as only one object will be added to the registry. template StrongPtrType Get(const KeyType& Key, CreateObjectType&& CreateObject // May throw @@ -157,6 +170,10 @@ class ObjectsRegistry StrongPtrType pObject; try { +#ifdef DILIGENT_OBJECTS_REGISTRY_TEST_HOOKS + if (m_BeforeGetObjectCallback != nullptr) + m_BeforeGetObjectCallback(m_pBeforeGetObjectCallbackCtx); +#endif pObject = pObjectWrpr->Get(std::forward(CreateObject)); } catch (...) @@ -172,7 +189,7 @@ class ObjectsRegistry // The object was created by another thread while we were waiting for the lock return pObject; } - else + else if (!IsObjectWrapperInUse(it->second, pObjectWrpr.get())) { m_Cache.erase(it); } @@ -200,7 +217,7 @@ class ObjectsRegistry { pObject = it->second->Lock(); // Note that the object may have been created by another thread while we were waiting for the lock - if (!pObject) + if (!pObject && !IsObjectWrapperInUse(it->second, pObjectWrpr.get())) m_Cache.erase(it); } } @@ -230,10 +247,10 @@ class ObjectsRegistry if (it != m_Cache.end()) { auto pObject = it->second->Lock(); - if (!pObject) + if (!pObject && !IsObjectWrapperInUse(it->second)) { - // Note that we may remove the entry from the cache while another thread is creating the object. - // This is OK as it will be added back to the cache. + // An empty wrapper may still be used by Get(Key, CreateObject) after it + // copies the wrapper from m_Cache and before it enters ObjectWrapper::Get(). m_Cache.erase(it); } @@ -277,12 +294,12 @@ class ObjectsRegistry { public: template - const StrongPtrType Get(CreateObjectType&& CreateObject) noexcept(false) + StrongPtrType Get(CreateObjectType&& CreateObject) noexcept(false) { StrongPtrType pObject; std::lock_guard Guard{m_CreateObjectMtx}; - pObject = _LockWeakPtr(m_wpObject); + pObject = Details::LockWeakPtr(m_wpObject); if (!pObject) { pObject = CreateObject(); // May throw @@ -295,13 +312,13 @@ class ObjectsRegistry StrongPtrType Lock() { std::lock_guard Guard{m_CreateObjectMtx}; - return _LockWeakPtr(m_wpObject); + return Details::LockWeakPtr(m_wpObject); } bool IsExpired() { std::lock_guard Guard{m_CreateObjectMtx}; - return _IsWeakPtrExpired(m_wpObject); + return Details::IsWeakPtrExpired(m_wpObject); } private: @@ -313,7 +330,9 @@ class ObjectsRegistry { for (auto it = m_Cache.begin(); it != m_Cache.end();) { - if (it->second->IsExpired()) + // Skip empty wrappers that are still referenced by Get(Key, CreateObject): + // removing them would allow another wrapper to be inserted for the same key. + if (!IsObjectWrapperInUse(it->second) && it->second->IsExpired()) { it = m_Cache.erase(it); } @@ -326,6 +345,19 @@ class ObjectsRegistry m_NumRequestsSinceLastPurge.store(0); } + static bool IsObjectWrapperInUse(const std::shared_ptr& pObjectWrpr, + const ObjectWrapper* pCurrentObjectWrpr = nullptr) + { + // m_CacheMtx must be held: ObjectWrapper references are copied from m_Cache under this mutex, + // so use_count() cannot grow while we make the erase decision. + // With no current Get(Key, CreateObject) call, m_Cache should be the only owner. + // When that call checks its own wrapper, it also holds pObjectWrpr locally, so the + // expected use count is 2. Any larger count means another thread may be using or + // initializing the wrapper, and erasing it could break the create-once guarantee. + const auto ExpectedUseCount = pObjectWrpr.get() == pCurrentObjectWrpr ? 2 : 1; + return pObjectWrpr.use_count() > ExpectedUseCount; + } + private: using CacheType = std::unordered_map, KeyHasher, KeyEqual>; @@ -335,6 +367,11 @@ class ObjectsRegistry std::mutex m_CacheMtx; CacheType m_Cache; + +#ifdef DILIGENT_OBJECTS_REGISTRY_TEST_HOOKS + BeforeGetObjectCallbackType m_BeforeGetObjectCallback = nullptr; + void* m_pBeforeGetObjectCallbackCtx = nullptr; +#endif }; } // namespace Diligent diff --git a/Tests/DiligentCoreTest/src/Common/ObjectsRegistryTest.cpp b/Tests/DiligentCoreTest/src/Common/ObjectsRegistryTest.cpp index 52f74c790e..723f3d376c 100644 --- a/Tests/DiligentCoreTest/src/Common/ObjectsRegistryTest.cpp +++ b/Tests/DiligentCoreTest/src/Common/ObjectsRegistryTest.cpp @@ -24,12 +24,17 @@ * of the possibility of such damages. */ +#define DILIGENT_OBJECTS_REGISTRY_TEST_HOOKS 1 #include "ObjectsRegistry.hpp" +#undef DILIGENT_OBJECTS_REGISTRY_TEST_HOOKS #include "gtest/gtest.h" +#include #include #include +#include +#include #include "ObjectBase.hpp" #include "ThreadSignal.hpp" @@ -168,6 +173,111 @@ TEST(Common_ObjectsRegistry, CreateDestroyRace_RefCntAutoPtr) TestObjectRegistryCreateDestroyRace(); } +enum class ObjectWrapperRemovalType +{ + Purge, + EmptyGet, +}; + +struct BeforeGetObjectHook +{ + Threading::Signal FirstCallStarted; + Threading::Signal ReleaseFirstCall; + std::atomic CallCount{0}; +}; + +template