desktop/source/deployment/gui/dp_gui_dialog2.cxx | 32 ++++++++++-------------
desktop/source/deployment/gui/dp_gui_dialog2.hxx | 1
2 files changed, 15 insertions(+), 18 deletions(-)
New commits:
commit 406a7e9d452201f3fd53abc770da6eb9589fff92
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Wed Jul 10 12:46:50 2024 +0200
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Jul 11 20:56:03 2024 +0200
fix locking in UpdateRequiredDialog
Ever since
commit 838036c304d474fc4c19e2fc59cadc6ba457c9ee
Author: Noel Grandin <noel.gran...@collabora.co.uk>
Date: Tue Mar 7 13:46:29 2023 +0200
osl::Mutex->std::mutex in UpdateRequiredDialog
Calling from disableAllEntries, which takes a mutex, to
hasActiveEntries, would deadlock.
Noticing that this class is trying to use a mix of the SolarMutex and
its own mutex, rather just use the SolarMutex everywhere, which is much
safer, and not a problem in a place where performance is not an issue
Change-Id: Id241c3b811f314d75de03c4c647c0594b8d498bb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170283
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Tested-by: Jenkins
diff --git a/desktop/source/deployment/gui/dp_gui_dialog2.cxx
b/desktop/source/deployment/gui/dp_gui_dialog2.cxx
index 99e1ccd489c2..4fe7a9622545 100644
--- a/desktop/source/deployment/gui/dp_gui_dialog2.cxx
+++ b/desktop/source/deployment/gui/dp_gui_dialog2.cxx
@@ -1079,7 +1079,7 @@ IMPL_LINK_NOARG(UpdateRequiredDialog, HandleCancelBtn,
weld::Button&, void)
IMPL_LINK( UpdateRequiredDialog, startProgress, void*, _bLockInterface, void )
{
- std::unique_lock aGuard( m_aMutex );
+ SolarMutexGuard aGuard;
bool bLockInterface = static_cast<bool>(_bLockInterface);
if ( m_bStartProgress && !m_bHasProgress )
@@ -1105,7 +1105,7 @@ IMPL_LINK( UpdateRequiredDialog, startProgress, void*,
_bLockInterface, void )
void UpdateRequiredDialog::showProgress( bool _bStart )
{
- std::unique_lock aGuard( m_aMutex );
+ SolarMutexGuard aGuard;
bool bStart = _bStart;
@@ -1129,9 +1129,9 @@ void UpdateRequiredDialog::showProgress( bool _bStart )
void UpdateRequiredDialog::updateProgress( const tools::Long nProgress )
{
+ SolarMutexGuard aGuard;
if ( m_nProgress != nProgress )
{
- std::unique_lock aGuard( m_aMutex );
m_nProgress = nProgress;
m_aIdle.Start();
}
@@ -1141,7 +1141,7 @@ void UpdateRequiredDialog::updateProgress( const
tools::Long nProgress )
void UpdateRequiredDialog::updateProgress( const OUString &rText,
const uno::Reference<
task::XAbortChannel > &xAbortChannel)
{
- std::unique_lock aGuard( m_aMutex );
+ SolarMutexGuard aGuard;
m_xAbortChannel = xAbortChannel;
m_sProgressText = rText;
@@ -1171,18 +1171,18 @@ void UpdateRequiredDialog::updatePackageInfo( const
uno::Reference< deployment::
IMPL_LINK_NOARG(UpdateRequiredDialog, HandleUpdateBtn, weld::Button&, void)
{
- std::unique_lock aGuard( m_aMutex );
-
std::vector< uno::Reference< deployment::XPackage > > vUpdateEntries;
- sal_Int32 nCount = m_xExtensionBox->GetEntryCount();
-
- for ( sal_Int32 i = 0; i < nCount; ++i )
{
- TEntry_Impl pEntry = m_xExtensionBox->GetEntryData( i );
- vUpdateEntries.push_back( pEntry->m_xPackage );
- }
+ SolarMutexGuard aGuard;
- aGuard.unlock();
+ sal_Int32 nCount = m_xExtensionBox->GetEntryCount();
+
+ for ( sal_Int32 i = 0; i < nCount; ++i )
+ {
+ TEntry_Impl pEntry = m_xExtensionBox->GetEntryData( i );
+ vUpdateEntries.push_back( pEntry->m_xPackage );
+ }
+ }
m_pManager->getCmdQueue()->checkForUpdates( std::move(vUpdateEntries) );
}
@@ -1190,7 +1190,7 @@ IMPL_LINK_NOARG(UpdateRequiredDialog, HandleUpdateBtn,
weld::Button&, void)
IMPL_LINK_NOARG(UpdateRequiredDialog, HandleCloseBtn, weld::Button&, void)
{
- std::unique_lock aGuard( m_aMutex );
+ SolarMutexGuard aGuard;
if ( !isBusy() )
{
@@ -1301,8 +1301,6 @@ bool UpdateRequiredDialog::checkDependencies( const
uno::Reference< deployment::
bool UpdateRequiredDialog::hasActiveEntries()
{
- std::unique_lock aGuard( m_aMutex );
-
bool bRet = false;
tools::Long nCount = m_xExtensionBox->GetEntryCount();
for ( tools::Long nIndex = 0; nIndex < nCount; nIndex++ )
@@ -1322,7 +1320,7 @@ bool UpdateRequiredDialog::hasActiveEntries()
void UpdateRequiredDialog::disableAllEntries()
{
- std::unique_lock aGuard( m_aMutex );
+ SolarMutexGuard aGuard;
incBusy();
diff --git a/desktop/source/deployment/gui/dp_gui_dialog2.hxx
b/desktop/source/deployment/gui/dp_gui_dialog2.hxx
index b235aed18589..e7910535cad3 100644
--- a/desktop/source/deployment/gui/dp_gui_dialog2.hxx
+++ b/desktop/source/deployment/gui/dp_gui_dialog2.hxx
@@ -184,7 +184,6 @@ class UpdateRequiredDialog : public
weld::GenericDialogController
{
const OUString m_sCloseText;
OUString m_sProgressText;
- std::mutex m_aMutex;
bool m_bHasProgress;
bool m_bProgressChanged;
bool m_bStartProgress;
