[Libreoffice-commits] core.git: vcl/inc vcl/osx vcl/README.scheduler vcl/win

[Jan-Marek Glogowski]

 vcl/README.scheduler    |    8 ++++++++
 vcl/inc/osx/salinst.h   |    2 ++
 vcl/osx/salinst.cxx     |   23 +++++++++++++++++------
 vcl/win/app/salinst.cxx |   28 +++++++---------------------
 4 files changed, 34 insertions(+), 27 deletions(-)

New commits:
commit ce8bbb782b806e429ffb44226162967bed244d94
Author: Jan-Marek Glogowski <glogow at fbihome.de>
Date:   Tue Aug 29 09:40:01 2017 +0200

    Don't wait-yield non-main threads in the main thread
    
    This prevents blocking the main thread by a yielding non-main thread.
    The current solution is to wait on a condition, which is set by the
    main thread on wakeup.
    
    Change-Id: I8d680bb51a36ce1e0d3d4713d47d8e2ef93d7297
    Reviewed-on: https://gerrit.libreoffice.org/42808
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 7e0d6ca8b467..ac4a0dd698d4 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -122,6 +122,14 @@ basically the same we're doing with the LO scheduler as a system event.
 The gen X11 backend has some levels of redirection, but needs quite some work
 to get this fixed.
 
+== General: non-main thread yield ==
+
+Yielding from a non-main thread must not wait in the main thread, as this
+may block the main thread until some events happen.
+
+Currently we wait on an extra conditional, which is cleared by the main event
+loop.
+
 == MacOS implementation details ==
 
 Generally the Scheduler is handled as expected, except on resize, which is
diff --git a/vcl/inc/osx/salinst.h b/vcl/inc/osx/salinst.h
index 65db0d0b0f2a..0e30dfafb693 100644
--- a/vcl/inc/osx/salinst.h
+++ b/vcl/inc/osx/salinst.h
@@ -72,6 +72,8 @@ class AquaSalInstance : public SalInstance
         {}
     };
 
+    bool RunInMainYield( bool bHandleAllCurrentEvents );
+
 public:
     SalYieldMutex*                          mpSalYieldMutex;        // Sal-Yield-Mutex
     OUString                                maDefaultPrinter;
diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx
index e3e101cbc13e..f55e9b4aa297 100644
--- a/vcl/osx/salinst.cxx
+++ b/vcl/osx/salinst.cxx
@@ -525,6 +525,13 @@ void AquaSalInstance::handleAppDefinedEvent( NSEvent* pEvent )
     };
 }
 
+bool AquaSalInstance::RunInMainYield( bool bHandleAllCurrentEvents )
+{
+    OSX_SALDATA_RUNINMAIN_UNION( DoYield( false, bHandleAllCurrentEvents), boolean )
+    assert( false && "Don't call this from the main thread!" );
+    return false;
+
+}
 static bool isWakeupEvent( NSEvent *pEvent )
 {
 SAL_WNODEPRECATED_DECLARATIONS_PUSH
@@ -645,13 +652,17 @@ SAL_WNODEPRECATED_DECLARATIONS_POP
         if ( bHadEvent )
             maWaitingYieldCond.set();
     }
-    else if( bWait )
+    else
     {
-        // #i103162#
-        // wait until the main thread has dispatched an event
-        maWaitingYieldCond.reset();
-        SolarMutexReleaser aReleaser;
-        maWaitingYieldCond.wait();
+        bHadEvent = RunInMainYield( bHandleAllCurrentEvents );
+        if ( !bHadEvent && bWait )
+        {
+            // #i103162#
+            // wait until the main thread has dispatched an event
+            maWaitingYieldCond.reset();
+            SolarMutexReleaser aReleaser;
+            maWaitingYieldCond.wait();
+        }
     }
 
     // we get some apple events way too early
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 23e48532d3b9..b77e84eac739 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -572,31 +572,16 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
     SolarMutexReleaser aReleaser;
     if ( !IsMainThread() )
     {
-        if ( bWait )
+        // If you change the SendMessageW function, you might need to update
+        // the PeekMessage( ... PM_QS_POSTMESSAGE) calls!
+        bDidWork = SendMessageW( mhComWnd, SAL_MSG_THREADYIELD,
+                                 (WPARAM) false, (LPARAM) bHandleAllCurrentEvents );
+        if ( !bDidWork && bWait )
         {
             maWaitingYieldCond.reset();
             maWaitingYieldCond.wait();
             bDidWork = true;
         }
-        else {
-            // #97739# A SendMessage call blocks until the called thread (here: the main thread)
-            // returns. During a yield however, messages are processed in the main thread that might
-            // result in a new message loop due to opening a dialog. Thus, SendMessage would not
-            // return which will block this thread!
-            // Solution: just give up the time slice and hope that messages are processed
-            // by the main thread anyway (where all windows are created)
-            // If the mainthread is not currently handling messages, then our SendMessage would
-            // also do nothing, so this seems to be reasonable.
-
-            // #i18883# only sleep if potential deadlock scenario, ie, when a dialog is open
-            if( ImplGetSVData()->maAppData.mnModalMode )
-                Sleep(1);
-            else
-                // If you change the SendMessageW function, you might need to update
-                // the PeekMessage( ... PM_QS_POSTMESSAGE) calls!
-                bDidWork = SendMessageW( mhComWnd, SAL_MSG_THREADYIELD,
-                                         (WPARAM)bWait, (LPARAM)bHandleAllCurrentEvents );
-        }
     }
     else
     {
@@ -616,7 +601,8 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
     switch ( nMsg )
     {
         case SAL_MSG_THREADYIELD:
-            nRet = static_cast<LRESULT>(ImplSalYield( (bool)wParam, (bool)lParam ));
+            assert( !(bool)wParam );
+            nRet = static_cast<LRESULT>(ImplSalYield( false, (bool)lParam ));
             rDef = FALSE;
             break;
         case SAL_MSG_STARTTIMER: