vcl: fix Win32 deadlocks from SolarMutexReleaser

To create and destroy thread-affine Win32 Windows and DCs, non-main
threads SendMessage() special messages like SAL_MSG_CREATEFRAME.

The main thread must handle these messages and return the result to
un-block the other thread.

This works fine as long as the main thread is in its message loop anyway
and blocked on GetMessage(); however if the main blocks trying to acquire
the SolarMutex that is held by the sending thread, deadlock results.

In order to work around this, there is some peculiar code in
ImplSalYieldMutexAcquireWithWait() to avoid blocking the main thread on
mpSalYieldMutex but instead block in GetMessage().

The crucial detail is that GetMessage() will immediately dispatch any
message sent via SendMessage(), which avoids the deadlock.

https://msdn.microsoft.com/en-us/library/windows/desktop/ms644936.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms644927.aspx

Most of the Win32 WndProc that acquire SolarMutex do so via
ImplSalYieldMutexAcquireWithWait(), but the main thread may also
temporarily drop SolarMutex and re-aquire it with the questionable
SolarMutexReleaser hack, which calls ImplSalAcquireYieldMutex()
instead, which blocks on mpSalYieldMutex.

Fix SolarMutexReleaser to call a new function
Application::ReAcquireSolarMutex() that does the right thing here:
acquire SolarMutex via ImplSalYieldMutexAcquireWithWait().

It turns out that this problem was already fixed before in commit
6a8fd4c76a969ac98d1aff91ff7442f43aee0006 but the problem was
insufficiently documented in the commit and it introduced the bug
that Application::Reschedule() was called without having the SolarMutex
locked, which caused timers to run without SolarMutex, so the commit
was reverted in 1ef1781390845d03b6e1518bbac81b818be62f3d.

Change-Id: I60aae555a9ee3c6390f584feddbc6b3cb7de0250

13 files changed, 56 insertions, 20 deletions

diff --git a/vcl/headless/svpinst.cxx b/vcl/headless/svpinst.cxx
index ddb48b8a26d3..17d98b0efb3a 100644
--- a/vcl/headless/svpinst.cxx
+++ b/vcl/headless/svpinst.cxx
@@ -259,8 +259,10 @@ SalBitmap* SvpSalInstance::CreateSalBitmap()
 #endif
 }
 
-void SvpSalInstance::Yield( bool bWait, bool bHandleAllCurrentEvents )
+void SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong const nReleased)
 {
+    (void) nReleased;
+    assert(nReleased == 0); // not implemented
     // first, check for already queued events.
 
     // release yield mutex
diff --git a/vcl/inc/headless/svpinst.hxx b/vcl/inc/headless/svpinst.hxx
index 44795d56ab49..f8c5d6e33e44 100644
--- a/vcl/inc/headless/svpinst.hxx
+++ b/vcl/inc/headless/svpinst.hxx
@@ -157,7 +157,7 @@ public:
     // wait next event and dispatch
     // must returned by UserEvent (SalFrame::PostEvent)
     // and timer
-    virtual void            Yield( bool bWait, bool bHandleAllCurrentEvents ) SAL_OVERRIDE;
+    virtual void            DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) SAL_OVERRIDE;
     virtual bool            AnyInput( VclInputFlags nType ) SAL_OVERRIDE;
 
     // may return NULL to disable session management
diff --git a/vcl/inc/osx/salinst.h b/vcl/inc/osx/salinst.h
index b2d6133495fa..24b7b6f6deb6 100644
--- a/vcl/inc/osx/salinst.h
+++ b/vcl/inc/osx/salinst.h
@@ -109,7 +109,7 @@ public:
     virtual sal_uLong       ReleaseYieldMutex() SAL_OVERRIDE;
     virtual void            AcquireYieldMutex( sal_uLong nCount ) SAL_OVERRIDE;
     virtual bool            CheckYieldMutex() SAL_OVERRIDE;
-    virtual void            Yield( bool bWait, bool bHandleAllCurrentEvents ) SAL_OVERRIDE;
+    virtual void            DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) SAL_OVERRIDE;
     virtual bool            AnyInput( VclInputFlags nType ) SAL_OVERRIDE;
     virtual SalMenu*        CreateMenu( bool bMenuBar, Menu* pVCLMenu ) SAL_OVERRIDE;
     virtual void            DestroyMenu( SalMenu* ) SAL_OVERRIDE;
diff --git a/vcl/inc/salinst.hxx b/vcl/inc/salinst.hxx
index eb5317746f96..4ad66af11a7f 100644
--- a/vcl/inc/salinst.hxx
+++ b/vcl/inc/salinst.hxx
@@ -127,7 +127,7 @@ public:
     // wait next event and dispatch
     // must returned by UserEvent (SalFrame::PostEvent)
     // and timer
