From bb01b111325c8dc26055f213073f044a8889a130 Mon Sep 17 00:00:00 2001 From: benstevens48 Date: Tue, 21 Jan 2020 20:07:36 +0000 Subject: [PATCH 1/4] fix device context pool deadlock scenario --- winrt/lib/drawing/DeviceContextPool.cpp | 36 +++++++++++++------------ 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/winrt/lib/drawing/DeviceContextPool.cpp b/winrt/lib/drawing/DeviceContextPool.cpp index 2a331cd2c..81753ff32 100644 --- a/winrt/lib/drawing/DeviceContextPool.cpp +++ b/winrt/lib/drawing/DeviceContextPool.cpp @@ -20,25 +20,27 @@ DeviceContextPool::DeviceContextPool(ID2D1Device1* d2dDevice) DeviceContextLease DeviceContextPool::TakeLease() { - Lock lock(m_mutex); - - if (!m_d2dDevice) - ThrowHR(RO_E_CLOSED); - - if (m_deviceContexts.empty()) + ID2D1Device1* localDevice; { - ComPtr deviceContext; - ThrowIfFailed(m_d2dDevice->CreateDeviceContext( - D2D1_DEVICE_CONTEXT_OPTIONS_NONE, - &deviceContext)); - return DeviceContextLease(this, std::move(deviceContext)); - } - else - { - DeviceContextLease newLease(this, std::move(m_deviceContexts.back())); - m_deviceContexts.pop_back(); - return newLease; + Lock lock(m_mutex); + + if (!m_deviceContexts.empty()) + { + DeviceContextLease newLease(this, std::move(m_deviceContexts.back())); + m_deviceContexts.pop_back(); + return newLease; + } + localDevice = m_d2dDevice; } + //Release lock before creating the device context to avoid potential deadlock scenarios + if (!localDevice) + ThrowHR(RO_E_CLOSED); + + ComPtr deviceContext; + ThrowIfFailed(localDevice->CreateDeviceContext( + D2D1_DEVICE_CONTEXT_OPTIONS_NONE, + &deviceContext)); + return DeviceContextLease(this, std::move(deviceContext)); } From 410aa6b76160e0b9779288edaf4258dc6f0bea94 Mon Sep 17 00:00:00 2001 From: benstevens48 Date: Tue, 21 Jan 2020 21:22:02 +0000 Subject: [PATCH 2/4] Revert "fix device context pool deadlock scenario" This reverts commit bb01b111325c8dc26055f213073f044a8889a130. --- winrt/lib/drawing/DeviceContextPool.cpp | 36 ++++++++++++------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/winrt/lib/drawing/DeviceContextPool.cpp b/winrt/lib/drawing/DeviceContextPool.cpp index 81753ff32..2a331cd2c 100644 --- a/winrt/lib/drawing/DeviceContextPool.cpp +++ b/winrt/lib/drawing/DeviceContextPool.cpp @@ -20,27 +20,25 @@ DeviceContextPool::DeviceContextPool(ID2D1Device1* d2dDevice) DeviceContextLease DeviceContextPool::TakeLease() { - ID2D1Device1* localDevice; + Lock lock(m_mutex); + + if (!m_d2dDevice) + ThrowHR(RO_E_CLOSED); + + if (m_deviceContexts.empty()) { - Lock lock(m_mutex); - - if (!m_deviceContexts.empty()) - { - DeviceContextLease newLease(this, std::move(m_deviceContexts.back())); - m_deviceContexts.pop_back(); - return newLease; - } - localDevice = m_d2dDevice; + ComPtr deviceContext; + ThrowIfFailed(m_d2dDevice->CreateDeviceContext( + D2D1_DEVICE_CONTEXT_OPTIONS_NONE, + &deviceContext)); + return DeviceContextLease(this, std::move(deviceContext)); + } + else + { + DeviceContextLease newLease(this, std::move(m_deviceContexts.back())); + m_deviceContexts.pop_back(); + return newLease; } - //Release lock before creating the device context to avoid potential deadlock scenarios - if (!localDevice) - ThrowHR(RO_E_CLOSED); - - ComPtr deviceContext; - ThrowIfFailed(localDevice->CreateDeviceContext( - D2D1_DEVICE_CONTEXT_OPTIONS_NONE, - &deviceContext)); - return DeviceContextLease(this, std::move(deviceContext)); } From 6ef7c76f08c5687d9dcc8eaaaa4dee0a807aa9ec Mon Sep 17 00:00:00 2001 From: benstevens48 Date: Tue, 21 Jan 2020 21:42:36 +0000 Subject: [PATCH 3/4] fix device context pool potential deadlock --- winrt/lib/drawing/DeviceContextPool.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/winrt/lib/drawing/DeviceContextPool.cpp b/winrt/lib/drawing/DeviceContextPool.cpp index 2a331cd2c..298a91165 100644 --- a/winrt/lib/drawing/DeviceContextPool.cpp +++ b/winrt/lib/drawing/DeviceContextPool.cpp @@ -27,8 +27,12 @@ DeviceContextLease DeviceContextPool::TakeLease() if (m_deviceContexts.empty()) { + //Store a local copy of the non-null device pointer before unlocking. + auto localDevice = m_d2dDevice; + //Release lock before creating the device context to avoid potential deadlock scenarios. + lock.unlock(); ComPtr deviceContext; - ThrowIfFailed(m_d2dDevice->CreateDeviceContext( + ThrowIfFailed(localDevice->CreateDeviceContext( D2D1_DEVICE_CONTEXT_OPTIONS_NONE, &deviceContext)); return DeviceContextLease(this, std::move(deviceContext)); From 5472da9f3da1fe77374fa454334d7a9d955a9fa3 Mon Sep 17 00:00:00 2001 From: benstevens48 Date: Fri, 22 Nov 2024 13:05:50 +0000 Subject: [PATCH 4/4] add ref even though not strictly needed --- winrt/lib/drawing/DeviceContextPool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winrt/lib/drawing/DeviceContextPool.cpp b/winrt/lib/drawing/DeviceContextPool.cpp index 298a91165..c41432724 100644 --- a/winrt/lib/drawing/DeviceContextPool.cpp +++ b/winrt/lib/drawing/DeviceContextPool.cpp @@ -28,7 +28,7 @@ DeviceContextLease DeviceContextPool::TakeLease() if (m_deviceContexts.empty()) { //Store a local copy of the non-null device pointer before unlocking. - auto localDevice = m_d2dDevice; + ComPtr localDevice(m_d2dDevice); //Release lock before creating the device context to avoid potential deadlock scenarios. lock.unlock(); ComPtr deviceContext;