From 94a5281912f08fc098c4360ccd4b45b0e10176fe67b07f8df77e65ae04c2cfe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Chv=C3=A1tal?= Date: Thu, 14 Mar 2019 14:52:17 +0000 Subject: [PATCH] - Add patch to fix build on Leap 42: * kde5.patch OBS-URL: https://build.opensuse.org/package/show/LibreOffice:Factory/libreoffice?expand=0&rev=769 --- kde5.patch | 350 ++++++++++++++++++++++++++++++++++++++++++++ libreoffice.changes | 6 + libreoffice.spec | 2 + 3 files changed, 358 insertions(+) create mode 100644 kde5.patch diff --git a/kde5.patch b/kde5.patch new file mode 100644 index 0000000..49c1250 --- /dev/null +++ b/kde5.patch @@ -0,0 +1,350 @@ +From 265caa4381c048c346c907b017561ab0fe0367ff Mon Sep 17 00:00:00 2001 +From: Michael Stahl +Date: Fri, 22 Feb 2019 19:19:27 +0100 +Subject: [PATCH] tdf#119856 vcl: Qt5/KDE5 RunInMainThread + +The problem with the current approach of transferring calls to the main +thread with Q_EMIT signals is that if the code that should run in the +main thread needs SolarMutex, then the non-main-thread must use +SolarMutexReleaser - but then the main thread will run not only the call +that is needed right now, but will potentially process all pending +events, and the other thread hasn't prepared for that. + +We need the inter-thread feature of Qt::BlockingQueuedConnection and the +non-queued feature of Qt::DirectConnection, but this combination doesn't +appear to exist. + +So the SolarMutexReleaser needs to go - but then the main thread does +need SolarMutex for some things, and hence we need to trick it into +believing it has SolarMutex with the m_bNoYieldLock hack. + +Then it becomes apparent that the main thread may be blocked on either +Qt events, which is fine, or on the SalYieldMutex's m_aMutex, which will +never be released now. + +So the main thread must never block on m_aMutex; the alternative is to +use the same approach as the osx code (and, in a somewhat different +form, the svp code), and add some condition variables on which the main +thread can block if it fails to acquire the m_aMutex immediately. + +It's even possible to do this in a somewhat generic way with lambdas. + +This does appear to work, but it makes the Q_EMIT approach entirely +untenable, because now the main thread will be blocked on the condition +variable and the non-main-thread will be blocked until the Qt event is +processed. + +Change-Id: I6480a6b909d5ec8814b2ff10dbefb0f3686a83c7 +Reviewed-on: https://gerrit.libreoffice.org/68232 +Tested-by: Jenkins +Reviewed-by: Katarina Behrens +--- + vcl/inc/qt5/Qt5Instance.hxx | 7 +- + vcl/qt5/Qt5Instance.cxx | 175 ++++++++++++++++++++++++++++--- + vcl/unx/kde5/KDE5SalInstance.cxx | 23 ++-- + vcl/unx/kde5/KDE5SalInstance.hxx | 8 +- + 4 files changed, 181 insertions(+), 32 deletions(-) + +Index: libreoffice-6.2.2.1/vcl/inc/qt5/Qt5Instance.hxx +=================================================================== +--- libreoffice-6.2.2.1.orig/vcl/inc/qt5/Qt5Instance.hxx ++++ libreoffice-6.2.2.1/vcl/inc/qt5/Qt5Instance.hxx +@@ -27,6 +27,8 @@ + + #include + ++#include ++ + class QApplication; + class SalYieldMutex; + class SalFrame; +@@ -50,15 +52,18 @@ public: + + private Q_SLOTS: + bool ImplYield(bool bWait, bool bHandleAllCurrentEvents); ++ void ImplRunInMain(); + + Q_SIGNALS: + bool ImplYieldSignal(bool bWait, bool bHandleAllCurrentEvents); +- std::unique_ptr createMenuSignal(bool, Menu*); ++ void ImplRunInMainSignal(); + + public: + explicit Qt5Instance(bool bUseCairo = false); + virtual ~Qt5Instance() override; + ++ void RunInMainThread(std::function func); ++ + virtual SalFrame* CreateFrame(SalFrame* pParent, SalFrameStyleFlags nStyle) override; + virtual SalFrame* CreateChildFrame(SystemParentData* pParent, + SalFrameStyleFlags nStyle) override; +Index: libreoffice-6.2.2.1/vcl/qt5/Qt5Instance.cxx +=================================================================== +--- libreoffice-6.2.2.1.orig/vcl/qt5/Qt5Instance.cxx ++++ libreoffice-6.2.2.1/vcl/qt5/Qt5Instance.cxx +@@ -43,13 +43,165 @@ + #include + + #include ++#include ++#include + #include + #include + + #include + ++#include ++#include ++ ++/// TODO: not much Qt5 specific here? could be generalised, esp. for OSX... ++/// this subclass allows for the transfer of a closure for running on the main ++/// thread, to handle all the thread affine stuff in Qt5; the SolarMutex is ++/// "loaned" to the main thread for the execution of the closure. ++/// @note it doesn't work to just use "emit" and signals/slots to move calls to ++/// the main thread, because the other thread has the SolarMutex; the other ++/// thread (typically) cannot release SolarMutex, because then the main thread ++/// will handle all sorts of events and whatnot; this design ensures that the ++/// main thread only runs the passed closure (unless the closure releases ++/// SolarMutex itself, which should probably be avoided). ++class Qt5YieldMutex : public SalYieldMutex ++{ ++public: ++ /// flag only accessed on main thread: ++ /// main thread has "borrowed" SolarMutex from another thread ++ bool m_bNoYieldLock = false; ++ /// members for communication from non-main thread to main thread ++ std::mutex m_RunInMainMutex; ++ std::condition_variable m_InMainCondition; ++ bool m_isWakeUpMain = false; ++ std::function m_Closure; ///< code for main thread to run ++ /// members for communication from main thread to non-main thread ++ std::condition_variable m_ResultCondition; ++ bool m_isResultReady = false; ++ ++ virtual bool IsCurrentThread() const override; ++ virtual void doAcquire(sal_uInt32 nLockCount) override; ++ virtual sal_uInt32 doRelease(bool const bUnlockAll) override; ++}; ++ ++bool Qt5YieldMutex::IsCurrentThread() const ++{ ++ auto const* pSalInst(static_cast(GetSalData()->m_pInstance)); ++ assert(pSalInst); ++ if (pSalInst->IsMainThread() && m_bNoYieldLock) ++ { ++ return true; // main thread has borrowed SolarMutex ++ } ++ return SalYieldMutex::IsCurrentThread(); ++} ++ ++void Qt5YieldMutex::doAcquire(sal_uInt32 nLockCount) ++{ ++ auto const* pSalInst(static_cast(GetSalData()->m_pInstance)); ++ assert(pSalInst); ++ if (!pSalInst->IsMainThread()) ++ { ++ SalYieldMutex::doAcquire(nLockCount); ++ return; ++ } ++ if (m_bNoYieldLock) ++ { ++ return; // special case for main thread: borrowed from other thread ++ } ++ do // main thread acquire... ++ { ++ std::function func; // copy of closure on thread stack ++ { ++ std::unique_lock g(m_RunInMainMutex); ++ if (m_aMutex.tryToAcquire()) ++ { ++ // if there's a closure, the other thread holds m_aMutex ++ assert(!m_Closure); ++ m_isWakeUpMain = false; ++ --nLockCount; // have acquired once! ++ ++m_nCount; ++ break; ++ } ++ m_InMainCondition.wait(g, [this]() { return m_isWakeUpMain; }); ++ m_isWakeUpMain = false; ++ std::swap(func, m_Closure); ++ } ++ if (func) ++ { ++ assert(!m_bNoYieldLock); ++ m_bNoYieldLock = true; // execute closure with borrowed SolarMutex ++ func(); ++ m_bNoYieldLock = false; ++ std::unique_lock g(m_RunInMainMutex); ++ assert(!m_isResultReady); ++ m_isResultReady = true; ++ m_ResultCondition.notify_all(); // unblock other thread ++ } ++ } while (true); ++ SalYieldMutex::doAcquire(nLockCount); ++} ++ ++sal_uInt32 Qt5YieldMutex::doRelease(bool const bUnlockAll) ++{ ++ auto const* pSalInst(static_cast(GetSalData()->m_pInstance)); ++ assert(pSalInst); ++ if (pSalInst->IsMainThread() && m_bNoYieldLock) ++ { ++ return 1; // dummy value ++ } ++ ++ std::unique_lock g(m_RunInMainMutex); ++ // read m_nCount before doRelease (it's guarded by m_aMutex) ++ bool const isReleased(bUnlockAll || m_nCount == 1); ++ sal_uInt32 nCount = SalYieldMutex::doRelease(bUnlockAll); ++ if (isReleased && !pSalInst->IsMainThread()) ++ { ++ m_isWakeUpMain = true; ++ m_InMainCondition.notify_all(); // unblock main thread ++ } ++ return nCount; ++} ++ ++// this could be abstracted to be independent of Qt5 by passing in the ++// event-trigger as another function parameter... ++// it could also be a template of the return type, then it could return the ++// result of func... but then how to handle the result in doAcquire? ++void Qt5Instance::RunInMainThread(std::function func) ++{ ++ DBG_TESTSOLARMUTEX(); ++ if (IsMainThread()) ++ { ++ func(); ++ return; ++ } ++ ++ Qt5YieldMutex* const pMutex(static_cast(GetYieldMutex())); ++ { ++ std::unique_lock g(pMutex->m_RunInMainMutex); ++ assert(!pMutex->m_Closure); ++ pMutex->m_Closure = func; ++ // unblock main thread in case it is blocked on condition ++ pMutex->m_isWakeUpMain = true; ++ pMutex->m_InMainCondition.notify_all(); ++ } ++ // wake up main thread in case it is blocked on event queue ++ // TriggerUserEventProcessing() appears to be insufficient in case the ++ // main thread does QEventLoop::WaitForMoreEvents ++ Q_EMIT ImplRunInMainSignal(); ++ { ++ std::unique_lock g(pMutex->m_RunInMainMutex); ++ pMutex->m_ResultCondition.wait(g, [pMutex]() { return pMutex->m_isResultReady; }); ++ pMutex->m_isResultReady = false; ++ } ++} ++ ++void Qt5Instance::ImplRunInMain() ++{ ++ SolarMutexGuard g; // trigger the dispatch code in Qt5YieldMutex::doAcquire ++ (void)this; // suppress unhelpful [loplugin:staticmethods]; can't be static ++} ++ + Qt5Instance::Qt5Instance(bool bUseCairo) +- : SalGenericInstance(o3tl::make_unique()) ++ : SalGenericInstance(o3tl::make_unique()) + , m_postUserEventId(-1) + , m_bUseCairo(bUseCairo) + { +@@ -65,8 +217,8 @@ Qt5Instance::Qt5Instance(bool bUseCairo) + // is processed before the thread emitting the signal continues + connect(this, SIGNAL(ImplYieldSignal(bool, bool)), this, SLOT(ImplYield(bool, bool)), + Qt::BlockingQueuedConnection); +- connect(this, &Qt5Instance::createMenuSignal, this, &Qt5Instance::CreateMenu, +- Qt::BlockingQueuedConnection); ++ connect(this, &Qt5Instance::ImplRunInMainSignal, this, &Qt5Instance::ImplRunInMain, ++ Qt::QueuedConnection); // no Blocking! + } + + Qt5Instance::~Qt5Instance() +@@ -136,15 +288,14 @@ Qt5Instance::CreateVirtualDevice(SalGrap + + std::unique_ptr Qt5Instance::CreateMenu(bool bMenuBar, Menu* pVCLMenu) + { +- if (qApp->thread() != QThread::currentThread()) +- { +- SolarMutexReleaser aReleaser; +- return Q_EMIT createMenuSignal(bMenuBar, pVCLMenu); +- } +- +- Qt5Menu* pSalMenu = new Qt5Menu(bMenuBar); +- pSalMenu->SetMenu(pVCLMenu); +- return std::unique_ptr(pSalMenu); ++ std::unique_ptr pRet; ++ RunInMainThread([&pRet, bMenuBar, pVCLMenu]() { ++ Qt5Menu* pSalMenu = new Qt5Menu(bMenuBar); ++ pRet.reset(pSalMenu); ++ pSalMenu->SetMenu(pVCLMenu); ++ }); ++ assert(pRet); ++ return pRet; + } + + std::unique_ptr Qt5Instance::CreateMenuItem(const SalItemParams& rItemData) +Index: libreoffice-6.2.2.1/vcl/unx/kde5/KDE5SalInstance.cxx +=================================================================== +--- libreoffice-6.2.2.1.orig/vcl/unx/kde5/KDE5SalInstance.cxx ++++ libreoffice-6.2.2.1/vcl/unx/kde5/KDE5SalInstance.cxx +@@ -46,21 +46,16 @@ KDE5SalInstance::KDE5SalInstance() + pSVData->maAppData.mxToolkitName = OUString("kde5"); + + KDE5SalData::initNWF(); +- +- connect(this, &KDE5SalInstance::createFrameSignal, this, &KDE5SalInstance::CreateFrame, +- Qt::BlockingQueuedConnection); +- connect(this, &KDE5SalInstance::createFilePickerSignal, this, +- &KDE5SalInstance::createFilePicker, Qt::BlockingQueuedConnection); + } + + SalFrame* KDE5SalInstance::CreateFrame(SalFrame* pParent, SalFrameStyleFlags nState) + { +- if (!IsMainThread()) +- { +- SolarMutexReleaser aReleaser; +- return Q_EMIT createFrameSignal(pParent, nState); +- } +- return new KDE5SalFrame(static_cast(pParent), nState, true); ++ SalFrame* pRet(nullptr); ++ RunInMainThread(std::function([&pRet, pParent, nState]() { ++ pRet = new KDE5SalFrame(static_cast(pParent), nState, true); ++ })); ++ assert(pRet); ++ return pRet; + } + + uno::Reference +@@ -68,7 +63,11 @@ KDE5SalInstance::createFilePicker(const + { + if (!IsMainThread()) + { +- return Q_EMIT createFilePickerSignal(xMSF); ++ uno::Reference xRet; ++ RunInMainThread( ++ std::function([&xRet, this, xMSF]() { xRet = this->createFilePicker(xMSF); })); ++ assert(xRet); ++ return xRet; + } + + // In order to insert custom controls, KDE5FilePicker currently relies on KFileWidget +Index: libreoffice-6.2.2.1/vcl/unx/kde5/KDE5SalInstance.hxx +=================================================================== +--- libreoffice-6.2.2.1.orig/vcl/unx/kde5/KDE5SalInstance.hxx ++++ libreoffice-6.2.2.1/vcl/unx/kde5/KDE5SalInstance.hxx +@@ -42,13 +42,7 @@ public: + + virtual bool IsMainThread() const override; + +-Q_SIGNALS: +- SalFrame* createFrameSignal(SalFrame* pParent, SalFrameStyleFlags nStyle); +- +- css::uno::Reference +- createFilePickerSignal(const css::uno::Reference&); +- +-private Q_SLOTS: ++private: + virtual SalFrame* CreateFrame(SalFrame* pParent, SalFrameStyleFlags nStyle) override; + + virtual css::uno::Reference diff --git a/libreoffice.changes b/libreoffice.changes index b69bda0..88f2188 100644 --- a/libreoffice.changes +++ b/libreoffice.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Mar 14 14:44:58 UTC 2019 - Tomáš Chvátal + +- Add patch to fix build on Leap 42: + * kde5.patch + ------------------------------------------------------------------- Tue Mar 12 10:07:34 UTC 2019 - Tomáš Chvátal diff --git a/libreoffice.spec b/libreoffice.spec index a1e9487..58142f9 100644 --- a/libreoffice.spec +++ b/libreoffice.spec @@ -103,6 +103,7 @@ Patch5: old-boost.patch Patch7: libreoffice-postgresql.patch Patch8: 0001-Fix-LTO-segfault-in-libtest_sw_uwriter-test.patch Patch9: boost_169.patch +Patch10: kde5.patch # try to save space by using hardlinks Patch990: install-with-hardlinks.diff # save time by relying on rpm check rather than doing stupid find+grep @@ -969,6 +970,7 @@ Provides %{langname} translations and additional resources (help files, etc.) fo %patch7 %patch8 -p1 %patch9 -p1 +%patch10 -p1 %patch990 -p1 %patch991 -p1