Fix race in chart::ChartView

Desktop / LibreOffice - Stephan Bergmann [redhat.com] - 14 September 2018 14:20 EDT

...that recently started to hit quite often during JunitTest_chart2_unoapi, when the main thread is painting windows and calls ChartView::createShapes (which calls impl_deleteCoordinateSystems, which clears m_aVCooSysList) while an URP thread executes chart::WrappedPropertySet::getPropertyValue and calls ChartView::getExplicitValuesForAxis (which calls findInCooSysList, which iterates over m_aVCooSysList), all apparently without locking access to m_aVCooSysList. (See e.g. the below backtraces from , where both threads operate on chart::ChartView instance 0x2633140.)

I assume this issue has always been there, and has only been made noticeable with recent 3e62ac3e9ef2f6759d8faca2c012dba51c314ba5 "loplugin:useuniqueptr in
VCoordinateSystem", which changed the implementation of impl_deleteCoordianteSystems from O(1) swap-and-clear to plain O(n) clear of m_aVCooSysList, which extends the time span in which the thread executing findInCooSysList can find an inconsistent m_aVCooSysList (and see in the below backtrace how much code is apparently executed during the destruction of m_aVCooSysList's VCartesianCoordinateSystem elements).

And indeed, "ChartView::getExplicitValuesForAxis accessing destroyed VCoordinateSystem" found apparently the same issue already in 2010, and the swap-and-clear was introduced as a means to address the race in 590a1a5225623eb922e63b02b62e711d153e9d55 "chart43: #i109770# ChartView::getExplicitValuesForAxis accessing destroyed VCoordinateSystem". (So the "//#i109770#" comment should have been removed from impl_deleteCoordinateSystem already when 3e62ac3e9ef2f6759d8faca2c012dba51c314ba5 dropped the swap; catching up on that now.)

For a proper fix, there appears to be no constitent existing locking scheme for ChartView (appart from a few scatter SolarMutexGuards). Lets be bold and see whether it works to put the whole bodies of (at least, for now) createShapes and getExpliticValuesForAxis under SolarMutexGuards. (Which means the SolarMutexGuard in createShapes2D can go, as it is exclusively called from createShapes.)

> Thread 2 (Thread 0x2ae81cbd2840 (LWP 10147)): > #0 0x00002ae81dee4f9a in __gnu_debug::_Safe_iterator_base::_M_attach(__gnu_debug::_Safe_sequence_base*, bool) () at /lib64/libstdc++.so.6 > #1 0x00002ae822e0a685 in __gnu_debug::_Safe_iterator_base::_Safe_iterator_base(__gnu_debug::_Safe_sequence_base const*, bool) (this=0x7ffedb73c598, __seq=0x2acc2f0, __constant=false) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/debug/safe_base.h:89 > #2 0x00002ae822f75c6a in __gnu_debug::_Safe_iterator, std::__debug::deque > >::_Safe_iterator(std::__cxx1998::_Deque_iterator const&, std::__debug::deque > const*) (this=0x7ffedb73c598, __i=, __seq=0x2acc2a0) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/debug/safe_iterator.h:152 [...] > #44 0x00002ae846166609 in chart::VCartesianCoordinateSystem::~VCartesianCoordinateSystem() (this=0x28c2b60) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/axes/VCartesianCoordinateSystem.cxx:64 > #45 0x00002ae8461767af in std::default_delete::operator()(chart::VCoordinateSystem*) const (this=0x28e4190, __ptr=0x28c2b60) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/unique_ptr.h:67 > #46 0x00002ae8461737d3 in std::unique_ptr >::~unique_ptr() (this=0x28e4190) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/unique_ptr.h:184 > #47 0x00002ae84626e295 in std::_Destroy > >(std::unique_ptr >*) (__pointer=0x28e4190) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_construct.h:93 > #48 0x00002ae84626e25f in std::_Destroy_aux::__destroy >*>(std::unique_ptr >*, std::unique_ptr >*) (__first=0x28e4190, __last=0x28e4198) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_construct.h:103 > #49 0x00002ae84626e1bd in std::_Destroy >*>(std::unique_ptr >*, std::unique_ptr >*) (__first=0x28e4190, __last=0x28e4198) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_construct.h:126 > #50 0x00002ae84626dce1 in std::_Destroy >*, std::unique_ptr > >(std::unique_ptr >*, std::unique_ptr >*, std::allocator > >&) (__first=0x28e4190, __last=0x28e4198) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_construct.h:151 > #51 0x00002ae846273bf3 in std::__cxx1998::vector >, std::allocator > > >::_M_erase_at_end(std::unique_ptr >*) (this=0x2633240, __pos=0x28e4190) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_vector.h:1352 > #52 0x00002ae846273b94 in std::__cxx1998::vector >, std::allocator > > >::clear() (this=0x2633240) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/bits/stl_vector.h:1126 > #53 0x00002ae846251d8c in std::__debug::vector >, std::allocator > > >::clear() (this=0x2633240) at /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/debug/vector:563 > #54 0x00002ae84623749c in chart::ChartView::impl_deleteCoordinateSystems() (this=0x2633140) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/main/ChartView.cxx:1136 > #55 0x00002ae846240346 in chart::ChartView::createShapes() (this=0x2633140) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/main/ChartView.cxx:2484 > #56 0x00002ae84623d8f6 in chart::ChartView::impl_updateView(bool) (this=0x2633140, bCheckLockedCtrler=true) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/main/ChartView.cxx:2563 > #57 0x00002ae846243eae in chart::ChartView::update() (this=0x2633140) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/main/ChartView.cxx:2720 > #58 0x00002ae845bafef5 in chart::ChartController::execute_Paint(OutputDevice&, tools::Rectangle const&) (this=0x2a60ac0, rRenderContext=..., rRect=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/main/ChartController_Window.cxx:479 > #59 0x00002ae845bceb26 in chart::ChartWindow::Paint(OutputDevice&, tools::Rectangle const&) (this=0x2928e60, rRenderContext=..., rRect=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/main/ChartWindow.cxx:100 [...] > #71 0x00002ae8280ee03c in Scheduler::ProcessTaskScheduling() () at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/source/app/scheduler.cxx:451 > #72 0x00002ae8280ed1ed in Scheduler::CallbackTaskScheduling() () at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/source/app/scheduler.cxx:270 > #73 0x00002ae828302716 in SalTimer::CallCallback() (this=0x20db780) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/inc/saltimer.hxx:55 > #74 0x00002ae828300b84 in SvpSalInstance::CheckTimeout(bool) (this=0x12640d0, bExecuteTimers=true) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/headless/svpinst.cxx:206 > #75 0x00002ae828301a61 in SvpSalInstance::DoYield(bool, bool) (this=0x12640d0, bWait=true, bHandleAllCurrentEvents=false) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/headless/svpinst.cxx:418 > #76 0x00002ae82811fbd1 in ImplYield(bool, bool) (i_bWait=true, i_bAllEvents=false) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/vcl/source/app/svapp.cxx:471 [...]

