diff --git a/idd/LGIdd/CIndirectMonitorContext.cpp b/idd/LGIdd/CIndirectMonitorContext.cpp index 4566aa4b..28637614 100644 --- a/idd/LGIdd/CIndirectMonitorContext.cpp +++ b/idd/LGIdd/CIndirectMonitorContext.cpp @@ -37,11 +37,19 @@ CIndirectMonitorContext::~CIndirectMonitorContext() void CIndirectMonitorContext::AssignSwapChain(IDDCX_SWAPCHAIN swapChain, LUID renderAdapter, HANDLE newFrameEvent) { - UnassignSwapChain(); + std::lock_guard assignGuard(m_assignMutex); - // Build everything into locals so the members are never observed - // half-constructed by a concurrent unassign, and so the processor (which - // spawns a worker thread) is created outside the lock. + // Finish tearing down the previous assignment before reserving a generation + // for the new one. Deleting the old processor can itself cause IddCx to + // re-enter UnassignSwapChain, and that old callback must happen before the + // new generation is established. + DetachSwapChain(); + + const UINT64 assignmentGeneration = + m_assignmentGeneration.fetch_add(1, std::memory_order_acq_rel) + 1; + + // Build the devices into locals so the members are never observed + // half-constructed and the expensive initialization stays outside m_lock. std::shared_ptr dx11Device; std::shared_ptr dx12Device; @@ -78,19 +86,33 @@ void CIndirectMonitorContext::AssignSwapChain(IDDCX_SWAPCHAIN swapChain, LUID re break; } - m_devContext->OnSwapChainAssigned(); - std::unique_ptr processor(new CSwapChainProcessor( - m_monitor, m_devContext, swapChain, dx11Device, dx12Device, newFrameEvent)); - AcquireSRWLockExclusive(&m_lock); + if (!IsAssignmentCurrent(assignmentGeneration)) + { + ReleaseSRWLockExclusive(&m_lock); + DEBUG_INFO("Swap chain assignment canceled before processor startup"); + return; + } + + // Publish the assignment atomically with starting its worker. An unassign + // now blocks on m_lock until m_swapChain exists, at which point it can + // signal and join the processor normally. + m_devContext->OnSwapChainAssigned(); m_dx11Device = std::move(dx11Device); m_dx12Device = std::move(dx12Device); - m_swapChain = std::move(processor); + m_swapChain.reset(new CSwapChainProcessor( + this, assignmentGeneration, m_monitor, m_devContext, swapChain, + m_dx11Device, m_dx12Device, newFrameEvent)); ReleaseSRWLockExclusive(&m_lock); } -void CIndirectMonitorContext::UnassignSwapChain() +void CIndirectMonitorContext::DetachSwapChain() { + // Invalidate setup in progress before waiting for m_lock. This also lets a + // worker about to call SetDevice observe an unassign whose callback is + // blocked waiting for the processor to be published. + m_assignmentGeneration.fetch_add(1, std::memory_order_acq_rel); + // Detach under the lock, then destroy outside it. Destroying the processor // joins its worker thread, whose teardown (WdfObjectDelete) re-enters this // method on another thread - holding the lock across that would deadlock. @@ -112,3 +134,8 @@ void CIndirectMonitorContext::UnassignSwapChain() if (hadSwapChain) m_devContext->OnSwapChainReleased(); } + +void CIndirectMonitorContext::UnassignSwapChain() +{ + DetachSwapChain(); +} diff --git a/idd/LGIdd/CIndirectMonitorContext.h b/idd/LGIdd/CIndirectMonitorContext.h index 9ac35a8d..fd42165c 100644 --- a/idd/LGIdd/CIndirectMonitorContext.h +++ b/idd/LGIdd/CIndirectMonitorContext.h @@ -24,7 +24,9 @@ #include #include +#include #include +#include #include "CIndirectDeviceContext.h" #include "CSwapChainProcessor.h" @@ -39,12 +41,24 @@ private: // concurrently (an unassign triggered by the worker's WdfObjectDelete can // race the next assign), and shared_ptr copy/reset is not thread safe. SRWLOCK m_lock = SRWLOCK_INIT; + + // IddCx can issue a replacement assignment before an earlier assignment + // has finished creating its devices. Serialize those expensive setup paths + // while still allowing UnassignSwapChain to cancel the active one. + std::mutex m_assignMutex; std::shared_ptr m_dx11Device; std::shared_ptr m_dx12Device; CIndirectDeviceContext * m_devContext; std::unique_ptr m_swapChain; + // Incremented whenever the current assignment is replaced or unassigned. + // Device creation is performed outside m_lock, so this lets an unassign + // cancel that work before a processor is started on a stale swap chain. + std::atomic m_assignmentGeneration = 0; + + void DetachSwapChain(); + public: CIndirectMonitorContext(_In_ IDDCX_MONITOR monitor, CIndirectDeviceContext * device); @@ -52,6 +66,10 @@ public: void AssignSwapChain(IDDCX_SWAPCHAIN swapChain, LUID renderAdapter, HANDLE newFrameEvent); void UnassignSwapChain(); + bool IsAssignmentCurrent(UINT64 generation) const + { + return m_assignmentGeneration.load(std::memory_order_acquire) == generation; + } CIndirectDeviceContext * GetDeviceContext() { return m_devContext; } }; @@ -67,4 +85,4 @@ struct CIndirectMonitorContextWrapper } }; -WDF_DECLARE_CONTEXT_TYPE(CIndirectMonitorContextWrapper); \ No newline at end of file +WDF_DECLARE_CONTEXT_TYPE(CIndirectMonitorContextWrapper); diff --git a/idd/LGIdd/CSwapChainProcessor.cpp b/idd/LGIdd/CSwapChainProcessor.cpp index 36a1f3c1..f9cd2c3b 100644 --- a/idd/LGIdd/CSwapChainProcessor.cpp +++ b/idd/LGIdd/CSwapChainProcessor.cpp @@ -19,13 +19,18 @@ */ #include "CSwapChainProcessor.h" +#include "CIndirectMonitorContext.h" #include #include "CDebug.h" #include "CPipeServer.h" -CSwapChainProcessor::CSwapChainProcessor(IDDCX_MONITOR monitor, CIndirectDeviceContext* devContext, IDDCX_SWAPCHAIN hSwapChain, +CSwapChainProcessor::CSwapChainProcessor(CIndirectMonitorContext * monitorContext, + UINT64 assignmentGeneration, IDDCX_MONITOR monitor, + CIndirectDeviceContext* devContext, IDDCX_SWAPCHAIN hSwapChain, std::shared_ptr dx11Device, std::shared_ptr dx12Device, HANDLE newFrameEvent) : + m_monitorContext(monitorContext), + m_assignmentGeneration(assignmentGeneration), m_monitor(monitor), m_devContext(devContext), m_hSwapChain(hSwapChain), @@ -41,10 +46,11 @@ CSwapChainProcessor::CSwapChainProcessor(IDDCX_MONITOR monitor, CIndirectDeviceC // Manual-reset: both worker threads wait on this, so it must stay signalled // once set or only one thread would ever observe termination. m_terminateEvent.Attach(CreateEvent(nullptr, TRUE, FALSE, nullptr)); - m_thread[0].Attach(CreateThread(nullptr, 0, _SwapChainThread, this, 0, nullptr)); - m_cursorDataEvent.Attach(CreateEvent(nullptr, FALSE, FALSE, nullptr)); m_shapeBuffer = new BYTE[512 * 512 * 4]; + + // Start the worker only after every object it can access is initialized. + m_thread[0].Attach(CreateThread(nullptr, 0, _SwapChainThread, this, 0, nullptr)); } CSwapChainProcessor::~CSwapChainProcessor() @@ -118,13 +124,24 @@ bool CSwapChainProcessor::SwapChainThreadCore() IDARG_IN_SWAPCHAINSETDEVICE setDevice = {}; setDevice.pDevice = dxgiDevice.Get(); + // IddCx can unassign a swap chain while its devices are still being + // created. In that case the owner signals termination and IddCx retains + // responsibility for the handle because SetDevice has not succeeded. + if (!m_monitorContext->IsAssignmentCurrent(m_assignmentGeneration) || + WaitForSingleObject(m_terminateEvent.Get(), 0) == WAIT_OBJECT_0) + return false; + // A failure here (commonly DXGI_ERROR_ACCESS_LOST on the first assignment) // is not recoverable on this handle - IddCx reassigns a fresh swap chain, // which is what actually succeeds. Bail cleanly and let that happen. hr = IddCxSwapChainSetDevice(m_hSwapChain, &setDevice); if (FAILED(hr)) { - DEBUG_ERROR_HR(hr, "IddCxSwapChainSetDevice Failed"); + if (!m_monitorContext->IsAssignmentCurrent(m_assignmentGeneration) || + WaitForSingleObject(m_terminateEvent.Get(), 0) == WAIT_OBJECT_0) + DEBUG_INFO("Swap chain was unassigned during device setup"); + else + DEBUG_ERROR_HR(hr, "IddCxSwapChainSetDevice Failed"); return false; } // Past this point SetDevice succeeded: we own the swap chain and are diff --git a/idd/LGIdd/CSwapChainProcessor.h b/idd/LGIdd/CSwapChainProcessor.h index 4420d52b..16343296 100644 --- a/idd/LGIdd/CSwapChainProcessor.h +++ b/idd/LGIdd/CSwapChainProcessor.h @@ -36,9 +36,13 @@ using namespace Microsoft::WRL; #define STAGING_TEXTURES 3 +class CIndirectMonitorContext; + class CSwapChainProcessor { private: + CIndirectMonitorContext * m_monitorContext; + UINT64 m_assignmentGeneration; IDDCX_MONITOR m_monitor; CIndirectDeviceContext * m_devContext; IDDCX_SWAPCHAIN m_hSwapChain; @@ -74,7 +78,8 @@ private: DXGI_COLOR_SPACE_TYPE colorSpace, UINT sdrWhiteLevel); public: - CSwapChainProcessor(IDDCX_MONITOR monitor, CIndirectDeviceContext * devContext, IDDCX_SWAPCHAIN hSwapChain, + CSwapChainProcessor(CIndirectMonitorContext * monitorContext, UINT64 assignmentGeneration, + IDDCX_MONITOR monitor, CIndirectDeviceContext * devContext, IDDCX_SWAPCHAIN hSwapChain, std::shared_ptr dx11Device, std::shared_ptr dx12Device, HANDLE newFrameEvent); ~CSwapChainProcessor();