[Libreoffice-commits] core.git: ucb/source

[Michael Stahl (via logerrit)]

 ucb/source/ucp/webdav-curl/SerfLockStore.cxx |   23 ++++++++++++-----------
 ucb/source/ucp/webdav-curl/SerfLockStore.hxx |    2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

New commits:
commit 21a1267b15724fcadb089065f57c5daf0fd6ba26
Author:     Michael Stahl <mst...@redhat.com>
AuthorDate: Thu May 30 17:13:50 2013 +0200
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Nov 1 18:24:26 2021 +0100

    ucb: webdav-curl: NeonLockStore::stopTicker(): really avoid deadlock
    
    Follow up on 68ba2785c55eaa1ea70ce135bdad5322b0e04ed7, which missed the
    sad fact that m_aMutex is locked recursively.
    
    Avoid that by passing a ClearableMutexGuard to stopTicker() and
    unlocking that.  Also lock m_aMutex in the destructor while at it.
    
    [ port of commit 13b60fd80a7adfb0ef81a818917cfec5edfc6edc ]
    
    Change-Id: I65bb4ec98f13d28089c3b2eb3300074944024188
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123181
    Tested-by: Michael Stahl <michael.st...@allotropia.de>
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>

diff --git a/ucb/source/ucp/webdav-curl/SerfLockStore.cxx 
b/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
index 6380fe6e7447..62526bd873fc 100644
--- a/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
+++ b/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
@@ -89,7 +89,9 @@ SerfLockStore::SerfLockStore()
 
 SerfLockStore::~SerfLockStore()
 {
-    stopTicker();
+    osl::ResettableMutexGuard aGuard(m_aMutex);
+    stopTicker(aGuard);
+    aGuard.reset(); // actually no threads should even try to access members 
now
     m_bFinishing = true;
 
     // release active locks, if any.
@@ -119,23 +121,22 @@ void SerfLockStore::startTicker()
 }
 
 
-void SerfLockStore::stopTicker()
+void SerfLockStore::stopTicker(osl::ClearableMutexGuard & rGuard)
 {
     rtl::Reference<TickerThread> pTickerThread;
-    {
-        osl::MutexGuard aGuard( m_aMutex );
 
-        if (!m_pTickerThread.is())
-        {
-            return; // nothing to do
-        }
+    if (m_pTickerThread.is())
+    {
         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)
+    rGuard.clear();
+
+    if (pTickerThread.is())
+        pTickerThread->join(); // without m_aMutex locked (to prevent deadlock)
 }
 
 OUString SerfLockStore::getLockToken( const OUString& rLock )
@@ -214,12 +215,12 @@ void SerfLockStore::updateLock( const OUString& rLock,
 
 void SerfLockStore::removeLock( const OUString& rLock )
 {
-    osl::MutexGuard aGuard( m_aMutex );
+    osl::ClearableMutexGuard aGuard( m_aMutex );
 
     m_aLockInfoMap.erase( rLock );
 
     if ( m_aLockInfoMap.empty() )
-        stopTicker();
+        stopTicker(aGuard);
 }
 
 
diff --git a/ucb/source/ucp/webdav-curl/SerfLockStore.hxx 
b/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
index 62a859be639d..4aeb3a020dbc 100644
--- a/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
+++ b/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
@@ -89,7 +89,7 @@ public:
 
 private:
     void startTicker();
-    void stopTicker();
+    void stopTicker(osl::ClearableMutexGuard & rGuard);
 };
 
 } // namespace http_dav_ucp