Move yield mutex handling into SalInstance

After the refectoring in commit 4c93de2c ("merge
GenericSolarMutex and SolarMutex"), there is no more need to
prevent instantiation of comphelper::SolarMutex objects.

Since every VCL backend implements the yield mutex management in
the same way, we can move the general implementation into the
SalInstance.

While at it use std::unique_ptr for the yield mutex on Mac and
Windows platforms.

Change-Id: Ibe0610bd92b4623152ee14e7a35b52465d403720
Reviewed-on: https://gerrit.libreoffice.org/60570
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

include/comphelper/solarmutex.hxx

@@ -44,6 +44,9 @@ class COMPHELPER_DLLPUBLIC SolarMutex {
 public:
     typedef void (*BeforeReleaseHandler) ();
 
+    SolarMutex();
+    virtual ~SolarMutex();
+
     void SetBeforeReleaseHandler( const BeforeReleaseHandler& rLink )
          { m_aBeforeReleaseHandler = rLink; }
 
@@ -59,9 +62,6 @@ public:
     static SolarMutex *get();
 
 protected:
-    SolarMutex();
-    virtual ~SolarMutex();
-
     virtual sal_uInt32 doRelease( bool bUnlockAll );
     virtual void doAcquire( sal_uInt32 nLockCount );
 

vcl/headless/svpinst.cxx

@@ -94,7 +94,7 @@ SvpSalInstance::~SvpSalInstance()
 
 void SvpSalInstance::CloseWakeupPipe(bool log)
 {
-    SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()));
+    SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
     if (!pMutex)
         return;
     if (pMutex->m_FeedbackFDs[0] != -1)
@@ -111,7 +111,7 @@ void SvpSalInstance::CloseWakeupPipe(bool log)
 
 void SvpSalInstance::CreateWakeupPipe(bool log)
 {
-    SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()));
+    SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
     if (!pMutex)
         return;
     if (pipe (pMutex->m_FeedbackFDs) == -1)
@@ -163,7 +163,7 @@ void SvpSalInstance::Wakeup(SvpRequest const request)
 #ifndef NDEBUG
     if (!g_CheckedMutex)
     {
-        assert(dynamic_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()) != nullptr
+        assert(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()) != nullptr
             && "This SvpSalInstance function requires use of SvpSalYieldMutex");
         g_CheckedMutex = true;
     }
@@ -171,7 +171,7 @@ void SvpSalInstance::Wakeup(SvpRequest const request)
 #ifdef IOS
     (void)request;
 #else
-    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()));
+    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
     std::unique_lock<std::mutex> g(pMutex->m_WakeUpMainMutex);
     if (request != SvpRequest::NONE)
     {
@@ -198,7 +198,7 @@ bool SvpSalInstance::CheckTimeout( bool bExecuteTimers )
                 m_aTimeout = aTimeOfDay;
                 m_aTimeout += m_nTimeoutMS;
 
-                osl::Guard< comphelper::SolarMutex > aGuard( mpSalYieldMutex.get() );
+                osl::Guard< comphelper::SolarMutex > aGuard( GetYieldMutex() );
 
                 // notify
                 ImplSVData* pSVData = ImplGetSVData();
@@ -287,12 +287,12 @@ void SvpSalInstance::ProcessEvent( SalUserEvent aEvent )
 #ifndef NDEBUG
     if (!g_CheckedMutex)
     {
-        assert(dynamic_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()) != nullptr
+        assert(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()) != nullptr
             && "This SvpSalInstance function requires use of SvpSalYieldMutex");
         g_CheckedMutex = true;
     }
 #endif
-    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()));
+    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
     pMutex->m_NonMainWaitingYieldCond.set();
 }
 
@@ -404,7 +404,7 @@ bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
 #ifndef NDEBUG
     if (!g_CheckedMutex)
     {
-        assert(dynamic_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()) != nullptr
+        assert(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()) != nullptr
             && "This SvpSalInstance function requires use of SvpSalYieldMutex");
         g_CheckedMutex = true;
     }
