tdf#170961: Don't hold mutex when calling component's setPropertyValue

A deadlock seen with the following call stacks.

Main thread is trying to paint in a timer event, and is waiting for
the FormComponentPropertyHandler's mutex:

  ntdll.dll!NtWaitForAlertByThreadId()
  ntdll.dll!RtlpWaitOnCriticalSection()
  ntdll.dll!RtlpEnterCriticalSectionContended()
  ntdll.dll!RtlEnterCriticalSection()
  sal3.dll!osl_acquireMutex(_oslMutexImpl * Mutex) Line 65
  pcrlo.dll!osl::Mutex::acquire() Line 63
  pcrlo.dll!osl::Guard<osl::Mutex>::Guard<osl::Mutex>(osl::Mutex & t) Line 144
• pcrlo.dll!pcr::FormComponentPropertyHandler::getPropertyValue(const rtl::OUString & _rPropertyName) Line 307
  pcrlo.dll!pcr::OPropertyBrowserController::impl_getPropertyValue_throw(const rtl::OUString & _rPropertyName) Line 907
  pcrlo.dll!pcr::OPropertyBrowserController::propertyChange(const com::sun::star::beans::PropertyChangeEvent & _rEvent) Line 661
  cppuhelper3MSC.dll!cppu::OPropertySetHelper::fire(long * pnHandles, const com::sun::star::uno::Any * pNewValues, const com::sun::star::uno::Any * pOldValues, long nHandles, unsigned char bVetoable) Line 733
  comphelper.dll!comphelper::OPropertySetAggregationHelper::propertiesChange(const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyChangeEvent> & _rEvents) Line 420
  cppuhelper3MSC.dll!cppu::OPropertySetHelper::firePropertiesChangeEvent(const com::sun::star::uno::Sequence<rtl::OUString> & rPropertyNames, const com::sun::star::uno::Reference<com::sun::star::beans::XPropertiesChangeListener> & rListener) Line 961
  tklo.dll!ControlContainerBase::ImplUpdateResourceResolver() Line 1698
  tklo.dll!ControlContainerBase::ImplStartListingForResourceEvents() Line 1675
  tklo.dll!ControlContainerBase::setModel(const com::sun::star::uno::Reference<com::sun::star::awt::XControlModel> & rxModel) Line 1428
  tklo.dll!UnoDialogControl::setModel(const com::sun::star::uno::Reference<com::sun::star::awt::XControlModel> & rxModel) Line 348
  svxcorelo.dll!sdr::contact::`anonymous namespace'::ControlHolder::setModel(const com::sun::star::uno::Reference<com::sun::star::awt::XControlModel> & _m) Line 186
  svxcorelo.dll!sdr::contact::ViewObjectContactOfUnoControl_Impl::createControlForDevice(const sdr::contact::`anonymous-namespace'::IPageViewAccess & _rPageView, const OutputDevice & _rDevice, const SdrUnoObj & _rUnoObject, const basegfx::B2DHomMatrix & _rInitialViewTransformation, const basegfx::B2DHomMatrix & _rInitialZoomNormalization, sdr::contact::`anonymous-namespace'::ControlHolder & _out_rControl) Line 1097
  svxcorelo.dll!sdr::contact::ViewObjectContactOfUnoControl_Impl::impl_ensureControl_nothrow(const sdr::contact::`anonymous-namespace'::IPageViewAccess & _rPageView, const OutputDevice & _rDevice, const basegfx::B2DHomMatrix & _rInitialViewTransformation) Line 1037
  svxcorelo.dll!sdr::contact::ViewObjectContactOfUnoControl_Impl::ensureControl(const basegfx::B2DHomMatrix * _pInitialViewTransformationOrNULL) Line 963
  svxcorelo.dll!sdr::contact::`anonymous namespace'::LazyControlCreationPrimitive2D::create2DDecomposition(const drawinglayer::geometry::ViewInformation2D & _rViewInformation) Line 1539
  drawinglayercorelo.dll!drawinglayer::primitive2d::BufferedDecompositionPrimitive2D::get2DDecomposition(drawinglayer::primitive2d::Primitive2DDecompositionVisitor & rVisitor, const drawinglayer::geometry::ViewInformation2D & rViewInformation) Line 79
  svxcorelo.dll!sdr::contact::`anonymous namespace'::LazyControlCreationPrimitive2D::get2DDecomposition(drawinglayer::primitive2d::Primitive2DDecompositionVisitor & rVisitor, const drawinglayer::geometry::ViewInformation2D & _rViewInformation) Line 1523
  drawinglayerlo.dll!drawinglayer::processor2d::BaseProcessor2D::process(const drawinglayer::primitive2d::BasePrimitive2D & rCandidate) Line 43
  drawinglayerlo.dll!drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(const drawinglayer::primitive2d::BasePrimitive2D & rCandidate) Line 374
  drawinglayerlo.dll!drawinglayer::processor2d::BaseProcessor2D::process(const drawinglayer::primitive2d::Primitive2DContainer & rSource) Line 67
  svxcorelo.dll!sdr::contact::ObjectContactOfPageView::DoProcessDisplay(sdr::contact::DisplayInfo & rDisplayInfo) Line 303
  svxcorelo.dll!sdr::contact::ObjectContactOfPageView::ProcessDisplay(sdr::contact::DisplayInfo & rDisplayInfo) Line 128
  svxcorelo.dll!SdrPageWindow::RedrawLayer(const o3tl::strong_int<short,SdrLayerIDTag> * pId, sdr::contact::ViewObjectContactRedirector * pRedirector, const basegfx::B2IRange * pPageFrame) Line 422
  svxcorelo.dll!SdrPageView::DrawLayer(o3tl::strong_int<short,SdrLayerIDTag> nID, OutputDevice * pGivenTarget, sdr::contact::ViewObjectContactRedirector * pRedirector, const tools::Rectangle & rRect, const basegfx::B2IRange * pPageFrame) Line 304
  svxcorelo.dll!SdrPaintView::ImpFormLayerDrawing(SdrPaintWindow & rPaintWindow, sdr::contact::ViewObjectContactRedirector * pRedirector) Line 798
  svxcorelo.dll!SdrPaintView::EndCompleteRedraw(SdrPaintWindow & rPaintWindow, bool bPaintFormLayer, sdr::contact::ViewObjectContactRedirector * pRedirector) Line 653
  svxcorelo.dll!SdrPaintView::EndDrawLayers(SdrPaintWindow & rPaintWindow, bool bPaintFormLayer, sdr::contact::ViewObjectContactRedirector * pRedirector) Line 726
  basctllo.dll!basctl::DlgEditor::Paint(OutputDevice & rRenderContext, const tools::Rectangle & rRect) Line 547
  basctllo.dll!basctl::DialogWindow::Paint(OutputDevice & rRenderContext, const tools::Rectangle & rRect) Line 124
  vcllo.dll!PaintHelper::DoPaint(const vcl::Region * pRegion) Line 316
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 618
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!PaintHelper::~PaintHelper() Line 553
  vcllo.dll!vcl::Window::ImplCallPaint(const vcl::Region * pRegion, ImplPaintFlags nPaintFlags) Line 624
  vcllo.dll!vcl::Window::ImplCallOverlapPaint() Line 645
  vcllo.dll!vcl::Window::ImplHandlePaintHdl(Timer * __formal) Line 668
  vcllo.dll!vcl::Window::LinkStubImplHandlePaintHdl(void * instance, Timer * data) Line 649
  vcllo.dll!Link<Timer *,void>::Call(Timer * data) Line 105
  vcllo.dll!Timer::Invoke() Line 75
  vcllo.dll!Scheduler::CallbackTaskScheduling() Line 615
  vcllo.dll!SalTimer::CallCallback() Line 53
  vclplug_winlo.dll!WinSalTimer::ImplHandleElapsedTimer() Line 169
  vclplug_winlo.dll!ImplSalYield(bool bWait, bool bHandleAllCurrentEvents) Line 443
  vclplug_winlo.dll!WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents) Line 499
  vcllo.dll!InnerYield(bool i_bWait, bool i_bAllEvents) Line 389
  vcllo.dll!Application::Yield() Line 502
  vcllo.dll!Application::Execute() Line 365
  sofficeapp.dll!desktop::Desktop::Main() Line 1681
  vcllo.dll!ImplSVMain() Line 230
  vcllo.dll!SVMain() Line 249
  sofficeapp.dll!soffice_main() Line 122
  soffice.bin!sal_main() Line 51
  soffice.bin!main(int argc, char * * argv) Line 49
  soffice.bin!invoke_main() Line 79
  soffice.bin!__scrt_common_main_seh() Line 288
  soffice.bin!__scrt_common_main() Line 331
  soffice.bin!mainCRTStartup(void * __formal) Line 17
  kernel32.dll!BaseThreadInitThunk()
  ntdll.dll!RtlUserThreadStart()

while the worker thread is holding the mutex and is sending a message
to the main thread synchronously:

  win32u.dll!NtUserMessageCall()
  user32.dll!SendMessageWorker()
  user32.dll!SendMessageInternal(struct HWND__ *,unsigned int,unsigned __int64,__int64,int)
  user32.dll!SendMessageW()
  vclplug_winlo.dll!WinSalInstance::SendWndMessage_impl(HWND__ * hWnd, unsigned int Msg, unsigned __int64 wParam, __int64 lParam) Line 689
  vclplug_winlo.dll!WinSalInstance::SendComWndMessage(unsigned int Msg, unsigned __int64 wParam, __int64 lParam) Line 700
  vclplug_winlo.dll!WinSalFrame::ReleaseGraphics(SalGraphics * pGraphics) Line 1052
  vcllo.dll!vcl::WindowOutputDevice::ReleaseGraphics(bool bRelease) Line 905
  vcllo.dll!vcl::Window::dispose() Line 487
  vcllo.dll!ImplBorderWindow::dispose() Line 1614
  vcllo.dll!VclReferenceBase::disposeOnce() Line 38
  vcllo.dll!VclPtr<ImplBorderWindow>::disposeAndClear() Line 224
  vcllo.dll!ScopedVclPtr<ImplBorderWindow>::~ScopedVclPtr<ImplBorderWindow>() Line 351
  vcllo.dll!ScopedVclPtrInstance<ImplBorderWindow>::~ScopedVclPtrInstance<ImplBorderWindow>()
  vcllo.dll!Dialog::GetDrawWindowBorder(long & rLeftBorder, long & rTopBorder, long & rRightBorder, long & rBottomBorder) Line 1389
  tklo.dll!VCLXDialog::getInfo() Line 2302
  basctllo.dll!basctl::DlgEdForm::getDeviceInfo() Line 1642
  basctllo.dll!basctl::DlgEdObj::TransformFormToSdrCoordinates(long nXIn, long nYIn, long nWidthIn, long nHeightIn, long & nXOut, long & nYOut, long & nWidthOut, long & nHeightOut) Line 351
  basctllo.dll!basctl::DlgEditor::AdjustPageSize() Line 1182
  basctllo.dll!basctl::DlgEditor::SetDialog(const com::sun::star::uno::Reference<com::sun::star::container::XNameContainer> & xUnoControlDialogModel) Line 351
  basctllo.dll!basctl::DlgEditor::ResetDialog() Line 409
  basctllo.dll!basctl::DlgEdObj::_propertyChange(const com::sun::star::beans::PropertyChangeEvent & evt) Line 1147
  basctllo.dll!basctl::DlgEdPropListenerImpl::propertyChange(const com::sun::star::beans::PropertyChangeEvent & evt) Line 44
  cppuhelper3MSC.dll!cppu::OPropertySetHelper::fire(long * pnHandles, const com::sun::star::uno::Any * pNewValues, const com::sun::star::uno::Any * pOldValues, long nHandles, unsigned char bVetoable) Line 733
  comphelper.dll!comphelper::OPropertySetAggregationHelper::propertiesChange(const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyChangeEvent> & _rEvents) Line 398
  comphelper.dll!comphelper::OPropertySetHelper::fire(std::unique_lock<std::mutex> & rGuard, const long * pnHandles, const com::sun::star::uno::Any * pNewValues, const com::sun::star::uno::Any * pOldValues, long nHandles, bool bVetoable) Line 594
  comphelper.dll!comphelper::OPropertySetHelper::impl_fireAll(std::unique_lock<std::mutex> & rGuard, long * i_handles, const com::sun::star::uno::Any * i_newValues, const com::sun::star::uno::Any * i_oldValues, long i_count) Line 487
  comphelper.dll!comphelper::OPropertySetHelper::setFastPropertyValues(std::unique_lock<std::mutex> & rGuard, long nSeqLen, long * pHandles, const com::sun::star::uno::Any * pValues, long nHitCount) Line 748
  tklo.dll!UnoControlModel::setFastPropertyValueImpl(std::unique_lock<std::mutex> & rGuard, long nPropId, const com::sun::star::uno::Any & rValue) Line 1247
  comphelper.dll!comphelper::OPropertySetHelper::setFastPropertyValue(long nHandle, const com::sun::star::uno::Any & rValue) Line 393
  comphelper.dll!comphelper::OPropertySetAggregationHelper::setFastPropertyValue(long _nHandle, const com::sun::star::uno::Any & _rValue) Line 528
  cppuhelper3MSC.dll!cppu::OPropertySetHelper::setPropertyValue(const rtl::OUString & rPropertyName, const com::sun::star::uno::Any & rValue) Line 264
• pcrlo.dll!pcr::FormComponentPropertyHandler::setPropertyValue(const rtl::OUString & _rPropertyName, const com::sun::star::uno::Any & _rValue) Line 465
  pcrlo.dll!pcr::OPropertyBrowserController::Commit(const rtl::OUString & rName, const com::sun::star::uno::Any & _rValue) Line 1326
  pcrlo.dll!pcr::OBrowserListBox::valueChanged(const com::sun::star::uno::Reference<com::sun::star::inspection::XPropertyControl> & _rxControl) Line 637
  pcrlo.dll!pcr::PropertyControlContext_Impl::impl_processEvent_throw(const comphelper::AnyEvent & _rEvent) Line 291
  pcrlo.dll!pcr::PropertyControlContext_Impl::processEvent(const comphelper::AnyEvent & _rEvent) Line 272
  comphelper.dll!comphelper::AsyncEventNotifierBase::execute() Line 138
  comphelper.dll!comphelper::AsyncEventNotifier::execute() Line 155
  salhelper3MSC.dll!salhelper::Thread::run() Line 40
  salhelper3MSC.dll!threadFunc(void * param) Line 190
  sal3.dll!oslWorkerWrapperFunction(void * pData) Line 67
  ucrtbased.dll!thread_start<unsigned int (__cdecl*)(void *),1>(void * const parameter) Line 97
  kernel32.dll!BaseThreadInitThunk()
  ntdll.dll!RtlUserThreadStart()

In this case, there is no reason to hold the mutex so long. It is only
needed to get copies of the object state, so only lock the mutex for
that necessary time frame.

Change-Id: Iab8bedb901295a6936d85de1cfb20efa2ce8a4f4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/202299
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/202462
Tested-by: Jenkins
Reviewed-by: Adolfo Jayme Barrientos <fitojb@ubuntu.com>

extensions/source/propctrlr/formcomponenthandler.cxx

@@ -316,14 +316,21 @@ namespace pcr
             return;
         }
 
-        ::osl::MutexGuard aGuard( m_aMutex );
-        PropertyId nPropId( impl_getPropertyId_throwUnknownProperty( _rPropertyName ) ); // check if property is known by the handler
+        PropertyId nPropId;
+        // tdf#170961: don't hold mutex longer than necessary; specifically, do not hold it while
+        // calling component's setPropertyValue, which may call into main thread
+        Reference<XPropertySet> xComponent;
+        {
+            osl::MutexGuard aGuard(m_aMutex);
+            nPropId = impl_getPropertyId_throwUnknownProperty(_rPropertyName);
+            xComponent = m_xComponent;
+        }
 
         Reference< graphic::XGraphicObject > xGrfObj;
         if ( PROPERTY_ID_IMAGE_URL == nPropId && ( _rValue >>= xGrfObj ) )
         {
             DBG_ASSERT( xGrfObj.is(), "FormComponentPropertyHandler::setPropertyValue() xGrfObj is invalid");
-            m_xComponent->setPropertyValue(PROPERTY_GRAPHIC, uno::Any(xGrfObj->getGraphic()));
+            xComponent->setPropertyValue(PROPERTY_GRAPHIC, uno::Any(xGrfObj->getGraphic()));
         }
         else if ( PROPERTY_ID_FONT == nPropId )
         {
@@ -333,21 +340,21 @@ namespace pcr
                 SAL_WARN("extensions.propctrlr", "setPropertyValue: unable to get property " << PROPERTY_ID_FONT);
 
             for (const NamedValue& fontPropertyValue : aFontPropertyValues)
-                m_xComponent->setPropertyValue( fontPropertyValue.Name, fontPropertyValue.Value );
+                xComponent->setPropertyValue( fontPropertyValue.Name, fontPropertyValue.Value );
         }
         else
         {
             Any aValue = _rValue;
 
             Reference< resource::XStringResourceResolver > xStringResourceResolver
-                = lcl_getStringResourceResolverForProperty( m_xComponent, _rPropertyName, _rValue );
+                = lcl_getStringResourceResolverForProperty( xComponent, _rPropertyName, _rValue );
             if( xStringResourceResolver.is() )
             {
                 Reference< resource::XStringResourceManager >
                     xStringResourceManager( xStringResourceResolver, UNO_QUERY );
                 if( xStringResourceManager.is() )
                 {
-                    Any aPropertyValue( m_xComponent->getPropertyValue( _rPropertyName ) );
+                    Any aPropertyValue( xComponent->getPropertyValue( _rPropertyName ) );
                     TypeClass eType = aPropertyValue.getValueTypeClass();
                     if( eType == TypeClass_STRING )
                     {
@@ -375,7 +382,7 @@ namespace pcr
 
                         // Create new Ids
                         std::unique_ptr<OUString[]> pNewPureIds(new OUString[nNewCount]);
-                        Any aNameAny = m_xComponent->getPropertyValue(PROPERTY_NAME);
+                        Any aNameAny = xComponent->getPropertyValue(PROPERTY_NAME);
                         OUString sControlName;
                         aNameAny >>= sControlName;
                         OUString aIdStrBase = aDot
@@ -462,7 +469,7 @@ namespace pcr
                 }
             }
 
-            m_xComponent->setPropertyValue( _rPropertyName, aValue );
+            xComponent->setPropertyValue( _rPropertyName, aValue );
         }
     }