desktop/source/deployment/gui/dp_gui_dialog2.cxx | 9 +++++----
desktop/source/deployment/gui/dp_gui_dialog2.hxx | 1 -
2 files changed, 5 insertions(+), 5 deletions(-)
New commits:
commit 7266e872863625f32270f75080703ff923dd242e
Author: Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Mon Oct 21 09:01:56 2024 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Tue Oct 22 15:28:58 2024 +0200
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<std::mutex>::lock (this=<optimized out>) 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<std::mutex>::unique_lock (__m=..., this=<optimized
out>) 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<void*, void>::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=<optimized out>) 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=<optimized out>,
nNewPercent=<optimized out>) 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<QEventLoop::ProcessEventsFlag>) ()
from /usr/lib/libQt6Core.so.6
#32 0x00007f8833fb789d in QtInstance::ImplYield
(this=this@entry=0x5782a62edad0, bWait=<optimized out>,
bHandleAllCurrentEvents=<optimized out>) 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=<optimized out>, argv=<optimized out>) 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 <m.wegh...@posteo.de>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Rafael Lima <rafael.palma.l...@gmail.com>
Tested-by: Ilmari Lauhakangas <ilmari.lauhakan...@libreoffice.org>
Reviewed-by: Ilmari Lauhakangas <ilmari.lauhakan...@libreoffice.org>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175402
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<bool>(_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;
commit 09cc2c84bcfa1c48cafb35353008a014952b0908
Author: Noel Grandin <noelgran...@gmail.com>
AuthorDate: Sun Oct 6 21:38:10 2024 +0200
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Tue Oct 22 15:28:46 2024 +0200
cid#1606611 Data race condition
and
cid#1607663 Data race condition
cid#1606843 Data race condition
cid#1608340 Check of thread-shared field evades lock acquisition
Change-Id: I97b82d4302ead6b96ae19c15502c427952df2ede
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174566
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175401
diff --git a/desktop/source/deployment/gui/dp_gui_dialog2.cxx
b/desktop/source/deployment/gui/dp_gui_dialog2.cxx
index 4fe7a9622545..2d978ce080f0 100644
--- a/desktop/source/deployment/gui/dp_gui_dialog2.cxx
+++ b/desktop/source/deployment/gui/dp_gui_dialog2.cxx
@@ -839,9 +839,9 @@ void ExtMgrDialog::showProgress( bool _bStart )
void ExtMgrDialog::updateProgress( const tools::Long nProgress )
{
+ std::unique_lock aGuard( m_aMutex );
if ( m_nProgress != nProgress )
{
- std::unique_lock aGuard( m_aMutex );
m_nProgress = nProgress;
m_aIdle.Start();
}
@@ -945,6 +945,7 @@ IMPL_LINK_NOARG(ExtMgrDialog, HandleUpdateBtn,
weld::Button&, void)
IMPL_LINK_NOARG(ExtMgrDialog, TimeOutHdl, Timer *, void)
{
+ std::unique_lock aGuard( m_aMutex );
if ( m_bStopProgress )
{
m_bHasProgress = false;
