From 162ef484df31829361950bb6cd19f6e7f010b54f Mon Sep 17 00:00:00 2001 From: Michael Weghorn Date: Mon, 21 Oct 2024 09:01:56 +0200 Subject: tdf#161625 Use SolarMutex instead of std::mutex in ExtMgrDialog Instead of using a custom (non-recursive) std::mutex in the extension manager dialog, hold the (recursive) SolarMutex instead. As the backtrace in attachment 197155 in tdf#161625 (s.a. below) shows, a recursive mutex is needed: dp_gui::ExtMgrDialog::TimeOutHdl (frame 17) locks the mutex, then dp_gui::ExtMgrDialog::startProgress (frame 6) wants to lock it again, causing a deadlock. (Switching ExtMgrDialog::m_aMutex to be a std::recursive_mutex could be an alternative, but follow the common pattern of holding the SolarMutex while doing UI stuff instead.) Somewhat similar commit: commit 406a7e9d452201f3fd53abc770da6eb9589fff92 Date: Wed Jul 10 12:46:50 2024 +0200 fix locking in UpdateRequiredDialog Backtrace of deadlock: #0 0x00007f883f6adc70 in ?? () from /usr/lib/libc.so.6 #1 0x00007f883f6b4b01 in pthread_mutex_lock () from /usr/lib/libc.so.6 #2 0x00007f87f3ea068e in __gthread_mutex_lock (__mutex=0x5782aab26e48) at /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/x86_64-pc-linux-gnu/bits/gthr-default.h:762 #3 std::mutex::lock (this=0x5782aab26e48) at /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/std_mutex.h:113 #4 std::unique_lock::lock (this=) at /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/unique_lock.h:147 #5 std::unique_lock::unique_lock (__m=..., this=) at /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/unique_lock.h:73 #6 dp_gui::ExtMgrDialog::startProgress (this=0x5782aab26de0, _bLockInterface=0x80) at /home/user/libreofficetwo/desktop/source/deployment/gui/dp_gui_dialog2.cxx:779 #7 0x00007f883b4080a1 in Link::Call (this=0x7f86b0000c08, data=0x80) at include/tools/link.hxx:111 #8 ImplHandleUserEvent (pSVEvent=pSVEvent@entry=0x7f86b0000c00) at /home/user/libreofficetwo/vcl/source/window/winproc.cxx:2285 #9 0x00007f883b40616a in ImplWindowFrameProc (_pWindow=0x5782a76df9f0, nEvent=SalEvent::UserEvent, pEvent=0x7f86b0000c00) at /home/user/libreofficetwo/vcl/source/window/winproc.cxx:2849 #10 0x00007f883b6c711e in SalUserEventList::DispatchUserEvents(bool)::$_0::operator()() const (this=) at /home/user/libreofficetwo/vcl/source/app/salusereventlist.cxx:119 #11 SalUserEventList::DispatchUserEvents (this=0x5782a62edb08, bHandleAllCurrentEvents=false) at /home/user/libreofficetwo/vcl/source/app/salusereventlist.cxx:120 #12 0x00007f8833fb7827 in QtInstance::ImplYield (this=this@entry=0x5782a62edad0, bWait=true, bHandleAllCurrentEvents=false) at vcl/qt6/../qt5/QtInstance.cxx:447 #13 0x00007f8833fb9e11 in QtInstance::DoYield (this=0x5782a62edad0, bWait=true, bHandleAllCurrentEvents=false) at vcl/qt6/../qt5/QtInstance.cxx:469 #14 0x00007f883b70fc72 in ImplYield (i_bWait=true, i_bAllEvents=false) at /home/user/libreofficetwo/vcl/source/app/svapp.cxx:385 #15 Application::Yield () at /home/user/libreofficetwo/vcl/source/app/svapp.cxx:473 #16 0x00007f883b486415 in ProgressBar::SetValue (this=, nNewPercent=) at /home/user/libreofficetwo/vcl/source/control/prgsbar.cxx:199 #17 0x00007f87f3ea14ca in dp_gui::ExtMgrDialog::TimeOutHdl (this=0x5782aab26de0) at /home/user/libreofficetwo/desktop/source/deployment/gui/dp_gui_dialog2.cxx:976 #18 0x00007f883b6ff8c7 in Scheduler::CallbackTaskScheduling () at /home/user/libreofficetwo/vcl/source/app/scheduler.cxx:509 #19 0x00007f8833fd4013 in SalTimer::CallCallback (this=0x5782a77b83d0) at vcl/inc/saltimer.hxx:53 #20 QtTimer::timeoutActivated (this=0x5782a77b83c0) at vcl/qt6/../qt5/QtTimer.cxx:51 #21 0x00007f88341a3397 in ?? () from /usr/lib/libQt6Core.so.6 #22 0x00007f88341ab5e5 in QTimer::timerEvent(QTimerEvent*) () from /usr/lib/libQt6Core.so.6 #23 0x00007f883418d859 in QObject::event(QEvent*) () from /usr/lib/libQt6Core.so.6 #24 0x00007f8832efc8cc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt6Widgets.so.6 #25 0x00007f8834145aa8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt6Core.so.6 #26 0x00007f88342c7658 in QTimerInfoList::activateTimers() () from /usr/lib/libQt6Core.so.6 #27 0x00007f88343a9f99 in ?? () from /usr/lib/libQt6Core.so.6 #28 0x00007f8837877299 in ?? () from /usr/lib/libglib-2.0.so.0 #29 0x00007f88378d9ec7 in ?? () from /usr/lib/libglib-2.0.so.0 #30 0x00007f8837876795 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0 #31 0x00007f88343a82bd in QEventDispatcherGlib::processEvents(QFlags) () from /usr/lib/libQt6Core.so.6 #32 0x00007f8833fb789d in QtInstance::ImplYield (this=this@entry=0x5782a62edad0, bWait=, bHandleAllCurrentEvents=) at vcl/qt6/../qt5/QtInstance.cxx:458 #33 0x00007f8833fb9e11 in QtInstance::DoYield (this=0x5782a62edad0, bWait=true, bHandleAllCurrentEvents=false) at vcl/qt6/../qt5/QtInstance.cxx:469 #34 0x00007f883b70fc72 in ImplYield (i_bWait=true, i_bAllEvents=false) at /home/user/libreofficetwo/vcl/source/app/svapp.cxx:385 #35 Application::Yield () at /home/user/libreofficetwo/vcl/source/app/svapp.cxx:473 #36 0x00007f883b70fb90 in Application::Execute () at /home/user/libreofficetwo/vcl/source/app/svapp.cxx:360 #37 0x00007f883f8e1770 in desktop::Desktop::Main (this=0x7ffc2a5b0d28) at /home/user/libreofficetwo/desktop/source/app/app.cxx:1691 #38 0x00007f883b717e1e in ImplSVMain () at /home/user/libreofficetwo/vcl/source/app/svmain.cxx:228 #39 0x00007f883f90ef8a in soffice_main () at /home/user/libreofficetwo/desktop/source/app/sofficemain.cxx:121 #40 0x000057829d6f683b in sal_main () at /home/user/libreofficetwo/desktop/source/app/main.c:51 #41 main (argc=, argv=) at /home/user/libreofficetwo/desktop/source/app/main.c:49 Change-Id: I96d746eb1493aaf5b56d50664c9d1817699f21bb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175298 Reviewed-by: Michael Weghorn Reviewed-by: Noel Grandin Tested-by: Jenkins Reviewed-by: Rafael Lima Tested-by: Ilmari Lauhakangas Reviewed-by: Ilmari Lauhakangas --- desktop/source/deployment/gui/dp_gui_dialog2.cxx | 10 +++++----- desktop/source/deployment/gui/dp_gui_dialog2.hxx | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) (limited to 'desktop/source') diff --git a/desktop/source/deployment/gui/dp_gui_dialog2.cxx b/desktop/source/deployment/gui/dp_gui_dialog2.cxx index 2d978ce080f0..cf53dddff48a 100644 --- a/desktop/source/deployment/gui/dp_gui_dialog2.cxx +++ b/desktop/source/deployment/gui/dp_gui_dialog2.cxx @@ -776,7 +776,7 @@ IMPL_LINK_NOARG(ExtMgrDialog, HandleCloseBtn, weld::Button&, void) IMPL_LINK( ExtMgrDialog, startProgress, void*, _bLockInterface, void ) { - std::unique_lock aGuard( m_aMutex ); + SolarMutexGuard aGuard; bool bLockInterface = static_cast(_bLockInterface); if ( m_bStartProgress && !m_bHasProgress ) @@ -815,7 +815,7 @@ IMPL_LINK( ExtMgrDialog, startProgress, void*, _bLockInterface, void ) void ExtMgrDialog::showProgress( bool _bStart ) { - std::unique_lock aGuard( m_aMutex ); + SolarMutexGuard aGuard; bool bStart = _bStart; @@ -839,7 +839,7 @@ void ExtMgrDialog::showProgress( bool _bStart ) void ExtMgrDialog::updateProgress( const tools::Long nProgress ) { - std::unique_lock aGuard( m_aMutex ); + SolarMutexGuard aGuard; if ( m_nProgress != nProgress ) { m_nProgress = nProgress; @@ -851,7 +851,7 @@ void ExtMgrDialog::updateProgress( const tools::Long nProgress ) void ExtMgrDialog::updateProgress( const OUString &rText, const uno::Reference< task::XAbortChannel > &xAbortChannel) { - std::unique_lock aGuard( m_aMutex ); + SolarMutexGuard aGuard; m_xAbortChannel = xAbortChannel; m_sProgressText = rText; @@ -945,7 +945,7 @@ IMPL_LINK_NOARG(ExtMgrDialog, HandleUpdateBtn, weld::Button&, void) IMPL_LINK_NOARG(ExtMgrDialog, TimeOutHdl, Timer *, void) { - std::unique_lock aGuard( m_aMutex ); + SolarMutexGuard aGuard; if ( m_bStopProgress ) { m_bHasProgress = false; diff --git a/desktop/source/deployment/gui/dp_gui_dialog2.hxx b/desktop/source/deployment/gui/dp_gui_dialog2.hxx index e7910535cad3..291b5c80981e 100644 --- a/desktop/source/deployment/gui/dp_gui_dialog2.hxx +++ b/desktop/source/deployment/gui/dp_gui_dialog2.hxx @@ -92,7 +92,6 @@ class ExtMgrDialog : public weld::GenericDialogController { const OUString m_sAddPackages; OUString m_sProgressText; - std::mutex m_aMutex; bool m_bHasProgress; bool m_bProgressChanged; bool m_bStartProgress; -- cgit