Directly acquire m_aMutex, instead of looping on m_condition.wait()

The code had been introduced with 9c9970952b0adec4a8c6de9a4cd54d0980cd47ec
"tdf#96887 enhance SolarMutex AcquireWithWait for Windows", which mentions
MsgWaitForMultipleObjects in multiple comments as if it had intended to make use
of it somewhere, but no such use can be found in that commit.

(I had come across this code when on Windows some CppunitTest had
deadlocked for me during DeInitVCL when re-acquiring the released SolarMutex at
the end of

> #if defined _WIN32
>     // See GetSystemClipboard (vcl/source/treelist/transfer2.cxx):
>     if (auto const comp = css::uno::Reference<css::lang::XComponent>(
>             pSVData->m_xSystemClipboard, css::uno::UNO_QUERY))
>     {
>         SolarMutexReleaser r; // unblock pending "clipboard content changed" notifications
>         comp->dispose(); // will use CWinClipbImpl::s_aMutex
>     }
> #endif

at vcl/source/app/svmain.cxx:490--498, and SalYieldMutex::doAcquire was waiting
in m_condition.wait() for an m_condition.set() from some other thread that would
never come---there were not even any other relevant threads left.  At first I
thought this might be an issue with broken-by-design osl::Condition, where some
other, meanwhile gone thread's m_condition.set() had been cancelled by this
thread's m_condition.reset() in SalYieldMutex::doAcquire, but then its
m_aMutex.tryToAcquire() should probably have succeeded so that it wouldn't have
ended up in m_codition.wait().  But even if the use of m_condition might not be
broken after all and the deadlock I observed would be caused by something else,
it nevertheless looks pointless, so better clean it up.)

Change-Id: Ia6c1aa133c14e19224d0f24c810f43363cb5898c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103253
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

1 files changed, 4 insertions, 30 deletions

diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 5ebb22226eff..3af78712b1d2 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -99,9 +99,6 @@ 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;
@@ -138,30 +135,11 @@ void SalYieldMutex::BeforeReleaseHandler()
 void SalYieldMutex::doAcquire( sal_uInt32 nLockCount )
 {
     WinSalInstance* pInst = GetSalData()->mpInstance;
-    if ( pInst && pInst->IsMainThread() )
+    if ( pInst && pInst->IsMainThread() && pInst->m_nNoYieldLock )
     {
-        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. 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 );
+        return;
     }
-    else
-        m_aMutex.acquire();
+    m_aMutex.acquire();
     ++m_nCount;
     --nLockCount;
 
@@ -174,11 +152,7 @@ sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll )
     if ( pInst && pInst->m_nNoYieldLock && pInst->IsMainThread() )
         return 1;
 
-    sal_uInt32 nCount = comphelper::SolarMutex::doRelease( bUnlockAll );
-    // wake up ImplSalYieldMutexAcquireWithWait() after release
-    if ( 0 == m_nCount )
-        m_condition.set();
-    return nCount;
+    return comphelper::SolarMutex::doRelease( bUnlockAll );
 }
 
 bool SalYieldMutex::tryToAcquire()