Skip to content

Commit

Permalink
fix locking in UpdateRequiredDialog
Browse files Browse the repository at this point in the history
Ever since
    commit 838036c
    Author: Noel Grandin <[email protected]>
    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 <[email protected]>
Reviewed-by: Caolán McNamara <[email protected]>
Tested-by: Jenkins
  • Loading branch information
Noel Grandin authored and caolanm committed Jul 11, 2024
1 parent f03610e commit 406a7e9
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 18 deletions.
32 changes: 15 additions & 17 deletions desktop/source/deployment/gui/dp_gui_dialog2.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -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;

Expand All @@ -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();
}
Expand All @@ -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;
Expand Down Expand Up @@ -1171,26 +1171,26 @@ 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) );
}


IMPL_LINK_NOARG(UpdateRequiredDialog, HandleCloseBtn, weld::Button&, void)
{
std::unique_lock aGuard( m_aMutex );
SolarMutexGuard aGuard;

if ( !isBusy() )
{
Expand Down Expand Up @@ -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++ )
Expand All @@ -1322,7 +1320,7 @@ bool UpdateRequiredDialog::hasActiveEntries()

void UpdateRequiredDialog::disableAllEntries()
{
std::unique_lock aGuard( m_aMutex );
SolarMutexGuard aGuard;

incBusy();

Expand Down
1 change: 0 additions & 1 deletion desktop/source/deployment/gui/dp_gui_dialog2.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 406a7e9

Please sign in to comment.