diff --git a/vcl/android/androidinst.cxx b/vcl/android/androidinst.cxx index ca130fb1b19f..56497db19270 100644 --- a/vcl/android/androidinst.cxx +++ b/vcl/android/androidinst.cxx @@ -157,7 +157,7 @@ SalFrame *AndroidSalInstance::CreateFrame( SalFrame* pParent, SalFrameStyleFlags extern "C" SalInstance *create_SalInstance() { LOGI("Android: create_SalInstance!"); - AndroidSalInstance* pInstance = new AndroidSalInstance( std::make_unique() ); + AndroidSalInstance* pInstance = new AndroidSalInstance( std::make_unique() ); new SvpSalData(); return pInstance; } diff --git a/vcl/headless/headlessinst.cxx b/vcl/headless/headlessinst.cxx index abe3e1cf92c7..162fd5bd865b 100644 --- a/vcl/headless/headlessinst.cxx +++ b/vcl/headless/headlessinst.cxx @@ -47,7 +47,7 @@ SalSystem *HeadlessSalInstance::CreateSalSystem() extern "C" SalInstance *create_SalInstance() { - HeadlessSalInstance* pInstance = new HeadlessSalInstance(std::make_unique()); + HeadlessSalInstance* pInstance = new HeadlessSalInstance(std::make_unique()); new SvpSalData(); return pInstance; } diff --git a/vcl/headless/svpinst.cxx b/vcl/headless/svpinst.cxx index bf53dc24faf3..10beeeb58fbd 100644 --- a/vcl/headless/svpinst.cxx +++ b/vcl/headless/svpinst.cxx @@ -56,115 +56,31 @@ #include #include #include +#include SvpSalInstance* SvpSalInstance::s_pDefaultInstance = nullptr; -#ifndef NDEBUG -static bool g_CheckedMutex = false; - -#define DBG_TESTSVPYIELDMUTEX() \ -do { \ - if (!g_CheckedMutex) \ - { \ - assert(dynamic_cast(GetYieldMutex()) != nullptr \ - && "This SvpSalInstance function requires use of SvpSalYieldMutex"); \ - g_CheckedMutex = true; \ - } \ -} while(false) - -#else // NDEBUG -#define DBG_TESTSVPYIELDMUTEX() ((void)0) -#endif - -#if !defined(ANDROID) && !defined(IOS) - -static void atfork_child() -{ - if (SvpSalInstance::s_pDefaultInstance != nullptr) - { - SvpSalInstance::s_pDefaultInstance->CloseWakeupPipe(false); - SvpSalInstance::s_pDefaultInstance->CreateWakeupPipe(false); - } -} - -#endif - SvpSalInstance::SvpSalInstance( std::unique_ptr pMutex ) : SalGenericInstance( std::move(pMutex) ) + , m_WaitCondition(m_NonMainYieldMutex) + , m_EventCondition(m_NonMainYieldMutex) + , m_MainYieldCondition(m_MainYieldMutex) { m_aTimeout.tv_sec = 0; m_aTimeout.tv_usec = 0; m_nTimeoutMS = 0; m_MainThread = osl::Thread::getCurrentIdentifier(); - CreateWakeupPipe(true); if( s_pDefaultInstance == nullptr ) s_pDefaultInstance = this; -#if !defined(ANDROID) && !defined(IOS) - pthread_atfork(nullptr, nullptr, atfork_child); -#endif + + m_bSupportsOpenGL = false; } SvpSalInstance::~SvpSalInstance() { if( s_pDefaultInstance == this ) s_pDefaultInstance = nullptr; - CloseWakeupPipe(true); -} - -void SvpSalInstance::CloseWakeupPipe(bool log) -{ - SvpSalYieldMutex *const pMutex(dynamic_cast(GetYieldMutex())); - if (!pMutex) - return; - if (pMutex->m_FeedbackFDs[0] != -1) - { - if (log) - { - SAL_INFO("vcl.headless", "CloseWakeupPipe: Closing inherited feedback pipe: [" << pMutex->m_FeedbackFDs[0] << "," << pMutex->m_FeedbackFDs[1] << "]"); - } - close (pMutex->m_FeedbackFDs[0]); - close (pMutex->m_FeedbackFDs[1]); - pMutex->m_FeedbackFDs[0] = pMutex->m_FeedbackFDs[1] = -1; - } -} - -void SvpSalInstance::CreateWakeupPipe(bool log) -{ - SvpSalYieldMutex *const pMutex(dynamic_cast(GetYieldMutex())); - if (!pMutex) - return; - if (pipe (pMutex->m_FeedbackFDs) == -1) - { - if (log) - { - SAL_WARN("vcl.headless", "Could not create feedback pipe: " << strerror(errno)); - std::abort(); - } - } - else - { - if (log) - { - SAL_INFO("vcl.headless", "CreateWakeupPipe: Created feedback pipe: [" << pMutex->m_FeedbackFDs[0] << "," << pMutex->m_FeedbackFDs[1] << "]"); - } - - int flags; - - // set close-on-exec descriptor flag. - if ((flags = fcntl (pMutex->m_FeedbackFDs[0], F_GETFD)) != -1) - { - flags |= FD_CLOEXEC; - (void) fcntl(pMutex->m_FeedbackFDs[0], F_SETFD, flags); - } - if ((flags = fcntl (pMutex->m_FeedbackFDs[1], F_GETFD)) != -1) - { - flags |= FD_CLOEXEC; - (void) fcntl(pMutex->m_FeedbackFDs[1], F_SETFD, flags); - } - - // retain the default blocking I/O for feedback pipe - } } void SvpSalInstance::TriggerUserEventProcessing() @@ -172,20 +88,21 @@ void SvpSalInstance::TriggerUserEventProcessing() Wakeup(); } -void SvpSalInstance::Wakeup(SvpRequest const request) +void SvpSalInstance::Wakeup() { - DBG_TESTSVPYIELDMUTEX(); + if (IsMainThread()) + return; - ImplSVData* pSVData = ImplGetSVData(); - if (pSVData->mpWakeCallback && pSVData->mpPollClosure) - pSVData->mpWakeCallback(pSVData->mpPollClosure); - - SvpSalYieldMutex *const pMutex(static_cast(GetYieldMutex())); - std::scoped_lock g(pMutex->m_WakeUpMainMutex); - if (request != SvpRequest::NONE) - pMutex->m_Request = request; - pMutex->m_wakeUpMain = true; - pMutex->m_WakeUpMainCond.notify_one(); + if (vcl::lok::isUnipoll()) + { + ImplSVData* pSVData = ImplGetSVData(); + if (pSVData->mpWakeCallback && pSVData->mpPollClosure) + pSVData->mpWakeCallback(pSVData->mpPollClosure); + } + else + { + m_MainYieldCondition.notify_all(); + } } bool SvpSalInstance::CheckTimeout( bool bExecuteTimers ) @@ -204,8 +121,6 @@ bool SvpSalInstance::CheckTimeout( bool bExecuteTimers ) m_aTimeout = aTimeOfDay; m_aTimeout += m_nTimeoutMS; - osl::Guard< comphelper::SolarMutex > aGuard( GetYieldMutex() ); - // notify ImplSVData* pSVData = ImplGetSVData(); if( pSVData->maSchedCtx.mpSalTimer ) @@ -311,8 +226,6 @@ std::shared_ptr SvpSalInstance::CreateSalBitmap() void SvpSalInstance::ProcessEvent( SalUserEvent aEvent ) { - DBG_TESTSVPYIELDMUTEX(); - aEvent.m_pFrame->CallCallback( aEvent.m_nEvent, aEvent.m_pData ); if( aEvent.m_nEvent == SalEvent::Resize ) { @@ -320,109 +233,6 @@ void SvpSalInstance::ProcessEvent( SalUserEvent aEvent ) const SvpSalFrame* pSvpFrame = static_cast( aEvent.m_pFrame); pSvpFrame->PostPaint(); } - - SvpSalYieldMutex *const pMutex(static_cast(GetYieldMutex())); - pMutex->m_NonMainWaitingYieldCond.set(); -} - -SvpSalYieldMutex::SvpSalYieldMutex() -{ -#ifndef IOS - m_FeedbackFDs[0] = m_FeedbackFDs[1] = -1; -#endif -} - -SvpSalYieldMutex::~SvpSalYieldMutex() -{ -} - -void SvpSalYieldMutex::doAcquire(sal_uInt32 const nLockCount) -{ - auto *const pInst = static_cast(GetSalInstance()); - if (pInst && pInst->IsMainThread()) - { - if (m_bNoYieldLock) - return; - - do - { - SvpRequest request = SvpRequest::NONE; - { - std::unique_lock g(m_WakeUpMainMutex); - if (m_aMutex.tryToAcquire()) { - // if there's a request, the other thread holds m_aMutex - assert(m_Request == SvpRequest::NONE); - m_wakeUpMain = false; - break; - } - m_WakeUpMainCond.wait(g, [this]() { return m_wakeUpMain; }); - m_wakeUpMain = false; - std::swap(m_Request, request); - } - if (request != SvpRequest::NONE) - { - // nested Yield on behalf of another thread - assert(!m_bNoYieldLock); - m_bNoYieldLock = true; - bool const bEvents = pInst->DoYield(false, request == SvpRequest::MainThreadDispatchAllEvents); - m_bNoYieldLock = false; - if (write(m_FeedbackFDs[1], &bEvents, sizeof(bool)) != sizeof(bool)) - { - SAL_WARN("vcl.headless", "Could not write: " << strerror(errno)); - std::abort(); - } - } - } - while (true); - } - else - { - m_aMutex.acquire(); - } - ++m_nCount; - SalYieldMutex::doAcquire(nLockCount - 1); -} - -sal_uInt32 SvpSalYieldMutex::doRelease(bool const bUnlockAll) -{ - auto *const pInst = static_cast(GetSalInstance()); - if (pInst && pInst->IsMainThread()) - { - if (m_bNoYieldLock) - return 1; - else - return SalYieldMutex::doRelease(bUnlockAll); - } - sal_uInt32 nCount; - { - // read m_nCount before doRelease - bool const isReleased(bUnlockAll || m_nCount == 1); - nCount = comphelper::SolarMutex::doRelease( bUnlockAll ); - - if (isReleased) - { - if (vcl::lok::isUnipoll()) - { - if (pInst) - pInst->Wakeup(); - } - else - { - std::scoped_lock g(m_WakeUpMainMutex); - m_wakeUpMain = true; - m_WakeUpMainCond.notify_one(); - } - } - } - return nCount; -} - -bool SvpSalYieldMutex::IsCurrentThread() const -{ - if (GetSalInstance()->IsMainThread() && m_bNoYieldLock) - return true; - else - return SalYieldMutex::IsCurrentThread(); } bool SvpSalInstance::IsMainThread() const @@ -439,25 +249,17 @@ void SvpSalInstance::updateMainThread() } } -bool SvpSalInstance::ImplYield(bool bWait, bool bHandleAllCurrentEvents) +bool SvpSalInstance::ImplYield(bool bWait) { - DBG_TESTSVPYIELDMUTEX(); DBG_TESTSOLARMUTEX(); assert(IsMainThread()); - bool bWasEvent = DispatchUserEvents(bHandleAllCurrentEvents); - if (!bHandleAllCurrentEvents && bWasEvent) - return true; - - bWasEvent = CheckTimeout() || bWasEvent; - const bool bMustSleep = bWait && !bWasEvent; - - // 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; + bool bWasEvent = DispatchUserEvents(true); + const bool bTimeout = CheckTimeout(); + bWasEvent = bTimeout || bWasEvent; sal_Int64 nTimeoutMicroS = 0; + const bool bMustSleep = bWait && !bWasEvent; if (bMustSleep) { if (m_aTimeout.tv_sec) // Timer is started. @@ -474,13 +276,21 @@ bool SvpSalInstance::ImplYield(bool bWait, bool bHandleAllCurrentEvents) } SolarMutexReleaser aReleaser; + m_bWasEvent = bWasEvent; if (vcl::lok::isUnipoll()) { ImplSVData* pSVData = ImplGetSVData(); if (pSVData->mpPollClosure) { - int nPollResult = pSVData->mpPollCallback(pSVData->mpPollClosure, nTimeoutMicroS); + int nPollResult = 0; + if (!bMustSleep || (nTimeoutMicroS != 0)) + nPollResult = pSVData->mpPollCallback(pSVData->mpPollClosure, 0); + if (bMustSleep && nPollResult == 0) + { + m_EventCondition.notify_all(); + nPollResult = pSVData->mpPollCallback(pSVData->mpPollClosure, nTimeoutMicroS); + } if (nPollResult < 0) pSVData->maAppData.mbAppQuit = true; bWasEvent = bWasEvent || (nPollResult != 0); @@ -488,59 +298,63 @@ bool SvpSalInstance::ImplYield(bool bWait, bool bHandleAllCurrentEvents) } else if (bMustSleep) { - SvpSalYieldMutex *const pMutex(static_cast(GetYieldMutex())); - std::unique_lock g(pMutex->m_WakeUpMainMutex); - // wait for doRelease() or Wakeup() to set the condition - if (nTimeoutMicroS == -1) + assert(!bWasEvent); + if (nTimeoutMicroS == 0) { - pMutex->m_WakeUpMainCond.wait(g, - [pMutex]() { return pMutex->m_wakeUpMain; }); + m_EventCondition.notify_all(); + return bWasEvent; } + + auto wakeup = [this]() { m_EventCondition.notify_all(); return false; }; + + if (nTimeoutMicroS == -1) + m_MainYieldCondition.wait(wakeup); 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; }); + m_MainYieldCondition.wait_for(std::chrono::milliseconds(nTimeoutMS), wakeup); } - // here no need to check m_Request because Acquire will do it } + if (bWasEvent) + m_WaitCondition.notify_all(); + m_EventCondition.notify_all(); + return bWasEvent; } -bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents) +bool SvpSalInstance::DoYield(bool bWait, bool) { - DBG_TESTSVPYIELDMUTEX(); DBG_TESTSOLARMUTEX(); bool bWasEvent(false); - SvpSalYieldMutex *const pMutex(static_cast(GetYieldMutex())); if (IsMainThread()) - { - bWasEvent = ImplYield(bWait, bHandleAllCurrentEvents); - if (bWasEvent) - pMutex->m_NonMainWaitingYieldCond.set(); // wake up other threads - } + bWasEvent = ImplYield(bWait); else { - // TODO: use a SolarMutexReleaser here and drop the m_bNoYieldLock usage - Wakeup(bHandleAllCurrentEvents - ? SvpRequest::MainThreadDispatchAllEvents - : SvpRequest::MainThreadDispatchOneEvent); - - // blocking read (for synchronisation) - auto const nRet = read(pMutex->m_FeedbackFDs[0], &bWasEvent, sizeof(bool)); - assert(nRet == 1); (void) nRet; - if (!bWasEvent && bWait) + SolarMutexReleaser aReleaser; + if (bWait) { - // block & release YieldMutex until the main thread does something - pMutex->m_NonMainWaitingYieldCond.reset(); - SolarMutexReleaser aReleaser; - pMutex->m_NonMainWaitingYieldCond.wait(); + m_WaitCondition.wait(); + bWasEvent = true; + } + else + { + // prevent ABBA deadlock + std::unique_lock g(m_MainYieldCondition.mutex()); + const bool bIsSleeping = m_MainYieldCondition.isSleeping(); + m_EventCondition.wait([bIsSleeping, &g]() + { + g.unlock(); + return bIsSleeping; + }); + if (bIsSleeping) + bWasEvent = false; + else + bWasEvent = m_bWasEvent; } } diff --git a/vcl/inc/headless/svpinst.hxx b/vcl/inc/headless/svpinst.hxx index 874ce672d97a..9bba1bec57cd 100644 --- a/vcl/inc/headless/svpinst.hxx +++ b/vcl/inc/headless/svpinst.hxx @@ -29,6 +29,9 @@ #include #include +#include +#include +#include #include @@ -39,10 +42,56 @@ #define SvpSalInstance AquaSalInstance #endif +class SvpYieldCondition final +{ + bool m_bReady = false; + bool m_bSleeping = false; + std::mutex& m_rMutex; + std::condition_variable m_Conditional; + +public: + SvpYieldCondition(std::mutex& rMutex) : m_rMutex(rMutex) {} + + void notify_all() + { + { + std::lock_guard g(m_rMutex); + m_bReady = true; + } + m_Conditional.notify_all(); + } + + void wait(std::function func = [](){ return false; }) + { + std::unique_lock g(m_rMutex); + if (func()) + return; + m_bSleeping = true; + m_Conditional.wait(g, [this]() { return m_bReady; }); + m_bSleeping = false; + m_bReady = false; + } + + void wait_for(std::chrono::milliseconds rel_time, std::function func = [](){ return false; }) + { + std::unique_lock g(m_rMutex); + if (func()) + return; + m_bSleeping = true; + m_Conditional.wait_for(g, rel_time, [this]() { return m_bReady; }); + m_bSleeping = false; + m_bReady = false; + } + + std::mutex& mutex() const { return m_rMutex; } + bool isSleeping() const { return m_bSleeping; } +}; + class SvpSalInstance; class SvpSalTimer final : public SalTimer { SvpSalInstance* m_pInstance; + public: SvpSalTimer( SvpSalInstance* pInstance ) : m_pInstance( pInstance ) {} virtual ~SvpSalTimer() override; @@ -55,51 +104,25 @@ public: class SvpSalFrame; class GenPspGraphics; -enum class SvpRequest -{ - NONE, - MainThreadDispatchOneEvent, - MainThreadDispatchAllEvents, -}; - -class SvpSalYieldMutex final : public SalYieldMutex -{ -private: - // note: these members might as well live in SvpSalInstance, but there is - // at least one subclass of SvpSalInstance (GTK3) that doesn't use them. - friend class SvpSalInstance; - // members for communication from main thread to non-main thread - int m_FeedbackFDs[2]; - osl::Condition m_NonMainWaitingYieldCond; - // members for communication from non-main thread to main thread - bool m_bNoYieldLock = false; // accessed only on main thread - std::mutex m_WakeUpMainMutex; // guard m_wakeUpMain & m_Request - std::condition_variable m_WakeUpMainCond; - bool m_wakeUpMain = false; - SvpRequest m_Request = SvpRequest::NONE; - - virtual void doAcquire( sal_uInt32 nLockCount ) override; - virtual sal_uInt32 doRelease( bool bUnlockAll ) override; - -public: - SvpSalYieldMutex(); - virtual ~SvpSalYieldMutex() override; - - virtual bool IsCurrentThread() const override; -}; - -// NOTE: the functions IsMainThread, DoYield and Wakeup *require* the use of -// SvpSalYieldMutex; if a subclass uses something else it must override these -// (Wakeup is only called by SvpSalTimer and SvpSalFrame) class VCL_DLLPUBLIC SvpSalInstance : public SalGenericInstance, public SalUserEventList { timeval m_aTimeout; sal_uLong m_nTimeoutMS; oslThreadIdentifier m_MainThread; + // members for communication from main thread to non-main thread + std::mutex m_NonMainYieldMutex; + SvpYieldCondition m_WaitCondition; + SvpYieldCondition m_EventCondition; + bool m_bWasEvent; + + // members for communication from non-main thread to main thread + std::mutex m_MainYieldMutex; + SvpYieldCondition m_MainYieldCondition; + virtual void TriggerUserEventProcessing() override; virtual void ProcessEvent( SalUserEvent aEvent ) override; - bool ImplYield(bool bWait, bool bHandleAllCurrentEvents); + bool ImplYield(bool bWait); public: static SvpSalInstance* s_pDefaultInstance; @@ -107,9 +130,7 @@ public: SvpSalInstance( std::unique_ptr pMutex ); virtual ~SvpSalInstance() override; - void CloseWakeupPipe(bool log); - void CreateWakeupPipe(bool log); - void Wakeup(SvpRequest request = SvpRequest::NONE); + void Wakeup(); void StartTimer( sal_uInt64 nMS ); void StopTimer(); diff --git a/vcl/ios/iosinst.cxx b/vcl/ios/iosinst.cxx index a9fa27e5ea82..0a0a11594dfe 100644 --- a/vcl/ios/iosinst.cxx +++ b/vcl/ios/iosinst.cxx @@ -146,7 +146,7 @@ void SalData::ensureThreadAutoreleasePool() {} extern "C" SalInstance *create_SalInstance() { - IosSalInstance* pInstance = new IosSalInstance( std::make_unique() ); + IosSalInstance* pInstance = new IosSalInstance( std::make_unique() ); new SvpSalData(pInstance); return pInstance; } diff --git a/vcl/source/app/salplug.cxx b/vcl/source/app/salplug.cxx index 378187dbfedc..bf0815567ca2 100644 --- a/vcl/source/app/salplug.cxx +++ b/vcl/source/app/salplug.cxx @@ -82,7 +82,7 @@ namespace { #if ENABLE_HEADLESS SalInstance* svp_create_SalInstance() { - SvpSalInstance* pInstance = new SvpSalInstance(std::make_unique()); + SvpSalInstance* pInstance = new SvpSalInstance(std::make_unique()); new SvpSalData(); return pInstance; }