svp: normalize DoYield

I somehow missed / forgot, that SvpInstance::DoYield was now also
yielding on the main thread and doesn't try to do "funky" multi-
threaded event processing anymore (because it's no GUI), since
commit 0efd06de8fca4036c4132b2745a11273b1755df2 ("vcl: fix hangs
in SvpSalInstance"),

So this just moves the main thread part into ImplYield and
implements DoYield like on all other architectures, as described
in README.scheduler.

I've tried to fix the LOK poll to be more sensible.

Change-Id: I4323685aa250e9d62a2f448cef358a7aa8ae862c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117899
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

3 files changed, 84 insertions, 72 deletions

diff --git a/vcl/README.scheduler.md b/vcl/README.scheduler.md
index ae62f10d04a3..bfaf53e017d0 100644
--- a/vcl/README.scheduler.md
+++ b/vcl/README.scheduler.md
@@ -130,11 +130,16 @@ There are three types of events, with different priority:
 They should be processed according to the following code:
 
 ```
-bool DoYield( bool bWait, bool bAllCurrent )
+bool ImplYield(bool bWait, bool bAllCurrent)
 {
+    DBG_TESTSOLARMUTEX();
+    assert(IsMainThread());
+
     bool bWasEvent = ProcessUserEvents( bAllCurrent );
     if ( !bAllCurrent && bWasEvent )
         return true;
+
+    SolarMutexReleaser();
     bWasEvent = ProcessSystemEvents( bAllCurrent, &bWasSchedulerEvent ) || bWasEvent;
     if ( !bWasSchedulerEvent && IsSchedulerEvent() )
     {
diff --git a/vcl/headless/svpinst.cxx b/vcl/headless/svpinst.cxx
index b573f756f411..a42b788a8249 100644
--- a/vcl/headless/svpinst.cxx
+++ b/vcl/headless/svpinst.cxx
@@ -56,6 +56,7 @@
 // FIXME: remove when we re-work the svp mainloop
 #include <unx/salunxtime.h>
 #include <comphelper/lok.hxx>
+#include <tools/debug.hxx>
 
 SvpSalInstance* SvpSalInstance::s_pDefaultInstance = nullptr;
 
@@ -404,7 +405,7 @@ sal_uInt32 SvpSalYieldMutex::doRelease(bool const bUnlockAll)
             if (vcl::lok::isUnipoll())
             {
                 if (pInst)
-                    pInst->Wakeup(SvpRequest::NONE);
+                    pInst->Wakeup();
             }
             else
             {
@@ -420,13 +421,9 @@ sal_uInt32 SvpSalYieldMutex::doRelease(bool const bUnlockAll)
 bool SvpSalYieldMutex::IsCurrentThread() const
 {
     if (GetSalData()->m_pInstance->IsMainThread() && m_bNoYieldLock)
-    {
         return true;
-    }
     else
-    {
         return SalYieldMutex::IsCurrentThread();
-    }
 }
 
 bool SvpSalInstance::IsMainThread() const
@@ -443,94 +440,103 @@ void SvpSalInstance::updateMainThread()
     }
 }
 
-bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
+bool SvpSalInstance::ImplYield(bool bWait, bool bHandleAllCurrentEvents)
 {
     DBG_TESTSVPYIELDMUTEX();
+    DBG_TESTSOLARMUTEX();
+    assert(IsMainThread());
 
-    // first, process current user events
-    bool bEvent = DispatchUserEvents(bHandleAllCurrentEvents);
-    if (!bHandleAllCurrentEvents && bEvent)
+    bool bWasEvent = DispatchUserEvents(bHandleAllCurrentEvents);
+    if (!bHandleAllCurrentEvents && bWasEvent)
         return true;
 
-    ImplSVData* pSVData = ImplGetSVData();
+    bWasEvent = CheckTimeout() || bWasEvent;
+    const bool bMustSleep = bWait && !bWasEvent;
 
-    bool bTimeout = CheckTimeout();
-    bool bSkipPoll = bEvent;
-    if (pSVData->mpPollCallback == nullptr)
-        bSkipPoll = bEvent || bTimeout;
-    // else - give the poll-callback visibility into waiting timeouts too.
+    // This is wrong and must be removed!
+    // We always want to drop the SolarMutex on yield; that is the whole point of yield.
+    if (!bMustSleep)
+        return bWasEvent;
 
-    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
-
-    if (IsMainThread())
+    sal_Int64 nTimeoutMicroS = 0;
+    if (bMustSleep)
     {
-        // in kit case
-        if (bWait && !bSkipPoll)
+        if (m_aTimeout.tv_sec) // Timer is started.
         {
-            sal_Int64 nTimeoutMicroS = 0;
-            if (m_aTimeout.tv_sec) // Timer is started.
-            {
-                timeval Timeout;
-                // determine remaining timeout.
-                gettimeofday (&Timeout, nullptr);
-                if (m_aTimeout > Timeout)
-                    nTimeoutMicroS = ((m_aTimeout.tv_sec - Timeout.tv_sec) * 1000 * 1000 +
-                                      (m_aTimeout.tv_usec - Timeout.tv_usec));
-            }
-            else
-                nTimeoutMicroS = -1; // wait until something happens
+            timeval Timeout;
+            // determine remaining timeout.
+            gettimeofday (&Timeout, nullptr);
+            if (m_aTimeout > Timeout)
+                nTimeoutMicroS = ((m_aTimeout.tv_sec - Timeout.tv_sec) * 1000 * 1000 +
+                                  (m_aTimeout.tv_usec - Timeout.tv_usec));
+        }
+        else
+            nTimeoutMicroS = -1; // wait until something happens
+    }
 
-            SolarMutexReleaser aReleaser;
+    SolarMutexReleaser aReleaser;
 
-            if (pSVData->mpPollCallback)
-            {
-                // Poll for events from the LOK client.
-                if (nTimeoutMicroS < 0)
-                    nTimeoutMicroS = 5000 * 1000;
-
-                // External poll.
-                if (pSVData->mpPollClosure != nullptr &&
-                    pSVData->mpPollCallback(pSVData->mpPollClosure, nTimeoutMicroS) < 0)
-                    pSVData->maAppData.mbAppQuit = true;
-            }
-            else
-            {
-                std::unique_lock<std::mutex> g(pMutex->m_WakeUpMainMutex);
-                // wait for doRelease() or Wakeup() to set the condition
-                if (nTimeoutMicroS == -1)
-                {
-                    pMutex->m_WakeUpMainCond.wait(g,
-                            [pMutex]() { return pMutex->m_wakeUpMain; });
-                }
-                else
-                {
-                    int nTimeoutMS = nTimeoutMicroS / 1000;
-                    if ( nTimeoutMicroS % 1000 )
-                        nTimeoutMS += 1;
-                    pMutex->m_WakeUpMainCond.wait_for(g,
-                            std::chrono::milliseconds(nTimeoutMS),
-                            [pMutex]() { return pMutex->m_wakeUpMain; });
-                }
-                // here no need to check m_Request because Acquire will do it
-            }
+    if (vcl::lok::isUnipoll())
+    {
+        ImplSVData* pSVData = ImplGetSVData();
+        if (pSVData->mpPollClosure)
+        {
+            int nPollResult = pSVData->mpPollCallback(pSVData->mpPollClosure, nTimeoutMicroS);
+            if (nPollResult < 0)
+                pSVData->maAppData.mbAppQuit = true;
+            bWasEvent = bWasEvent || (nPollResult != 0);
+        }
+    }
+    else if (bMustSleep)
+    {
+        SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
+        std::unique_lock<std::mutex> g(pMutex->m_WakeUpMainMutex);
+        // wait for doRelease() or Wakeup() to set the condition
+        if (nTimeoutMicroS == -1)
+        {
+            pMutex->m_WakeUpMainCond.wait(g,
+                    [pMutex]() { return pMutex->m_wakeUpMain; });
         }
         else
         {
-            if (bSkipPoll)
-                pMutex->m_NonMainWaitingYieldCond.set(); // wake up other threads
+            int nTimeoutMS = nTimeoutMicroS / 1000;
+            if (nTimeoutMicroS % 1000)
+                nTimeoutMS += 1;
+            pMutex->m_WakeUpMainCond.wait_for(g,
+                    std::chrono::milliseconds(nTimeoutMS),
+                    [pMutex]() { return pMutex->m_wakeUpMain; });
         }
+        // here no need to check m_Request because Acquire will do it
+    }
+
+    return bWasEvent;
+}
+
+bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
+{
+    DBG_TESTSVPYIELDMUTEX();
+    DBG_TESTSOLARMUTEX();
+
+    bool bWasEvent(false);
+    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
+
+    if (IsMainThread())
+    {
+        bWasEvent = ImplYield(bWait, bHandleAllCurrentEvents);
+        if (bWasEvent)
+            pMutex->m_NonMainWaitingYieldCond.set(); // wake up other threads
     }
-    else // !IsMainThread()
+    else
     {
+        // TODO: use a SolarMutexReleaser here and drop the m_bNoYieldLock usage
         Wakeup(bHandleAllCurrentEvents
                 ? SvpRequest::MainThreadDispatchAllEvents
                 : SvpRequest::MainThreadDispatchOneEvent);
 
-        bool bDidWork(false);
         // blocking read (for synchronisation)
-        auto const nRet = read(pMutex->m_FeedbackFDs[0], &bDidWork, sizeof(bool));
+        auto const nRet = read(pMutex->m_FeedbackFDs[0], &bWasEvent, sizeof(bool));
         assert(nRet == 1); (void) nRet;
-        if (!bDidWork && bWait)
+        if (!bWasEvent && bWait)
         {
             // block & release YieldMutex until the main thread does something
             pMutex->m_NonMainWaitingYieldCond.reset();
@@ -539,7 +545,7 @@ bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
         }
     }
 
-    return bSkipPoll;
+    return bWasEvent;
 }
 
 bool SvpSalInstance::AnyInput( VclInputFlags nType )
diff --git a/vcl/inc/headless/svpinst.hxx b/vcl/inc/headless/svpinst.hxx
index 51de503c3b24..06c6e432ffed 100644
--- a/vcl/inc/headless/svpinst.hxx
+++ b/vcl/inc/headless/svpinst.hxx
@@ -101,6 +101,7 @@ class VCL_DLLPUBLIC SvpSalInstance : public SalGenericInstance, public SalUserEv
 
     virtual void            TriggerUserEventProcessing() override;
     virtual void            ProcessEvent( SalUserEvent aEvent ) override;
+    bool ImplYield(bool bWait, bool bHandleAllCurrentEvents);
 
 public:
     static SvpSalInstance*  s_pDefaultInstance;