vcl/win/app/salinst.cxx | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-)
New commits: commit 9771e61666455c969de751e4d8f27c1c160780e1 Author: Stephan Bergmann <sberg...@redhat.com> AuthorDate: Thu Sep 24 13:27:17 2020 +0200 Commit: Stephan Bergmann <sberg...@redhat.com> CommitDate: Thu Sep 24 18:03:40 2020 +0200 Revert "Directly acquire m_aMutex, instead of looping on m_condition.wait()" This reverts commit e9a16702ba025ca340bcded4fda37235d22410a1. My understanding that the code was pointless was apparently wrong (the relevant part being hidden in m_condition.wait() internally doing the call to MsgWaitForMultipleObjects; I've improved the relevant comment now). (And my fears that the code using osl::Condition might be broken by design were hopefully unfounded.) Change-Id: Icb244ec9df8a2b0dacf115700ed850d471446f3a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103310 Reviewed-by: Michael Stahl <michael.st...@cib.de> Tested-by: Jenkins diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index 3af78712b1d2..476b6350147a 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -99,6 +99,9 @@ static LRESULT CALLBACK SalComWndProcW( HWND hWnd, UINT nMsg, WPARAM wParam, LPA class SalYieldMutex : public comphelper::SolarMutex { +public: // for ImplSalYield() and ImplSalYieldMutexAcquireWithWait() + osl::Condition m_condition; /// for MsgWaitForMultipleObjects() + protected: virtual void doAcquire( sal_uInt32 nLockCount ) override; virtual sal_uInt32 doRelease( bool bUnlockAll ) override; @@ -135,11 +138,31 @@ void SalYieldMutex::BeforeReleaseHandler() void SalYieldMutex::doAcquire( sal_uInt32 nLockCount ) { WinSalInstance* pInst = GetSalData()->mpInstance; - if ( pInst && pInst->IsMainThread() && pInst->m_nNoYieldLock ) + if ( pInst && pInst->IsMainThread() ) { - return; + if ( pInst->m_nNoYieldLock ) + return; + // tdf#96887 If this is the main thread, then we must wait for two things: + // - the yield mutex being unlocked + // - SendMessage() being triggered + // This can nicely be done using MsgWaitForMultipleObjects, which is called in + // m_condition.wait(). The 2nd one is + // needed because if we don't reschedule, then we create deadlocks if a + // Window's create/destroy is called via SendMessage() from another thread. + // Have a look at the osl_waitCondition implementation for more info. + do { + // reset condition *before* acquiring! + m_condition.reset(); + if (m_aMutex.tryToAcquire()) + break; + // wait for SalYieldMutex::release() to set the condition + osl::Condition::Result res = m_condition.wait(); + assert(osl::Condition::Result::result_ok == res); + } + while ( true ); } - m_aMutex.acquire(); + else + m_aMutex.acquire(); ++m_nCount; --nLockCount; @@ -152,7 +175,11 @@ sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll ) if ( pInst && pInst->m_nNoYieldLock && pInst->IsMainThread() ) return 1; - return comphelper::SolarMutex::doRelease( bUnlockAll ); + sal_uInt32 nCount = comphelper::SolarMutex::doRelease( bUnlockAll ); + // wake up ImplSalYieldMutexAcquireWithWait() after release + if ( 0 == m_nCount ) + m_condition.set(); + return nCount; } bool SalYieldMutex::tryToAcquire() _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits