Bug 118377

Summary: CRASH: Writer crashes inserting a section
Product: LibreOffice Reporter: Xisco Faulí <xiscofauli>
Component: WriterAssignee: Armin Le Grand <Armin.Le.Grand>
Status: VERIFIED FIXED    
Severity: critical CC: Armin.Le.Grand, buzea.bogdan, serval2412, telesto, vmiklos, xiscofauli
Priority: highest Keywords: bibisected, bisected, haveBacktrace, regression
Version: 6.1.0.0.beta2+   
Hardware: All   
OS: All   
Whiteboard: target:6.2.0 target:6.1.1
Crash report or crash signature: ["std::deque<std::unique_ptr<OutDevState,std::default_delete<OutDevState> >,std::allocator<std::unique_ptr<OutDevState,std::default_delete<OutDevState> > > >::push_back(std::unique_ptr<OutDevState,std::default_delete<OutDevState> > &&)"] Regression By:
Bug Depends on:    
Bug Blocks: 116979    
Attachments: sample file
gdb backtrace
Valgrind trace

Description Xisco Faulí 2018-06-25 16:34:13 UTC 
Created attachment 143107 [details]
sample file

Steps to reproduce:
1. Open Writer
2. Insert - Section
3. Check Link
4. Insert the attached document
5. Click on insert

-> CRASH!

Reproduced in

Version: 6.2.0.0.alpha0+
Build ID: dbfa1c452fd9d02330cb3ec5bf2fd4f2c7782d1a
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded
Comment 1 Xisco Faulí 2018-06-25 16:37:40 UTC 
Regression introduced by:

author	Armin Le Grand <Armin.Le.Grand@cib.de (CIB)>	2018-03-01 15:54:32 +0100
committer	Armin Le Grand <Armin.Le.Grand@cib.de>	2018-04-06 22:29:02 +0200
commit 6c14c27c75a03e2363f2b363ddf0a6f2f46cfa91 (patch)
tree e66f50adb222dbc1490b4df2d3c63541dad999ac
parent e1b247a843c5eb850fe0079819239d9883412d38 (diff)
SOSAW080: Added first bunch of basic changes to helpers

Bisected with: bibisect-linux64-6.1

Adding Cc: to Armin Le Grand
Comment 2 Xisco Faulí 2018-06-25 16:56:57 UTC 
Created attachment 143109 [details]
gdb backtrace
Comment 3 Julien Nabet 2018-06-30 11:33:08 UTC 
Created attachment 143227 [details]
Valgrind trace

On pc Debian x86-64 with master sources updated yesterday, it crashes my whole laptop.
I could retrieve some info by using valgrind.
Comment 4 Armin Le Grand 2018-07-04 11:36:52 UTC 
Stack says:

