[Libreoffice-commits] core.git: Branch 'distro/suse/suse-4.0' - 4 commits - sfx2/source sw/source ucb/source

Tor Lillqvist tlillqvist at suse.com
Thu May 30 00:01:24 PDT 2013


 sfx2/source/doc/objxtor.cxx                  |   15 +++++++++++
 sw/source/core/doc/poolfmt.cxx               |    1 
 ucb/source/ucp/webdav-neon/NeonLockStore.cxx |   35 ++++++++++++++++++++-------
 ucb/source/ucp/webdav-neon/NeonLockStore.hxx |    2 +
 ucb/source/ucp/webdav-neon/NeonSession.cxx   |   11 ++++++++
 ucb/source/ucp/webdav-neon/NeonSession.hxx   |    3 +-
 ucb/source/ucp/webdav-neon/webdavcontent.cxx |    6 ++++
 7 files changed, 63 insertions(+), 10 deletions(-)

New commits:
commit 259f3815cad69ef86007331bb231554c4cb8b4b1
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 e5f204f..74da3f3 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -207,9 +207,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 9c15384bc67e7878e8efefa422b4ab39d438413f
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.

diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx
index 08b2743..6e64fae 100644
--- a/sfx2/source/doc/objxtor.cxx
+++ b/sfx2/source/doc/objxtor.cxx
@@ -21,6 +21,8 @@
 #include "arrdecl.hxx"
 #include <map>
 
+#include <osl/module.h>
+
 #include <cppuhelper/implbase1.hxx>
 
 #include <com/sun/star/util/XCloseable.hpp>
@@ -469,6 +471,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 c4d8533..e5f204f 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -212,6 +212,21 @@ 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()
 {
diff --git a/ucb/source/ucp/webdav-neon/NeonLockStore.hxx b/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
index c8f0f0f..27e37d4 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.hxx
@@ -91,6 +91,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 f1f3b9f..b4a90a9 100644
--- a/ucb/source/ucp/webdav-neon/NeonSession.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonSession.cxx
@@ -2204,4 +2204,15 @@ rtl::OUString NeonSession::makeAbsoluteURL( rtl::OUString const & rURL ) const
     return rtl::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 15c88bb..0c6cc8d 100644
--- a/ucb/source/ucp/webdav-neon/NeonSession.hxx
+++ b/ucb/source/ucp/webdav-neon/NeonSession.hxx
@@ -68,12 +68,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 rtl::OUString& inUri,
                  const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::NamedValue >& rFlags,
diff --git a/ucb/source/ucp/webdav-neon/webdavcontent.cxx b/ucb/source/ucp/webdav-neon/webdavcontent.cxx
index a00763e..03578fe 100644
--- a/ucb/source/ucp/webdav-neon/webdavcontent.cxx
+++ b/ucb/source/ucp/webdav-neon/webdavcontent.cxx
@@ -500,6 +500,12 @@ uno::Any SAL_CALL Content::execute(
         }
 
         aRet = open( aOpenCommand, Environment );
+#if 1
+        if ( (aOpenCommand.Mode == ucb::OpenMode::DOCUMENT ||
+              aOpenCommand.Mode == ucb::OpenMode::DOCUMENT_SHARE_DENY_WRITE) &&
+             supportsExclusiveWriteLock( Environment ) )
+            lock( Environment );
+#endif
     }
     else if ( aCommand.Name == "insert" )
     {
commit 5ffde2307ba2d0d139bccf4e2cdf487849bf4f5f
Author: Tor Lillqvist <tlillqvist at suse.com>
Date:   Wed May 29 20:57:55 2013 +0300

    Bin uninteresting (?) OSL_FAIL()

diff --git a/sw/source/core/doc/poolfmt.cxx b/sw/source/core/doc/poolfmt.cxx
index 6edbaf6..d8e1c02 100644
--- a/sw/source/core/doc/poolfmt.cxx
+++ b/sw/source/core/doc/poolfmt.cxx
@@ -1379,7 +1379,6 @@ bool SwDoc::IsPoolFmtUsed( sal_uInt16 nId ) const
     }
     else
     {
-        OSL_FAIL( "invalid Id" );
         bFnd = false;
     }
 
commit 47c45cee8642407c74e53d17ecfa9c49c54ce91f
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 77e6258..c4d8533 100644
--- a/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
+++ b/ucb/source/ucp/webdav-neon/NeonLockStore.cxx
@@ -133,14 +133,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)
 }
 
 // -------------------------------------------------------------------


More information about the Libreoffice-commits mailing list