chart2/source/view/main/ChartView.cxx | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
New commits:
commit 375fda169be2af64db91ac970b8f83d663e40f21
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Fri Sep 14 08:25:18 2018 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Fri Sep 14 16:20:51 2018 +0200
Fix race in chart::ChartView
...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
<https://ci.libreoffice.org/job/gerrit_linux_clang_dbgutil/14432/console>,
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, <https://bz.apache.org/ooo/show_bug.cgi?id=109770>
"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::__cxx1998::_Deque_iterator<SfxBroadcaster*,
SfxBroadcaster*&, SfxBroadcaster**>, std::__debug::deque<SfxBroadcaster*,
std::allocator<SfxBroadcaster*> >
>::_Safe_iterator(std::__cxx1998::_Deque_iterator<SfxBroadcaster*,
SfxBroadcaster*&, SfxBroadcaster**> const&,
std::__debug::deque<SfxBroadcaster*, std::allocator<SfxBroadcaster*> > 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<chart::VCoordinateSystem>::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<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >::~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<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >
>(std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*) (__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<false>::__destroy<std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem>
>*>(std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*,
std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*) (__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<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem>
>*>(std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*,
std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*) (__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<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*,
std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >
>(std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*,
std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*,
std::allocator<std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> > >&) (__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::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >,
std::allocator<std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> > >
>::_M_erase_at_end(std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >*) (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::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >,
std::allocator<std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> > > >::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::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> >,
std::allocator<std::unique_ptr<chart::VCoordinateSystem,
std::default_delete<chart::VCoordinateSystem> > > >::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<com::sun::star::chart2::XScaling>::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<com::sun::star::chart2::XScaling>::operator=(com::sun::star::uno::Reference<com::sun::star::chart2::XScaling>
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<com::sun::star::chart2::XAxis>,
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<com::sun::star::chart2::XAxis>
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<com::sun::star::beans::XPropertySet> 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<com::sun::star::beans::XPropertySet> 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<com::sun::star::beans::XPropertySet>
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
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/chart2/source/view/main/ChartView.cxx
b/chart2/source/view/main/ChartView.cxx
index 80d99d8d5e4e..f53fd6b39549 100644
--- a/chart2/source/view/main/ChartView.cxx
+++ b/chart2/source/view/main/ChartView.cxx
@@ -1133,7 +1133,7 @@ ChartView::~ChartView()
void ChartView::impl_deleteCoordinateSystems()
{
//delete all coordinate systems
- m_aVCooSysList.clear();//#i109770#
+ m_aVCooSysList.clear();
}
// datatransfer::XTransferable
@@ -1718,6 +1718,8 @@ bool ChartView::getExplicitValuesForAxis(
, ExplicitScaleData& rExplicitScale
, ExplicitIncrementData& rExplicitIncrement )
{
+ SolarMutexGuard aSolarGuard;
+
impl_updateView();
if(!xAxis.is())
@@ -2471,6 +2473,8 @@ static const char* envChartDummyFactory =
getenv("CHART_DUMMY_FACTORY");
void ChartView::createShapes()
{
+ SolarMutexGuard aSolarGuard;
+
osl::ResettableMutexGuard aTimedGuard(maTimeMutex);
if(mrChartModel.isTimeBased())
{
@@ -2484,7 +2488,6 @@ void ChartView::createShapes()
impl_deleteCoordinateSystems();
if( m_pDrawModelWrapper )
{
- SolarMutexGuard aSolarGuard;
// #i12587# support for shapes in chart
m_pDrawModelWrapper->getSdrModel().EnableUndo( false );
m_pDrawModelWrapper->clearMainDrawPage();
@@ -2512,7 +2515,6 @@ void ChartView::createShapes()
// #i12587# support for shapes in chart
if ( m_pDrawModelWrapper )
{
- SolarMutexGuard aSolarGuard;
m_pDrawModelWrapper->getSdrModel().EnableUndo( true );
}
@@ -2961,8 +2963,6 @@ void ChartView::createShapes2D( const awt::Size&
rPageSize )
{
ShapeFactory* pShapeFactory =
ShapeFactory::getOrCreateShapeFactory(m_xShapeFactory);
- SolarMutexGuard aSolarGuard;
-
// todo: it would be nicer to just pass the page m_xDrawPage and format it,
// but the draw page does not support XPropertySet
formatPage( mrChartModel, rPageSize, mxRootShape, m_xShapeFactory );
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