vcllo.dll!std::_Container_base12::_Orphan_all() Line 217 (c:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\xutility:217)
vcllo.dll!std::_Deque_alloc<std::_Deque_base_types<std::unique_ptr<OutDevState,std::default_delete<OutDevState> >,std::allocator<std::unique_ptr<OutDevState,std::default_delete<OutDevState> > > > >::_Orphan_all() Line 877 (c:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\deque:877)
vcllo.dll!std::deque<std::unique_ptr<OutDevState,std::default_delete<OutDevState> >,std::allocator<std::unique_ptr<OutDevState,std::default_delete<OutDevState> > > >::push_back(std::unique_ptr<OutDevState,std::default_delete<OutDevState> > && _Val) Line 1160 (c:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\deque:1160)
vcllo.dll!OutDevStateStack::push_back(OutDevState * p) Line 25 (c:\lo\work01\vcl\source\outdev\outdevstatestack.cxx:25)
vcllo.dll!OutputDevice::Push(PushFlags nFlags) Line 117 (c:\lo\work01\vcl\source\outdev\outdevstate.cxx:117)
vcllo.dll!vcl::ReferenceDeviceTextLayout::ReferenceDeviceTextLayout(const Control & _rControl, OutputDevice & _rTargetDevice, OutputDevice & _rReferenceDevice) Line 140 (c:\lo\work01\vcl\source\gdi\textlayout.cxx:140)
vcllo.dll!vcl::ControlTextRenderer::ControlTextRenderer(const Control & _rControl, OutputDevice & _rTargetDevice, OutputDevice & _rReferenceDevice) Line 336 (c:\lo\work01\vcl\source\gdi\textlayout.cxx:336)
vcllo.dll!Control::DrawControlText(OutputDevice & _rTargetDevice, const tools::Rectangle & rRect, const rtl::OUString & _rStr, DrawTextFlags _nStyle, std::vector<tools::Rectangle,std::allocator<tools::Rectangle> > * _pVector, rtl::OUString * _pDisplayText, const Size * i_pDeviceSize) Line 436 (c:\lo\work01\vcl\source\control\ctrl.cxx:436)
vcllo.dll!Button::ImplDrawAlignedImage(OutputDevice * pDev, Point & rPos, Size & rSize, unsigned long nImageSep, DrawFlags nDrawFlags, DrawTextFlags nTextStyle, tools::Rectangle * pSymbolRect, bool bAddImageSep) Line 271 (c:\lo\work01\vcl\source\control\button.cxx:271)
vcllo.dll!RadioButton::ImplDraw(OutputDevice * pDev, DrawFlags nDrawFlags, const Point & rPos, const Size & rSize, const Size & rImageSize, tools::Rectangle & rStateRect, tools::Rectangle & rMouseRect) Line 2066 (c:\lo\work01\vcl\source\control\button.cxx:2066)
vcllo.dll!RadioButton::ImplDrawRadioButton(OutputDevice & rRenderContext) Line 2165 (c:\lo\work01\vcl\source\control\button.cxx:2165)
vcllo.dll!RadioButton::Paint(OutputDevice & rRenderContext, const tools::Rectangle & __formal) Line 2427 (c:\lo\work01\vcl\source\control\button.cxx:2427)
vcllo.dll!vcl::Window::ImplPaintToDevice(OutputDevice * i_pTargetOutDev, const Point & i_rPos) Line 1448 (c:\lo\work01\vcl\source\window\paint.cxx:1448)
vcllo.dll!vcl::Window::PaintToDevice(OutputDevice * pDev, const Point & rPos, const Size & __formal) Line 1521 (c:\lo\work01\vcl\source\window\paint.cxx:1521)
tklo.dll!VCLXWindow::draw(long nX, long nY) Line 2333 (c:\lo\work01\toolkit\source\awt\vclxwindow.cxx:2333)
tklo.dll!UnoControl::draw(long x, long y) Line 1014 (c:\lo\work01\toolkit\source\controls\unocontrol.cxx:1014)
drawinglayerlo.dll!drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(const drawinglayer::primitive2d::BasePrimitive2D & rCandidate) Line 672 (c:\lo\work01\drawinglayer\source\processor2d\vclpixelprocessor2d.cxx:672)
drawinglayerlo.dll!drawinglayer::processor2d::BaseProcessor2D::process(const drawinglayer::primitive2d::Primitive2DContainer & rSource) Line 72 (c:\lo\work01\drawinglayer\source\processor2d\baseprocessor2d.cxx:72)
drawinglayerlo.dll!drawinglayer::processor2d::BaseProcessor2D::process(const drawinglayer::primitive2d::BasePrimitive2D & rCandidate) Line 49 (c:\lo\work01\drawinglayer\source\processor2d\baseprocessor2d.cxx:49)
drawinglayerlo.dll!drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(const drawinglayer::primitive2d::BasePrimitive2D & rCandidate) Line 913 (c:\lo\work01\drawinglayer\source\processor2d\vclpixelprocessor2d.cxx:913)

Rendering of a RadioButton (probably unrelated to my changes, but you never know).
Fact is that in OutputDevice::Push the mpOutDevStateStack is 'empty' -> mpOutDevStateStack->push_back( pState ) crashes. I have no idea what mpOutDevStateStack is or how it is managed. Taking a look...
Comment 5 Armin Le Grand 2018-07-04 12:03:26 UTC 
mpOutDevStateStack is a std::unique_ptr<OutDevStateStack> and gets initialized in OutputDevice::OutputDevice (mpOutDevStateStack.reset(new OutDevStateStack)). It only gets reset (mpOutDevStateStack.reset()) in OutputDevice::dispose(), thus I guess this must happen somehow.
When breaking in Control::DrawControlText at the end where ControlTextRenderer gets constructed, mpControlData->mpReferenceDevice has a device, but it's VclReferenceBase says indeed -> {mnRefCnt=2 mbDisposed=true }.
That mpControlData->mpReferenceDevice comes from Control directly, so that may also be disposed already. Question is if this can be detected? There is already Control::ImplCallEventListenersAndHandler where xThis->IsDisposed() is used with 'VclPtr<Control> xThis(this)' (whyever).
Comment 6 Armin Le Grand 2018-07-04 12:17:29 UTC 
One working solution is to add

    mpControlData->mpReferenceDevice->isDisposed()