> Thread 1 (Thread 0x2ae842779700 (LWP 13177)): > #0 0x00002ae8461307dc in com::sun::star::uno::Reference::set(com::sun::star::chart2::XScaling*) (this=0x2ae842776bb8, pInterface=0x9999999999999999) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/include/com/sun/star/uno/Reference.hxx:234 > #1 0x00002ae84612f8a0 in com::sun::star::uno::Reference::operator=(com::sun::star::uno::Reference const&) (this=0x2ae842776bb8, rRef=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/include/com/sun/star/uno/Reference.hxx:349 > #2 0x00002ae846140bea in chart::ExplicitScaleData::operator=(chart::ExplicitScaleData const&) (this=0x2ae842776b98) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/inc/chartview/ExplicitScaleValues.hxx:37 > #3 0x00002ae846171ad2 in chart::VCoordinateSystem::getExplicitScale(int, int) const (this=0x28c2b60, nDimensionIndex=1, nAxisIndex=0) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/axes/VCoordinateSystem.cxx:272 > #4 0x00002ae84623d475 in chart::ChartView::getExplicitValuesForAxis(com::sun::star::uno::Reference, chart::ExplicitScaleData&, chart::ExplicitIncrementData&) (this=0x2633140, xAxis=..., rExplicitScale=..., rExplicitIncrement=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/view/main/ChartView.cxx:1735 > #5 0x00002ae845954d69 in chart::wrapper::Chart2ModelContact::getExplicitValuesForAxis(com::sun::star::uno::Reference const&, chart::ExplicitScaleData&, chart::ExplicitIncrementData&) (this=0x26b6920, xAxis=..., rOutExplicitScale=..., rOutExplicitIncrement=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/chartapiwrapper/Chart2ModelContact.cxx:143 > #6 0x00002ae8459d1d20 in chart::wrapper::WrappedScaleProperty::getPropertyValue(chart::wrapper::WrappedScaleProperty::tScaleProperty, com::sun::star::uno::Reference const&) const (this=0x2b68480, eScaleProperty=chart::wrapper::WrappedScaleProperty::SCALE_PROP_STEPMAIN, xInnerPropertySet=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/chartapiwrapper/WrappedScaleProperty.cxx:389 > #7 0x00002ae8459d114b in chart::wrapper::WrappedScaleProperty::setPropertyValue(chart::wrapper::WrappedScaleProperty::tScaleProperty, com::sun::star::uno::Any const&, com::sun::star::uno::Reference const&) const (this=0x2b68480, eScaleProperty=chart::wrapper::WrappedScaleProperty::SCALE_PROP_AUTO_STEPMAIN, rOuterValue=..., xInnerPropertySet=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/chartapiwrapper/WrappedScaleProperty.cxx:234 > #8 0x00002ae8459d0893 in chart::wrapper::WrappedScaleProperty::setPropertyValue(com::sun::star::uno::Any const&, com::sun::star::uno::Reference const&) const (this=0x2b68480, rOuterValue=..., xInnerPropertySet=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/controller/chartapiwrapper/WrappedScaleProperty.cxx:130 > #9 0x00002ae846523605 in chart::WrappedPropertySet::setPropertyValue(rtl::OUString const&, com::sun::star::uno::Any const&) (this=0x2b64cc0, rPropertyName=..., rValue=...) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/chart2/source/tools/WrappedPropertySet.cxx:89 > #10 0x00002ae841d97854 in gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*) (pThis=0x2b64cf8, nVtableIndex=4, pRegisterReturn=0x0, pReturnTypeRef=0x127fb10, bSimpleReturn=true, pStack=0x2ae8427777d0, nStack=0, pGPR=0x2ae842777ae0, pFPR=0x2ae842777aa0) at /home/tdf/lode/jenkins/workspace/lo_gerrit/Config/linux_clang_dbgutil_64/bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77 [...]

Change-Id: Id0236bf04bff9f06f8fc5ee9e2db295216a54c16 Reviewed-on: https://gerrit.libreoffice.org/60474

375fda169be2 Fix race in chart::ChartView
chart2/source/view/main/ChartView.cxx | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

Upstream: cgit.freedesktop.org


  • Share