[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-4.1' - 3 commits - sfx2/source ucb/source

Tor Lillqvist tlillqvist at suse.com
Mon Oct 28 13:19:10 PDT 2013


 sfx2/source/doc/objxtor.cxx                  |   15 +++++++++++
 ucb/source/ucp/webdav-neon/NeonLockStore.cxx |   36 +++++++++++++++++++++------
 ucb/source/ucp/webdav-neon/NeonLockStore.hxx |    2 +
 ucb/source/ucp/webdav-neon/NeonSession.cxx   |   11 ++++++++
 ucb/source/ucp/webdav-neon/NeonSession.hxx   |    3 +-
 5 files changed, 58 insertions(+), 9 deletions(-)

New commits:
commit be51484b6b16bb11f3e03f446b35ed57680f0815
Author: Tor Lillqvist <tlillqvist at suse.com>
Date:   Thu May 30 09:15:10 2013 +0300

    Unfortunately mstahl's fix for the webdav-neon deadlock was not sufficient
    
    Even if stopTicker() now by itself is OK, it is called from
    removeLock(), which already has acquired the same mutex that
    stopTicker() now carefully makes sure to not hold when calling join()
    on the ticker thread. (Remember, our mutexes are recursive.) So we
    still easily get a deadlock.
    
    So for now, just don't bother with the micro-optimisation of stopping
    the ticker thread if there are no WebDAV locks to refresh.

diff --git a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
index b4bbaec..e89da12 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -197,9 +197,6 @@ void NeonLockStore::removeLock( NeonLock * pLock )
 
     m_aLockInfoMap.erase( pLock );
     ne_lockstore_remove( m_pNeonLockStore, pLock );
-
-    if ( m_aLockInfoMap.empty() )
-        stopTicker();
 }
 
 void NeonLockStore::unlockLock( NeonLock * pLock )
commit d051654df39b63bbfe23c995e85f4d708c09e629
Author: Tor Lillqvist <tlillqvist at suse.com>
Date:   Thu May 30 06:48:39 2013 +0200

    bnc#805901: Lock WebDAV document that is opened for potential modification
    
    Really horrible fix, breaking all rules of proper abstraction etc.
    
    Basically, two parts:
    
    1) When opening a document over WebDAV, in Content::execute(), lock it
    too. This is simple. With just this change, the WebDAV resource gets
    locked but it stays locked for the rest of the soffice.bin lifetime,
    even if the document is closed much earlier. This also means you can't
    re-open it without re-starting LibreOffice...
    
    The NeonLockStore (which is effectively a singleton, as the only
    object of this type that exists is the static
    NeonSession::m_aNeonLockStore field) destructor takes care of blowing
    awway the locks when the process exists.
    
    So obviously that is not good enough. Thus a second part is needed:
    
    2) Then closing a document, over in SfxObjectShell::Close(), do a
    horrible trick: look up the ucpdav1 module, and if it is loaded, look
    up the NeonSessionUnlockByUri() function in it, and call it, passing
    the document's URI.
    
    That function, in NeonSession.cxx, looks up the NeonLock for the URI,
    and if it exists, unlocks it, and removs the lock from the
    NeonLockStore.
    
    Conflicts:
    
    	ucb/source/ucp/webdav-neon/NeonLockStore.cxx
    	ucb/source/ucp/webdav-neon/webdavcontent.cxx
    
    Change-Id: If9af1f8b5d3a89cdea34ccd0b751d5f671ccccba

diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx
index e606c44..a7fe41f 100644
--- a/sfx2/source/doc/objxtor.cxx
+++ b/sfx2/source/doc/objxtor.cxx
@@ -22,6 +22,8 @@
 #include "arrdecl.hxx"
 #include <map>
 
+#include <osl/module.h>
+
 #include <cppuhelper/implbase1.hxx>
 
 #include <com/sun/star/util/XCloseable.hpp>
@@ -473,6 +475,19 @@ sal_Bool SfxObjectShell::Close()
             if ( it != rDocs.end() )
                 rDocs.erase( it );
             pImp->bInList = sal_False;
+
+            if (pMedium && !pMedium->GetOrigURL().isEmpty())
+            {
+                oslModule aUcpDav1;
+                if (osl_getModuleHandle(OUString(SAL_MODULENAME("ucpdav1")).pData, &aUcpDav1))
+                {
+                    void (*pNeonSessionUnlockByUri)(rtl::OUString uri) =
+                        (void (*)(rtl::OUString)) osl_getAsciiFunctionSymbol(aUcpDav1, "NeonSessionUnlockByUri");
+                    if (pNeonSessionUnlockByUri)
+                        pNeonSessionUnlockByUri(pMedium->GetOrigURL());
+                }
+            }
+
         }
     }
 
