libreofficekit/source/gtk/lokdocview.cxx | 9 + vcl/README.scheduler.md | 7 + vcl/headless/svpinst.cxx | 148 ++++++++++++++++--------------- vcl/inc/headless/svpinst.hxx | 1 4 files changed, 89 insertions(+), 76 deletions(-)
New commits: commit 5fbc7ab55a119504fc484cef28de52b6d5cf8b12 Author: Jan-Marek Glogowski <[email protected]> AuthorDate: Fri Jun 25 09:55:20 2021 +0200 Commit: Jan-Marek Glogowski <[email protected]> CommitDate: Tue Nov 30 20:39:33 2021 +0100 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 <[email protected]> diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx index 015995016a36..2d7852f3781e 100644 --- a/libreofficekit/source/gtk/lokdocview.cxx +++ b/libreofficekit/source/gtk/lokdocview.cxx @@ -2729,16 +2729,17 @@ static gboolean timeout_wakeup(void *) // integrate our mainloop with LOK's static int lok_poll_callback(void*, int timeoutUs) { - if (timeoutUs) + bool bWasEvent(false); + if (timeoutUs > 0) { guint timeout = g_timeout_add(timeoutUs / 1000, timeout_wakeup, nullptr); - g_main_context_iteration(nullptr, true); + bWasEvent = g_main_context_iteration(nullptr, true); g_source_remove(timeout); } else - g_main_context_iteration(nullptr, FALSE); + bWasEvent = g_main_context_iteration(nullptr, timeoutUs < 0); - return 0; + return bWasEvent ? 1 : 0; } // thread-safe wakeup of our mainloop 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;
