From cdae2f16ef9da75c1b3f9cc3f8abc3a180f324d1 Mon Sep 17 00:00:00 2001 From: Thorsten Behrens Date: Thu, 19 Nov 2020 14:00:57 +0100 Subject: fix tdf#138335 guard sidebar uno methods with SolarMutex Wasn't threadsafe before; using vcl/gui code, so we need the SolarMutex. Change-Id: I3d4407f095837d03ad492fcdf9a08746cf911d25 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106169 Tested-by: Jenkins Reviewed-by: Thorsten Behrens --- sfx2/source/sidebar/SidebarController.cxx | 23 ++++++++++++++++++++--- sfx2/source/sidebar/SidebarPanelBase.cxx | 14 ++++++++++++++ sfx2/source/sidebar/Theme.cxx | 20 ++++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) (limited to 'sfx2') diff --git a/sfx2/source/sidebar/SidebarController.cxx b/sfx2/source/sidebar/SidebarController.cxx index 4a0ff69f93ce..f257b5e815ca 100644 --- a/sfx2/source/sidebar/SidebarController.cxx +++ b/sfx2/source/sidebar/SidebarController.cxx @@ -252,6 +252,8 @@ void SidebarController::disposeDecks() void SAL_CALL SidebarController::disposing() { + SolarMutexGuard aSolarMutexGuard; + mpCloseIndicator.disposeAndClear(); maFocusManager.Clear(); @@ -307,6 +309,8 @@ void SAL_CALL SidebarController::disposing() void SAL_CALL SidebarController::notifyContextChangeEvent (const css::ui::ContextChangeEventObject& rEvent) { + SolarMutexGuard aSolarMutexGuard; + // Update to the requested new context asynchronously to avoid // subtle errors caused by SFX2 which in rare cases can not // properly handle a synchronous update. @@ -318,7 +322,9 @@ void SAL_CALL SidebarController::notifyContextChangeEvent (const css::ui::Contex if (maRequestedContext != maCurrentContext) { mxCurrentController.set(rEvent.Source, css::uno::UNO_QUERY); - maContextChangeUpdate.RequestCall(); + maContextChangeUpdate.RequestCall(); // async call, not a prob + // calling with held + // solarmutex // TODO: this call is redundant but mandatory for unit test to update context on document loading UpdateConfigurations(); } @@ -326,16 +332,24 @@ void SAL_CALL SidebarController::notifyContextChangeEvent (const css::ui::Contex void SAL_CALL SidebarController::disposing (const css::lang::EventObject& ) { + SolarMutexGuard aSolarMutexGuard; + dispose(); } void SAL_CALL SidebarController::propertyChange (const css::beans::PropertyChangeEvent& ) { - maPropertyChangeForwarder.RequestCall(); + SolarMutexGuard aSolarMutexGuard; + + maPropertyChangeForwarder.RequestCall(); // async call, not a prob + // to call with held + // solarmutex } void SAL_CALL SidebarController::statusChanged (const css::frame::FeatureStateEvent& rEvent) { + SolarMutexGuard aSolarMutexGuard; + bool bIsReadWrite (true); if (rEvent.IsEnabled) rEvent.State >>= bIsReadWrite; @@ -349,12 +363,15 @@ void SAL_CALL SidebarController::statusChanged (const css::frame::FeatureStateEv SwitchToDefaultDeck(); mnRequestedForceFlags |= SwitchFlag_ForceSwitch; - maContextChangeUpdate.RequestCall(); + maContextChangeUpdate.RequestCall(); // async call, ok to call + // with held solarmutex } } void SAL_CALL SidebarController::requestLayout() { + SolarMutexGuard aSolarMutexGuard; + sal_Int32 nMinimalWidth = 0; if (mpCurrentDeck && !mpCurrentDeck->isDisposed()) { diff --git a/sfx2/source/sidebar/SidebarPanelBase.cxx b/sfx2/source/sidebar/SidebarPanelBase.cxx index a1f0b8b6208c..6865047df252 100644 --- a/sfx2/source/sidebar/SidebarPanelBase.cxx +++ b/sfx2/source/sidebar/SidebarPanelBase.cxx @@ -78,6 +78,8 @@ SidebarPanelBase::~SidebarPanelBase() void SAL_CALL SidebarPanelBase::disposing() { + SolarMutexGuard aGuard; + mpControl.disposeAndClear(); if (mxFrame.is()) @@ -94,6 +96,8 @@ void SAL_CALL SidebarPanelBase::disposing() void SAL_CALL SidebarPanelBase::notifyContextChangeEvent ( const ui::ContextChangeEventObject& rEvent) { + SolarMutexGuard aGuard; + IContextChangeReceiver* pContextChangeReceiver = dynamic_cast(mpControl.get()); if (pContextChangeReceiver != nullptr) @@ -108,6 +112,8 @@ void SAL_CALL SidebarPanelBase::notifyContextChangeEvent ( void SAL_CALL SidebarPanelBase::disposing ( const css::lang::EventObject&) { + SolarMutexGuard aGuard; + mxFrame = nullptr; mpControl = nullptr; } @@ -141,6 +147,8 @@ Reference SAL_CALL SidebarPanelBase::createAccessibl Reference SAL_CALL SidebarPanelBase::getWindow() { + SolarMutexGuard aGuard; + if (mpControl != nullptr) return Reference( mpControl->GetComponentInterface(), @@ -151,6 +159,8 @@ Reference SAL_CALL SidebarPanelBase::getWindow() ui::LayoutSize SAL_CALL SidebarPanelBase::getHeightForWidth (const sal_Int32 nWidth) { + SolarMutexGuard aGuard; + if (maLayoutSize.Minimum >= 0) return maLayoutSize; else @@ -176,6 +186,8 @@ ui::LayoutSize SAL_CALL SidebarPanelBase::getHeightForWidth (const sal_Int32 nWi sal_Int32 SAL_CALL SidebarPanelBase::getMinimalWidth () { + SolarMutexGuard aGuard; + if (isLayoutEnabled(mpControl)) { // widget layout-based sidebar @@ -187,6 +199,8 @@ sal_Int32 SAL_CALL SidebarPanelBase::getMinimalWidth () void SAL_CALL SidebarPanelBase::updateModel(const css::uno::Reference& xModel) { + SolarMutexGuard aGuard; + SidebarModelUpdate* pModelUpdate = dynamic_cast(mpControl.get()); if (!pModelUpdate) return; diff --git a/sfx2/source/sidebar/Theme.cxx b/sfx2/source/sidebar/Theme.cxx index 361a77414a8e..66294a5cd9a3 100644 --- a/sfx2/source/sidebar/Theme.cxx +++ b/sfx2/source/sidebar/Theme.cxx @@ -256,6 +256,8 @@ void Theme::UpdateTheme() void SAL_CALL Theme::disposing() { + SolarMutexGuard aGuard; + ChangeListeners aListeners; aListeners.swap(maChangeListeners); @@ -290,6 +292,8 @@ void SAL_CALL Theme::setPropertyValue ( const OUString& rsPropertyName, const css::uno::Any& rValue) { + SolarMutexGuard aGuard; + PropertyNameToIdMap::const_iterator iId (maPropertyNameToIdMap.find(rsPropertyName)); if (iId == maPropertyNameToIdMap.end()) throw beans::UnknownPropertyException(rsPropertyName); @@ -332,6 +336,8 @@ void SAL_CALL Theme::setPropertyValue ( Any SAL_CALL Theme::getPropertyValue ( const OUString& rsPropertyName) { + SolarMutexGuard aGuard; + PropertyNameToIdMap::const_iterator iId (maPropertyNameToIdMap.find(rsPropertyName)); if (iId == maPropertyNameToIdMap.end()) throw beans::UnknownPropertyException(rsPropertyName); @@ -349,6 +355,8 @@ void SAL_CALL Theme::addPropertyChangeListener( const OUString& rsPropertyName, const css::uno::Reference& rxListener) { + SolarMutexGuard aGuard; + ThemeItem eItem (AnyItem_); if (rsPropertyName.getLength() > 0) { @@ -371,6 +379,8 @@ void SAL_CALL Theme::removePropertyChangeListener( const OUString& rsPropertyName, const css::uno::Reference& rxListener) { + SolarMutexGuard aGuard; + ThemeItem eItem (AnyItem_); if (rsPropertyName.getLength() > 0) { @@ -403,6 +413,8 @@ void SAL_CALL Theme::addVetoableChangeListener( const OUString& rsPropertyName, const css::uno::Reference& rxListener) { + SolarMutexGuard aGuard; + ThemeItem eItem (AnyItem_); if (rsPropertyName.getLength() > 0) { @@ -425,6 +437,8 @@ void SAL_CALL Theme::removeVetoableChangeListener( const OUString& rsPropertyName, const css::uno::Reference& rxListener) { + SolarMutexGuard aGuard; + ThemeItem eItem (AnyItem_); if (rsPropertyName.getLength() > 0) { @@ -454,6 +468,8 @@ void SAL_CALL Theme::removeVetoableChangeListener( css::uno::Sequence SAL_CALL Theme::getProperties() { + SolarMutexGuard aGuard; + ::std::vector aProperties; sal_Int32 const nEnd(End_); @@ -479,6 +495,8 @@ css::uno::Sequence SAL_CALL Theme::getProperties() beans::Property SAL_CALL Theme::getPropertyByName (const OUString& rsPropertyName) { + SolarMutexGuard aGuard; + PropertyNameToIdMap::const_iterator iId (maPropertyNameToIdMap.find(rsPropertyName)); if (iId == maPropertyNameToIdMap.end()) throw beans::UnknownPropertyException(rsPropertyName); @@ -498,6 +516,8 @@ beans::Property SAL_CALL Theme::getPropertyByName (const OUString& rsPropertyNam sal_Bool SAL_CALL Theme::hasPropertyByName (const OUString& rsPropertyName) { + SolarMutexGuard aGuard; + PropertyNameToIdMap::const_iterator iId (maPropertyNameToIdMap.find(rsPropertyName)); if (iId == maPropertyNameToIdMap.end()) return false; -- cgit