Fix deadlock

...introduced with 73e062be1ff10e2dd889d9ec9c2d07b692077f62 "blind fix for some
7.4.3.2 crashes in UpdateCheckJob" (and wreaking havoc during
--enable-online-update `make check`)

Change-Id: Idde4e4fa8ab94dafa141eb740aba1d874de4717d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144320
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

1 files changed, 26 insertions, 16 deletions

diff --git a/extensions/source/update/check/updatecheckjob.cxx b/extensions/source/update/check/updatecheckjob.cxx
index bf30c1686842..b79c438108ee 100644
--- a/extensions/source/update/check/updatecheckjob.cxx
+++ b/extensions/source/update/check/updatecheckjob.cxx
@@ -96,8 +96,9 @@ public:
     virtual void SAL_CALL notifyTermination( lang::EventObject const & evt ) override;
 
 private:
-    std::mutex m_mutex;
     uno::Reference<uno::XComponentContext>  m_xContext;
+
+    std::mutex m_mutex;
     uno::Reference< frame::XDesktop2 >      m_xDesktop;
     std::unique_ptr< InitUpdateCheckJobThread > m_pInitThread;
 
@@ -175,7 +176,6 @@ UpdateCheckJob::~UpdateCheckJob()
 uno::Any
 UpdateCheckJob::execute(const uno::Sequence<beans::NamedValue>& namedValues)
 {
-    std::scoped_lock l(m_mutex);
     for ( sal_Int32 n=namedValues.getLength(); n-- > 0; )
     {
         if ( namedValues[ n ].Name == "DynamicData" )
@@ -207,10 +207,13 @@ UpdateCheckJob::execute(const uno::Sequence<beans::NamedValue>& namedValues)
 
     OUString aEventName = getValue< OUString > (aEnvironment, "EventName");
 
-    m_pInitThread.reset(
-        new InitUpdateCheckJobThread(
-            m_xContext, aConfig,
-            aEventName != "onFirstVisibleTask"));
+    auto thread = std::make_unique<InitUpdateCheckJobThread >(
+        m_xContext, aConfig,
+        aEventName != "onFirstVisibleTask");
+    {
+        std::scoped_lock l(m_mutex);
+        m_pInitThread = std::move(thread);
+    }
 
     return uno::Any();
 }
@@ -276,14 +279,18 @@ UpdateCheckJob::supportsService( OUString const & serviceName )
 // XEventListener
 void SAL_CALL UpdateCheckJob::disposing( lang::EventObject const & rEvt )
 {
-    std::scoped_lock l(m_mutex);
-    bool shutDown = ( rEvt.Source == m_xDesktop );
+    css::uno::Reference<css::frame::XDesktop2> desktop;
+    {
+        std::scoped_lock l(m_mutex);
+        if ( rEvt.Source == m_xDesktop ) {
+            std::swap(m_xDesktop, desktop);
+        }
+    }
 
-    if ( shutDown && m_xDesktop.is() )
+    if ( desktop.is() )
     {
         terminateAndJoinThread();
-        m_xDesktop->removeTerminateListener( this );
-        m_xDesktop.clear();
+        desktop->removeTerminateListener( this );
     }
 }
 
@@ -295,12 +302,15 @@ void SAL_CALL UpdateCheckJob::queryTermination( lang::EventObject const & )
 
 void UpdateCheckJob::terminateAndJoinThread()
 {
-    std::scoped_lock l(m_mutex);
-    if (m_pInitThread != nullptr)
+    std::unique_ptr<InitUpdateCheckJobThread> thread;
+    {
+        std::scoped_lock l(m_mutex);
+        std::swap(m_pInitThread, thread);
+    }
+    if (thread != nullptr)
     {
-        m_pInitThread->setTerminating();
-        m_pInitThread->join();
-        m_pInitThread.reset();
+        thread->setTerminating();
+        thread->join();
     }
 }