[Libreoffice-commits] core.git: 2 commits - include/vcl sc/source svx/source

[Michael Stahl]

 include/vcl/svapp.hxx            |   47 +++++++++++++++++++++++++++++++++++++++
 sc/source/core/data/documen3.cxx |    5 +---
 sc/source/core/tool/compiler.cxx |    7 +----
 svx/source/fmcomp/gridctrl.cxx   |   13 ++--------
 4 files changed, 54 insertions(+), 18 deletions(-)

New commits:
commit ed4791eb2d516e2ce509c13ae63c95dd6bcb52c4
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Sep 26 22:25:46 2014 +0200

    svx: convert some horrible code to SolarMutexTryAndBuyGuard
    
    Try to preserve the existing spin-lock insanity as-is.
    
    Change-Id: Ic98aa33d2e56536856892fcf61de672952101ae1

diff --git a/svx/source/fmcomp/gridctrl.cxx b/svx/source/fmcomp/gridctrl.cxx
index 0673bfe..d9e92c3 100644
--- a/svx/source/fmcomp/gridctrl.cxx
+++ b/svx/source/fmcomp/gridctrl.cxx
@@ -3545,23 +3545,16 @@ void DbGridControl::FieldValueChanged(sal_uInt16 _nId, const PropertyChangeEvent
     DbGridColumn* pColumn = ( Location < m_aColumns.size() ) ? m_aColumns[ Location ] : NULL;
     if (pColumn)
     {
-        bool bAcquiredPaintSafety = false;
-        while (!m_bWantDestruction && !bAcquiredPaintSafety)
-            bAcquiredPaintSafety  = Application::GetSolarMutex().tryToAcquire();
+        boost::scoped_ptr<vcl::SolarMutexTryAndBuyGuard> pGuard;
+        while (!m_bWantDestruction && (!pGuard || !pGuard->isAcquired()))
+            pGuard.reset(new vcl::SolarMutexTryAndBuyGuard);
 
         if (m_bWantDestruction)
         {   // at this moment, within another thread, our destructor tries to destroy the listener which called this method
             // => don't do anything
             // 73365 - 23.02.00 - FS
-            if (bAcquiredPaintSafety)
-                // though the above while-loop suggests that (m_bWantDestruction && bAcquiredPaintSafety) is impossible,
-                // it isnt't, as m_bWantDestruction isn't protected with any mutex
-                Application::GetSolarMutex().release();
             return;
         }
-        // here we got the solar mutex, transfer it to a guard for safety reasons
-        SolarMutexGuard aPaintSafety;
-        Application::GetSolarMutex().release();
 
         // and finally do the update ...
         pColumn->UpdateFromField(m_xCurrentRow, m_xFormatter);
commit 423142538e81d28229c769e0617c6a00a648709a
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Sep 26 22:24:14 2014 +0200

    add a RAII class that tries to acquire the SolarMutex and releases it
    
    Motivated by ScCompiler::IsMacro() which has an error return that does
    not release the SolarMutex.
    
    Change-Id: I064219bb3c0d68839a133101491d5f8828a26c7a

diff --git a/include/vcl/svapp.hxx b/include/vcl/svapp.hxx
index e303bc6..64225ec 100644
--- a/include/vcl/svapp.hxx
+++ b/include/vcl/svapp.hxx
@@ -1659,6 +1659,53 @@ protected:
     comphelper::SolarMutex& m_solarMutex;
 };
 
+namespace vcl
+{
+
+/** guard class that uses tryToAcquire() and has isAcquired() to check
+ */
+class SolarMutexTryAndBuyGuard
+    : private boost::noncopyable
+{
+    private:
+        bool m_isAcquired;
+#if OSL_DEBUG_LEVEL > 0
+        bool m_isChecked;
+#endif
+        comphelper::SolarMutex& m_rSolarMutex;
+
+    public:
+
+    SolarMutexTryAndBuyGuard()
+        : m_isAcquired(false)
+#if OSL_DEBUG_LEVEL > 0
+        , m_isChecked(false)
+#endif
+        , m_rSolarMutex(Application::GetSolarMutex())
+
+    {
+        m_isAcquired = m_rSolarMutex.tryToAcquire();
+    }
+
+    ~SolarMutexTryAndBuyGuard()
+    {
+#if OSL_DEBUG_LEVEL > 0
+        assert(m_isChecked);
+#endif
+        if (m_isAcquired)
+            m_rSolarMutex.release();
+    }
+
+    bool isAcquired()
+    {
+#if OSL_DEBUG_LEVEL > 0
+        m_isChecked = true;
+#endif
+        return m_isAcquired;
+    }
+};
+
+} // namespace vcl
 
 /**
  A helper class that calls Application::ReleaseSolarMutex() in its constructor
diff --git a/sc/source/core/data/documen3.cxx b/sc/source/core/data/documen3.cxx
index 6bf981c..739b1f3 100644
--- a/sc/source/core/data/documen3.cxx
+++ b/sc/source/core/data/documen3.cxx
@@ -877,13 +877,12 @@ void ScDocument::RemoveUnoObject( SfxListener& rObject )
             // This check is done after calling EndListening, so a later BroadcastUno call
             // won't touch this object.
 
-            comphelper::SolarMutex& rSolarMutex = Application::GetSolarMutex();
-            if ( rSolarMutex.tryToAcquire() )
+            vcl::SolarMutexTryAndBuyGuard g;
+            if (g.isAcquired())
             {
                 // BroadcastUno is always called with the SolarMutex locked, so if it
                 // can be acquired, this is within the same thread (should not happen)
                 OSL_FAIL( "RemoveUnoObject called from BroadcastUno" );
-                rSolarMutex.release();
             }
             else
             {
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index b4872e2..fa98f83 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -2819,8 +2819,8 @@ bool ScCompiler::IsMacro( const OUString& rName )
     // formulas are compiled from a threaded import may result in a deadlock.
     // Check first if we actually could acquire it and if not bail out.
     /* FIXME: yes, but how ... */
-    comphelper::SolarMutex& rSolarMutex = Application::GetSolarMutex();
-    if (!rSolarMutex.tryToAcquire())
+    vcl::SolarMutexTryAndBuyGuard g;
+    if (!g.isAcquired())
     {
         SAL_WARN( "sc.core", "ScCompiler::IsMacro - SolarMutex would deadlock, not obtaining Basic");
         return false;   // bad luck
@@ -2854,7 +2854,6 @@ bool ScCompiler::IsMacro( const OUString& rName )
     SbxMethod* pMeth = (SbxMethod*) pObj->Find( aName, SbxCLASS_METHOD );
     if( !pMeth )
     {
-        rSolarMutex.release();
         return false;
     }
     // It really should be a BASIC function!
@@ -2862,12 +2861,10 @@ bool ScCompiler::IsMacro( const OUString& rName )
      || ( pMeth->IsFixed() && pMeth->GetType() == SbxEMPTY )
      || !pMeth->ISA(SbMethod) )
     {
-        rSolarMutex.release();
         return false;
     }
     maRawToken.SetExternal( aName.getStr() );
     maRawToken.eOp = ocMacro;
-    rSolarMutex.release();
     return true;
 #endif
 }