diff --git a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
index 043ea7d..b4bbaec 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -202,6 +202,22 @@ void NeonLockStore::removeLock( NeonLock * pLock )
         stopTicker();
 }
 
+void NeonLockStore::unlockLock( NeonLock * pLock )
+{
+    osl::MutexGuard aGuard( m_aMutex );
+
+    LockInfoMap::const_iterator it( m_aLockInfoMap.begin() );
+    const LockInfoMap::const_iterator end( m_aLockInfoMap.end() );
+    while ( it != end )
+    {
+        NeonLock * pLockIt = (*it).first;
+        if (pLockIt == pLock)
+            (*it).second.xSession->UNLOCK( pLock );
+        ++it;
+    }
+ }
+
+// -------------------------------------------------------------------
 void NeonLockStore::refreshLocks()
 {
     osl::MutexGuard aGuard( m_aMutex );
diff --git a/ucb/source/ucp/webdav-neon/NeonLockStore.hxx b/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
index dd9b185..1b041ad 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
@@ -92,6 +92,8 @@ public:
 
     void removeLock( NeonLock * pLock );
 
+    void unlockLock( NeonLock * pLock );
+
     void refreshLocks();
 
 private:
diff --git a/ucb/source/ucp/webdav-neon/NeonSession.cxx b/ucb/source/ucp/webdav-neon/NeonSession.cxx
index cee643a..65afb25 100644
--- a/ucb/source/ucp/webdav-neon/NeonSession.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonSession.cxx
@@ -2074,4 +2074,15 @@ OUString NeonSession::makeAbsoluteURL( OUString const & rURL ) const
     return OUString();
 }
 
+extern "C" SAL_DLLPUBLIC_EXPORT void NeonSessionUnlockByUri(rtl::OUString uri)
+{
+    NeonLock *pLock = NeonSession::m_aNeonLockStore.findByUri(uri);
+
+    if (!pLock)
+        return;
+
+    NeonSession::m_aNeonLockStore.unlockLock(pLock);
+    NeonSession::m_aNeonLockStore.removeLock(pLock);
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/ucb/source/ucp/webdav-neon/NeonSession.hxx b/ucb/source/ucp/webdav-neon/NeonSession.hxx
index f62ec00..806c9f1 100644
--- a/ucb/source/ucp/webdav-neon/NeonSession.hxx
+++ b/ucb/source/ucp/webdav-neon/NeonSession.hxx
@@ -65,12 +65,13 @@ private:
     DAVRequestEnvironment m_aEnv;
 
     static bool          m_bGlobalsInited;
-    static NeonLockStore m_aNeonLockStore;
 
 protected:
     virtual ~NeonSession();
 
 public:
+    static NeonLockStore m_aNeonLockStore;
+
     NeonSession( const rtl::Reference< DAVSessionFactory > & rSessionFactory,
                  const OUString& inUri,
                  const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::NamedValue >& rFlags,
commit 3404f57c7e36f525279b126ef6a1c874aa3f2cad
Author: Michael Stahl <mstahl at redhat.com>
Date:   Wed May 29 21:18:29 2013 +0200

    ucb: NeonLockStore::stopTicker(): avoid deadlock
    
    Tor reports that NeonLockStore::stopTicker() m_pTickerThread->join()
    can deadlock with TickerThread running NeonLockStore::refreshLocks().
    
    This can be avoided by copying m_pTickerThread to the stack, and
    releasing the m_aMutex before calling join().
    
    Change-Id: I387f83a530c5b893f79fa677b1092e0902c8af65
    (cherry picked from commit 68ba2785c55eaa1ea70ce135bdad5322b0e04ed7)

diff --git a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
index 04608a6..043ea7d 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -128,14 +128,21 @@ void NeonLockStore::startTicker()
 
 void NeonLockStore::stopTicker()
 {
-    osl::MutexGuard aGuard( m_aMutex );
-
-    if ( m_pTickerThread.is() )
+    rtl::Reference<TickerThread> pTickerThread;
     {
-        m_pTickerThread->finish();
-        m_pTickerThread->join();
+        osl::MutexGuard aGuard( m_aMutex );
+
+        if (!m_pTickerThread.is())
+        {
+            return; // nothing to do
+        }
+        m_pTickerThread->finish(); // needs mutex
+        // the TickerThread may run refreshLocks() at most once after this
+        pTickerThread = m_pTickerThread;
         m_pTickerThread.clear();
     }
+
+    pTickerThread->join(); // without m_aMutex locked (to prevent deadlock)
 }
 
 void NeonLockStore::registerSession( HttpSession * pHttpSession )


More information about the Libreoffice-commits mailing list