@@ -417,7 +417,7 @@ bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
 
     bEvent = CheckTimeout() || bEvent;
 
-    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(mpSalYieldMutex.get()));
+    SvpSalYieldMutex *const pMutex(static_cast<SvpSalYieldMutex*>(GetYieldMutex()));
 
     if (IsMainThread())
     {

vcl/inc/osx/salinst.h

@@ -76,7 +76,6 @@ class AquaSalInstance : public SalInstance, public SalUserEventList
 public:
     virtual void TriggerUserEventProcessing() override;
 
-    SalYieldMutex*                          mpSalYieldMutex;        // Sal-Yield-Mutex
     OUString                                maDefaultPrinter;
     oslThreadIdentifier                     maMainThread;
     int                                     mnActivePrintJobs;
@@ -113,9 +112,6 @@ public:
     virtual SalTimer*       CreateSalTimer() override;
     virtual SalSystem*      CreateSalSystem() override;
     virtual std::shared_ptr<SalBitmap> CreateSalBitmap() override;
-    virtual comphelper::SolarMutex* GetYieldMutex() override;
-    virtual sal_uInt32      ReleaseYieldMutexAll() override;
-    virtual void            AcquireYieldMutex( sal_uInt32 nCount = 1 ) override;
     virtual bool            DoYield(bool bWait, bool bHandleAllCurrentEvents) override;
     virtual bool            AnyInput( VclInputFlags nType ) override;
     virtual std::unique_ptr<SalMenu>     CreateMenu( bool bMenuBar, Menu* pVCLMenu ) override;

vcl/inc/qt5/Qt5Instance.hxx

@@ -54,7 +54,7 @@ Q_SIGNALS:
     bool ImplYieldSignal(bool bWait, bool bHandleAllCurrentEvents);
 
 public:
-    explicit Qt5Instance(std::unique_ptr<SalYieldMutex> pMutex, bool bUseCairo = false);
+    explicit Qt5Instance(bool bUseCairo = false);
     virtual ~Qt5Instance() override;
 
     virtual SalFrame* CreateFrame(SalFrame* pParent, SalFrameStyleFlags nStyle) override;

vcl/inc/salinst.hxx

@@ -81,9 +81,10 @@ class VCL_PLUGIN_PUBLIC SalInstance
 {
 private:
     rtl::Reference< vcl::DisplayConnectionDispatch > m_pEventInst;
+    const std::unique_ptr<comphelper::SolarMutex> m_pYieldMutex;
 
 public:
-    SalInstance() {}
+    SalInstance(std::unique_ptr<comphelper::SolarMutex> pMutex);
     virtual ~SalInstance();
 
     //called directly after Application::Init
@@ -135,11 +136,11 @@ public:
     virtual std::shared_ptr<SalBitmap> CreateSalBitmap() = 0;
 
     // YieldMutex
-    virtual comphelper::SolarMutex*
-                            GetYieldMutex() = 0;
-    virtual sal_uInt32      ReleaseYieldMutexAll() = 0;
-    virtual void            AcquireYieldMutex( sal_uInt32 nCount = 1 ) = 0;
-    // return true, if yield mutex is owned by this thread, else false
+    comphelper::SolarMutex* GetYieldMutex();
+    sal_uInt32              ReleaseYieldMutexAll();
+    void                    AcquireYieldMutex(sal_uInt32 nCount = 1);
+
+    // return true, if the current thread is the main thread
     virtual bool            IsMainThread() const = 0;
 
     /**

vcl/inc/unx/geninst.h

@@ -20,6 +20,7 @@
 #ifndef INCLUDED_VCL_INC_GENERIC_GENINST_H
 #define INCLUDED_VCL_INC_GENERIC_GENINST_H
 
+#include <memory>
 #include <comphelper/solarmutex.hxx>
 #include <tools/solar.h>
 #include <osl/thread.hxx>
@@ -44,18 +45,12 @@ class VCL_DLLPUBLIC SalGenericInstance : public SalInstance
 {
 protected:
     bool           mbPrinterInit;
-    std::unique_ptr<SalYieldMutex> mpSalYieldMutex;
 
 public:
-    SalGenericInstance( std::unique_ptr<SalYieldMutex> pMutex )
-        : mbPrinterInit( false ), mpSalYieldMutex( std::move(pMutex) ) {}
+    SalGenericInstance( std::unique_ptr<comphelper::SolarMutex> pMutex )
+        : SalInstance(std::move(pMutex)), mbPrinterInit(false) {}
     virtual ~SalGenericInstance() override;
 
-    // Yield mutex
-    virtual comphelper::SolarMutex* GetYieldMutex() override;
-    virtual sal_uInt32         ReleaseYieldMutexAll() override;
-    virtual void               AcquireYieldMutex( sal_uInt32 nCount = 1 ) override;
-
     // Printing
     virtual SalInfoPrinter*     CreateInfoPrinter      ( SalPrinterQueueInfo* pQueueInfo,
                                                          ImplJobSetup* pSetupData ) override;

vcl/inc/win/salinst.h

@@ -35,8 +35,6 @@ public:
     HINSTANCE           mhInst;
     /// invisible Window so non-main threads can SendMessage() the main thread
     HWND                mhComWnd;
-    /// The Yield mutex ensures that only one thread calls into VCL
-    SalYieldMutex*      mpSalYieldMutex;
 
     osl::Condition      maWaitingYieldCond;
     unsigned            m_nNoYieldLock;
@@ -65,9 +63,6 @@ public:
     virtual SalTimer*           CreateSalTimer() override;
     virtual SalSystem*          CreateSalSystem() override;
     virtual std::shared_ptr<SalBitmap> CreateSalBitmap() override;
-    virtual comphelper::SolarMutex* GetYieldMutex() override;
-    virtual sal_uInt32          ReleaseYieldMutexAll() override;
-    virtual void                AcquireYieldMutex( sal_uInt32 nCount = 1 ) override;
     virtual bool                IsMainThread() const override;
 
     virtual bool                DoYield(bool bWait, bool bHandleAllCurrentEvents) override;

vcl/osx/salinst.cxx

@@ -114,7 +114,7 @@ public:
 
 void AquaSalInstance::delayedSettingsChanged( bool bInvalidate )
 {
-    osl::Guard< comphelper::SolarMutex > aGuard( *mpSalYieldMutex );
+    osl::Guard< comphelper::SolarMutex > aGuard( *GetYieldMutex() );
     AquaDelayedSettingsChanged* pIdle = new AquaDelayedSettingsChanged( bInvalidate );
     pIdle->SetDebugName( "AquaSalInstance AquaDelayedSettingsChanged" );
     pIdle->Start();
@@ -355,7 +355,7 @@ bool ImplSalYieldMutexTryToAcquire()
 {
     AquaSalInstance* pInst = GetSalData()->mpInstance;
     if ( pInst )
-        return pInst->mpSalYieldMutex->tryToAcquire();
+        return pInst->GetYieldMutex()->tryToAcquire();
     else
         return FALSE;
 }
@@ -364,7 +364,7 @@ void ImplSalYieldMutexRelease()
 {
     AquaSalInstance* pInst = GetSalData()->mpInstance;
     if ( pInst )
-        pInst->mpSalYieldMutex->release();
+        pInst->GetYieldMutex()->release();
 }
 
 SalInstance* CreateSalInstance()
@@ -398,20 +398,19 @@ void DestroySalInstance( SalInstance* pInst )
 }
 
 AquaSalInstance::AquaSalInstance()
-    : mnActivePrintJobs( 0 )
+    : SalInstance(o3tl::make_unique<SalYieldMutex>())
+    , mnActivePrintJobs( 0 )
     , mbIsLiveResize( false )
     , mbNoYieldLock( false )
     , mbTimerProcessed( false )
 {
-    mpSalYieldMutex = new SalYieldMutex;
-    mpSalYieldMutex->acquire();
+    GetYieldMutex()->acquire();
     maMainThread = osl::Thread::getCurrentIdentifier();
 }
 
 AquaSalInstance::~AquaSalInstance()
 {
-    mpSalYieldMutex->release();
-    delete mpSalYieldMutex;
+    GetYieldMutex()->release();
 }
 
 void AquaSalInstance::TriggerUserEventProcessing()
@@ -427,21 +426,6 @@ void AquaSalInstance::ProcessEvent( SalUserEvent aEvent )
     maWaitingYieldCond.set();
 }
 
-comphelper::SolarMutex* AquaSalInstance::GetYieldMutex()
-{
-    return mpSalYieldMutex;
-}
-
-sal_uInt32 AquaSalInstance::ReleaseYieldMutexAll()
-{
-    return mpSalYieldMutex->release( true/*bUnlockAll*/ );
-}
-
-void AquaSalInstance::AcquireYieldMutex( sal_uInt32 nCount )
-{
-    mpSalYieldMutex->acquire( nCount );
-}
-
 bool AquaSalInstance::IsMainThread() const
 {
     return osl::Thread::getCurrentIdentifier() == maMainThread;

vcl/qt5/Qt5Instance.cxx

@@ -43,8 +43,8 @@
 
 #include <headless/svpbmp.hxx>
 
-Qt5Instance::Qt5Instance(std::unique_ptr<SalYieldMutex> pMutex, bool bUseCairo)
-    : SalGenericInstance(std::move(pMutex))
+Qt5Instance::Qt5Instance(bool bUseCairo)
+    : SalGenericInstance(o3tl::make_unique<SalYieldMutex>())
     , m_postUserEventId(-1)
     , m_bUseCairo(bUseCairo)
 {
@@ -286,7 +286,7 @@ VCLPLUG_QT5_PUBLIC SalInstance* create_SalInstance()
     QApplication::setQuitOnLastWindowClosed(false);
 
     static const bool bUseCairo = (nullptr != getenv("SAL_VCL_QT5_USE_CAIRO"));
-    Qt5Instance* pInstance = new Qt5Instance(o3tl::make_unique<SalYieldMutex>(), bUseCairo);
+    Qt5Instance* pInstance = new Qt5Instance(bUseCairo);
 
     // initialize SalData
     new Qt5Data(pInstance);

vcl/source/app/salvtables.cxx

@@ -79,10 +79,30 @@ void SalFrame::SetRepresentedURL( const OUString& )
     // currently this is Mac only functionality
 }
 
+SalInstance::SalInstance(std::unique_ptr<comphelper::SolarMutex> pMutex)
+    : m_pYieldMutex(std::move(pMutex))
+{
+}
+
 SalInstance::~SalInstance()
 {
 }
 
+comphelper::SolarMutex* SalInstance::GetYieldMutex()
+{
+    return m_pYieldMutex.get();
+}
+
+sal_uInt32 SalInstance::ReleaseYieldMutexAll()
+{
+    return m_pYieldMutex->release(true);
+}
+
+void SalInstance::AcquireYieldMutex(sal_uInt32 nCount)
+{
+    m_pYieldMutex->acquire(nCount);
+}
+
 std::unique_ptr<SalSession> SalInstance::CreateSalSession()
 {
     return nullptr;

vcl/unx/generic/app/geninst.cxx

@@ -48,21 +48,6 @@ SalYieldMutex::~SalYieldMutex()
 {
 }
 
-comphelper::SolarMutex* SalGenericInstance::GetYieldMutex()
-{
-    return mpSalYieldMutex.get();
-}
-
-sal_uInt32 SalGenericInstance::ReleaseYieldMutexAll()
-{
-    return mpSalYieldMutex.get()->release( true/*bUnlockAll*/ );
-}
-
-void SalGenericInstance::AcquireYieldMutex( sal_uInt32 nCount )
-{
-    mpSalYieldMutex.get()->acquire( nCount );
-}
-
 SalGenericInstance::~SalGenericInstance()
 {
 }

vcl/unx/kde5/KDE5SalInstance.cxx

@@ -39,8 +39,8 @@
 
 using namespace com::sun::star;
 
-KDE5SalInstance::KDE5SalInstance(std::unique_ptr<SalYieldMutex> pMutex)
-    : Qt5Instance(std::move(pMutex), true)
+KDE5SalInstance::KDE5SalInstance()
+    : Qt5Instance(true)
 {
     ImplSVData* pSVData = ImplGetSVData();
     pSVData->maAppData.mxToolkitName = OUString("kde5");
@@ -134,7 +134,7 @@ VCLPLUG_KDE5_PUBLIC SalInstance* create_SalInstance()
 
     QApplication::setQuitOnLastWindowClosed(false);
 
-    KDE5SalInstance* pInstance = new KDE5SalInstance(o3tl::make_unique<SalYieldMutex>());
+    KDE5SalInstance* pInstance = new KDE5SalInstance();
 
     // initialize SalData
     new KDE5SalData(pInstance);

vcl/unx/kde5/KDE5SalInstance.hxx

@@ -32,7 +32,7 @@ class SalFrame;
 class KDE5SalInstance : public Qt5Instance
 {
 public:
-    explicit KDE5SalInstance(std::unique_ptr<SalYieldMutex> pMutex);
+    explicit KDE5SalInstance();
     virtual SalFrame* CreateFrame(SalFrame* pParent, SalFrameStyleFlags nStyle) override;
 
     virtual bool hasNativeFileSelection() const override { return true; }

vcl/win/app/salinst.cxx

@@ -133,7 +133,7 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount )
         if ( pInst->m_nNoYieldLock )
             return;
         // tdf#96887 If this is the main thread, then we must wait for two things:
-        // - the mpSalYieldMutex being freed
+        // - the yield mutex being unlocked
         // - SendMessage() being triggered
         // This can nicely be done using MsgWaitForMultipleObjects. The 2nd one is
         // needed because if we don't reschedule, then we create deadlocks if a
@@ -189,13 +189,13 @@ void ImplSalYieldMutexAcquireWithWait( sal_uInt32 nCount )
 {
     WinSalInstance* pInst = GetSalData()->mpInstance;
     if ( pInst )
-        pInst->mpSalYieldMutex->acquire( nCount );
+        pInst->GetYieldMutex()->acquire( nCount );
 }
 
 bool ImplSalYieldMutexTryToAcquire()
 {
     WinSalInstance* pInst = GetSalData()->mpInstance;
-    return pInst && pInst->mpSalYieldMutex->tryToAcquire();
+    return pInst && pInst->GetYieldMutex()->tryToAcquire();
 }
 
 void ImplSalYieldMutexRelease()
@@ -204,7 +204,7 @@ void ImplSalYieldMutexRelease()
     if ( pInst )
     {
         GdiFlush();
-        pInst->mpSalYieldMutex->release();
+        pInst->GetYieldMutex()->release();
     }
 }
 
@@ -422,35 +422,19 @@ void DestroySalInstance( SalInstance* pInst )
 }
 
 WinSalInstance::WinSalInstance()
-    : mhComWnd( nullptr )
+    : SalInstance(o3tl::make_unique<SalYieldMutex>())
+    , mhComWnd( nullptr )
     , m_nNoYieldLock( 0 )
 {
-    mpSalYieldMutex = new SalYieldMutex();
-    mpSalYieldMutex->acquire();
+    GetYieldMutex()->acquire();
 }
 
 WinSalInstance::~WinSalInstance()
 {
-    mpSalYieldMutex->release();
-    delete mpSalYieldMutex;
+    GetYieldMutex()->release();
     DestroyWindow( mhComWnd );
 }
 
-comphelper::SolarMutex* WinSalInstance::GetYieldMutex()
-{
-    return mpSalYieldMutex;
-}
-
-sal_uInt32 WinSalInstance::ReleaseYieldMutexAll()
-{
-    return mpSalYieldMutex->release( true/*bUnlockAll*/ );
-}
-
-void WinSalInstance::AcquireYieldMutex( sal_uInt32 nCount )
-{
-    mpSalYieldMutex->acquire( nCount );
-}
-
 static LRESULT ImplSalDispatchMessage( const MSG* pMsg )
 {
     SalData* pSalData = GetSalData();