-    virtual void            Yield( bool bWait, bool bHandleAllCurrentEvents ) = 0;
+    virtual void            DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) = 0;
     virtual bool            AnyInput( VclInputFlags nType ) = 0;
 
     // menus
diff --git a/vcl/inc/unx/gtk/gtkinst.hxx b/vcl/inc/unx/gtk/gtkinst.hxx
index 28bca3645b21..9a11df1093d9 100644
--- a/vcl/inc/unx/gtk/gtkinst.hxx
+++ b/vcl/inc/unx/gtk/gtkinst.hxx
@@ -80,7 +80,7 @@ public:
                                                      const SystemGraphicsData* ) SAL_OVERRIDE;
     virtual SalBitmap*          CreateSalBitmap() SAL_OVERRIDE;
 
-    virtual void                Yield( bool bWait, bool bHandleAllCurrentEvents ) SAL_OVERRIDE;
+    virtual void                DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) SAL_OVERRIDE;
     virtual bool                AnyInput( VclInputFlags nType ) SAL_OVERRIDE;
 
     virtual GenPspGraphics     *CreatePrintGraphics() SAL_OVERRIDE;
diff --git a/vcl/inc/unx/salinst.h b/vcl/inc/unx/salinst.h
index 381ddda2e35f..7197e7c6dca2 100644
--- a/vcl/inc/unx/salinst.h
+++ b/vcl/inc/unx/salinst.h
@@ -71,7 +71,7 @@ public:
     virtual SalBitmap*          CreateSalBitmap() SAL_OVERRIDE;
     virtual SalSession*         CreateSalSession() SAL_OVERRIDE;
 
-    virtual void                Yield( bool bWait, bool bHandleAllCurrentEvents ) SAL_OVERRIDE;
+    virtual void                DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) SAL_OVERRIDE;
     virtual bool                AnyInput( VclInputFlags nType ) SAL_OVERRIDE;
 
     virtual void*               GetConnectionIdentifier( ConnectionIdentifierType& rReturnedType, int& rReturnedBytes ) SAL_OVERRIDE;
diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h
index 450d07cfc139..b3337aca9f85 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -62,7 +62,7 @@ public:
     virtual void                AcquireYieldMutex( sal_uIntPtr nCount ) SAL_OVERRIDE;
     virtual bool                CheckYieldMutex() SAL_OVERRIDE;
 
-    virtual void                Yield( bool bWait, bool bHandleAllCurrentEvents ) SAL_OVERRIDE;
+    virtual void                DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong nReleased) SAL_OVERRIDE;
     virtual bool                AnyInput( VclInputFlags nType ) SAL_OVERRIDE;
     virtual SalMenu*            CreateMenu( bool bMenuBar, Menu* ) SAL_OVERRIDE;
     virtual void                DestroyMenu( SalMenu* ) SAL_OVERRIDE;
diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx
index 0d6c710f46ca..f05b15a68603 100644
--- a/vcl/osx/salinst.cxx
+++ b/vcl/osx/salinst.cxx
@@ -559,8 +559,10 @@ class ReleasePoolHolder
     ~ReleasePoolHolder() { [mpPool release]; }
 };
 
-void AquaSalInstance::Yield( bool bWait, bool bHandleAllCurrentEvents )
+void AquaSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong const nReleased)
 {
+    (void) nReleased;
+    assert(nReleased == 0); // not implemented
     // ensure that the per thread autorelease pool is top level and
     // will therefore not be destroyed by cocoa implicitly
     SalData::ensureThreadAutoreleasePool();
diff --git a/vcl/qa/cppunit/timer.cxx b/vcl/qa/cppunit/timer.cxx
index 70f832d1a225..88807fc76207 100644
--- a/vcl/qa/cppunit/timer.cxx
+++ b/vcl/qa/cppunit/timer.cxx
@@ -127,7 +127,7 @@ void TimerTest::testIdleMainloop()
         // can't test this via Application::Yield since this
         // also processes all tasks directly via the scheduler.
         pSVData->maAppData.mnDispatchLevel++;
-        pSVData->mpDefInst->Yield( true, false );
+        pSVData->mpDefInst->DoYield(true, false, 0);
         pSVData->maAppData.mnDispatchLevel--;
     }
     CPPUNIT_ASSERT_MESSAGE("mainloop idle triggered", bTriggered);
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index 536e81b7d229..42efa3b9c39c 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -338,19 +338,23 @@ void Application::Execute()
     pSVData->maAppData.mbInAppExecute = false;
 }
 
