extensions/source/update/check/updatecheckjob.cxx | 38 +++++++++++++++------- 1 file changed, 26 insertions(+), 12 deletions(-)
New commits: commit 95973a19a3f75d52d224e48e75717bf2ede3e3e5 Author: Stephan Bergmann <[email protected]> AuthorDate: Fri Dec 16 12:19:31 2022 +0100 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Dec 19 09:06:44 2022 +0000 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 <[email protected]> Tested-by: Jenkins Reviewed-by: Stephan Bergmann <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144348 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(); } } commit 2bd1c84e8cd455bdae7ba8d160bf5805de3fa0be Author: Noel Grandin <[email protected]> AuthorDate: Thu Dec 15 19:28:12 2022 +0200 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Dec 19 09:06:37 2022 +0000 blind fix for some 7.4.3.2 crashes in UpdateCheckJob I am guessing that state in this object is being touched from multiple threads (since it is an UNO service object), and one or more of the threads are consequently seeing inconsistent state. Change-Id: Ib925740d3b8db2713c4f132d0367b794a412a269 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144248 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144347 diff --git a/extensions/source/update/check/updatecheckjob.cxx b/extensions/source/update/check/updatecheckjob.cxx index 7fabb98bff4c..bf30c1686842 100644 --- a/extensions/source/update/check/updatecheckjob.cxx +++ b/extensions/source/update/check/updatecheckjob.cxx @@ -96,6 +96,7 @@ public: virtual void SAL_CALL notifyTermination( lang::EventObject const & evt ) override; private: + std::mutex m_mutex; uno::Reference<uno::XComponentContext> m_xContext; uno::Reference< frame::XDesktop2 > m_xDesktop; std::unique_ptr< InitUpdateCheckJobThread > m_pInitThread; @@ -174,6 +175,7 @@ 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" ) @@ -274,6 +276,7 @@ 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 ); if ( shutDown && m_xDesktop.is() ) @@ -292,6 +295,7 @@ void SAL_CALL UpdateCheckJob::queryTermination( lang::EventObject const & ) void UpdateCheckJob::terminateAndJoinThread() { + std::scoped_lock l(m_mutex); if (m_pInitThread != nullptr) { m_pInitThread->setTerminating();