to Control::DrawControlText last 'if' to avoid a ControlTextRenderer being constructed. I will also try to construct a ControlTextRenderer by using _rTargetDevice if mpReferenceDevice is disposed.
Comment 7 Armin Le Grand 2018-07-04 12:25:11 UTC 
Second solution works too (adding

    mpControlData->mpReferenceDevice->isDisposed() ? _rTargetDevice : *mpControlData->mpReferenceDevice

as 2nd parameter to ControlTextRenderer contructor.
But at every repaint this gets repeated and the mpControlData->mpReferenceDevice stays disposed - also after dialog is closed...

-> We somehow have a living Control with disposed mpControlData->mpReferenceDevice. I do not know enough abou the current Window implementation to say if this is okay or very bad.

@Miklos: Do you have an idea who might nowadays know enough about Windows/Control vcl stuff to take a look at this one...?
Comment 8 Miklos Vajna 2018-07-04 13:05:49 UTC 
Armin, I'm afraid others already consider you an expert in vcl topics. ;-)

What about just asking on the dev mailing list, which has more subscribers than this bug? Best if you push your proof of concept patch to gerrit, so there is something concrete to discuss.
Comment 9 Armin Le Grand 2018-07-04 17:06:16 UTC 
@Miklos: All correct - except me being a specialist for VCL ;-) Todo for tomorrow...
Comment 10 Armin Le Grand 2018-07-12 08:24:37 UTC 
@Miklos: okay, added https://gerrit.libreoffice.org/#/c/57319/
Any comments...? People to add...?
Comment 11 Armin Le Grand 2018-07-13 13:07:29 UTC 
Have now secured access to mpControlData->mpReferenceDevice by using Control::GetReferenceDevice() everywhere and checking for disposed inside of Control::GetReferenceDevice() which will reset mpControlData->mpReferenceDevice on demand. That way all usages (and hopefully future ones) will be safe.
Executing the test case of this task looks good (same visualization as lo-6-1), so I think this is a valid fix.
Comment 12 Commit Notification 2018-07-15 12:11:08 UTC 
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3debc8c6eded517f7401c2e7ad45f73854c32e57

tdf#118377 Do not use disposed OutputDevice

It will be available in 6.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Xisco Faulí 2018-07-17 17:41:14 UTC 
Verified in

Version: 6.2.0.0.alpha0+
Build ID: f543b6a0ac6cf30922c1a1ae9bfce1d605f1d4f1
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded

@Armin, thanks for fixing this!! Should this issue be closed as RESOLVED FIXED ?
Comment 14 Armin Le Grand 2018-07-18 14:53:44 UTC 
yes
Comment 15 Commit Notification 2018-07-27 06:26:29 UTC 
Armin Le Grand committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5906980ed16d99a2872a7b90c9a55dc339430f63&h=libreoffice-6-1

tdf#118377 Do not use disposed OutputDevice

It will be available in 6.1.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 BogdanB 2018-11-19 14:20:41 UTC 
Fixed.

Verified also in 
Versiune: 6.1.3.2
Identificator construire: 86daf60bf00efa86ad547e59e09d6bb77c699acb
Fire CPU: 4; OS: Linux 4.15; Redare UI: implicit; VCL: gtk2; 
Setări regionale: ro-RO (ro_RO.UTF-8); Calc: group threaded

and in
Version: 6.2.0.0.beta1
Build ID: d1b41307be3f8c19fe6f1938cf056e7ff1eb1d18
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ro-RO (ro_RO.UTF-8); UI-Language: en-US
Calc: threaded