-inline void ImplYield( bool i_bWait, bool i_bAllEvents )
+inline void ImplYield(bool i_bWait, bool i_bAllEvents, sal_uLong const nReleased)
 {
     ImplSVData* pSVData = ImplGetSVData();
 
-    //Process all Tasks
-    Scheduler::ProcessTaskScheduling(false);
+    if (nReleased == 0) // else thread doesn't have SolarMutex so avoid race
+    {   // Process all Tasks
+        Scheduler::ProcessTaskScheduling(false);
+    }
 
     pSVData->maAppData.mnDispatchLevel++;
     // do not wait for events if application was already quit; in that
     // case only dispatch events already available
     // do not wait for events either if the app decided that it is too busy for timers
     // (feature added for the slideshow)
-    pSVData->mpDefInst->Yield( i_bWait && !pSVData->maAppData.mbAppQuit && !pSVData->maAppData.mbNoYield, i_bAllEvents );
+    pSVData->mpDefInst->DoYield(
+        i_bWait && !pSVData->maAppData.mbAppQuit && !pSVData->maAppData.mbNoYield,
+        i_bAllEvents, nReleased);
     pSVData->maAppData.mnDispatchLevel--;
 
     DBG_TESTSOLARMUTEX(); // must be locked on return from Yield
@@ -374,12 +378,32 @@ inline void ImplYield( bool i_bWait, bool i_bAllEvents )
 
 void Application::Reschedule( bool i_bAllEvents )
 {
-    ImplYield( false, i_bAllEvents );
+    ImplYield(false, i_bAllEvents, 0);
 }
 
 void Application::Yield()
 {
-    ImplYield( true, false );
+    ImplYield(true, false, 0);
+}
+
+void Application::ReAcquireSolarMutex(sal_uLong const nReleased)
+{
+    // 0 would mean that events/timers will be handled without locking
+    // SolarMutex (racy)
+    assert(nReleased != 0);
+#ifdef WNT
+    if (ImplGetSVData()->mbDeInit) // do not Yield in DeInitVCL
+        AcquireSolarMutex(nReleased);
+    else
+        ImplYield(false, false, nReleased);
+#else
+    // a) Yield is not needed on non-WNT platforms
+    // b) some Yield implementations for X11 (e.g. kde4) make it non-obvious
+    //    how to use nReleased
+    // c) would require a review of what all Yield implementations do
+    //    currently _before_ releasing SolarMutex that would run without lock
+    AcquireSolarMutex(nReleased);
+#endif
 }
 
 IMPL_STATIC_LINK_NOARG( ImplSVAppData, ImplQuitMsg )
diff --git a/vcl/unx/generic/app/salinst.cxx b/vcl/unx/generic/app/salinst.cxx
index 47b64290e40e..4b5f7e2af19d 100644
--- a/vcl/unx/generic/app/salinst.cxx
+++ b/vcl/unx/generic/app/salinst.cxx
@@ -151,8 +151,10 @@ bool X11SalInstance::AnyInput(VclInputFlags nType)
     return bRet;
 }
 
-void X11SalInstance::Yield( bool bWait, bool bHandleAllCurrentEvents )
+void X11SalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong const nReleased)
 {
+    (void) nReleased;
+    assert(nReleased == 0); // not implemented
     mpXLib->Yield( bWait, bHandleAllCurrentEvents );
 }
 
diff --git a/vcl/unx/gtk/app/gtkinst.cxx b/vcl/unx/gtk/app/gtkinst.cxx
index be1bfc20f1da..d31563efe369 100644
--- a/vcl/unx/gtk/app/gtkinst.cxx
+++ b/vcl/unx/gtk/app/gtkinst.cxx
@@ -392,8 +392,10 @@ void GtkInstance::RemoveTimer (SalTimer *pTimer)
         m_aTimers.erase( it );
 }
 
-void GtkInstance::Yield( bool bWait, bool bHandleAllCurrentEvents )
+void GtkInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong const nReleased)
 {
+    (void) nReleased;
+    assert(nReleased == 0); // not implemented
     EnsureInit();
     GetGtkSalData()->Yield( bWait, bHandleAllCurrentEvents );
 }
diff --git a/vcl/win/source/app/salinst.cxx b/vcl/win/source/app/salinst.cxx
index eb39ad32725e..36b93ff1be90 100644
--- a/vcl/win/source/app/salinst.cxx
+++ b/vcl/win/source/app/salinst.cxx
@@ -628,13 +628,17 @@ void ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
     }
 }
 
-void WinSalInstance::Yield( bool bWait, bool bHandleAllCurrentEvents )
+void WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong const nReleased)
 {
+    // NOTE: if nReleased != 0 this will be called without SolarMutex
+    //       so don't do anything dangerous before releasing it here
     SalYieldMutex*  pYieldMutex = mpSalYieldMutex;
     SalData*        pSalData = GetSalData();
     DWORD           nCurThreadId = GetCurrentThreadId();
-    sal_uLong           nCount = pYieldMutex->GetAcquireCount( nCurThreadId );
-    sal_uLong           n = nCount;
+    sal_uLong const nCount = (nReleased != 0)
+                                ? nReleased
+                                : pYieldMutex->GetAcquireCount(nCurThreadId);
+    sal_uLong       n = (nReleased != 0) ? 0 : nCount;
     while ( n )
     {
         pYieldMutex->release();