Bug 118199 - Draw/Impress crashes upon pasting a table from Writer (not on Windows)
Summary: Draw/Impress crashes upon pasting a table from Writer (not on Windows)
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Draw (show other bugs)
Version:
(earliest affected)
6.2.0.0.alpha0+
Hardware: All Linux (All)
: highest critical
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:6.2.0 target:6.1.3
Keywords: bibisected, bisected, haveBacktrace, needUITest, regression
Depends on:
Blocks: Regressions-AW080
  Show dependency treegraph
 
Reported: 2018-06-16 18:41 UTC by Buovjaga
Modified: 2018-10-04 09:05 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
gdb backtrace (48.42 KB, text/plain)
2018-06-17 10:16 UTC, Xisco Faulí
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Buovjaga 2018-06-16 18:41:59 UTC
1. Create a table in Writer
2. Select the whole table, copy
3. Paste it in a new Draw document

Only on Linux, not on Win. Also happens with gtk3 backend. When I tried to get a backtrace, it did not crash. Crashes in safe mode as well.

Arch Linux 64-bit
Version: 6.2.0.0.alpha0+
Build ID: dc9ee533dc707cc10b99d537eaccc3ee5aa555fe
CPU threads: 8; OS: Linux 4.16; UI render: default; VCL: kde4; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group threaded
Built on June 16th 2018
Comment 1 MM 2018-06-17 08:44:46 UTC
Confirmed on ubuntu 16.04 x64 with Version: 6.2.0.0.alpha0+
Build ID: c4c56de1b0e62ec866b519b2b24c5e805f0a86d3
CPU threads: 2; OS: Linux 4.4; UI render: default; VCL: gtk2; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-06-09_00:23:35
Locale: en-US (en_US.UTF-8); Calc: threaded

Strangely enough, when recovering the docs, there is a table copied in draw.
Comment 2 raal 2018-06-17 09:45:54 UTC
This seems to have begun at the below commit.
Adding Cc: to Armin Le Grand ; Could you possibly take a look at this one?
Thanks
 c91d690754f29061acf6749200df464d2f0c5c8b is the first bad commit
commit c91d690754f29061acf6749200df464d2f0c5c8b
Author: Jenkins Build User <tdf@pollux.tdf>
Date:   Fri Apr 6 23:11:09 2018 +0200

    source 6c14c27c75a03e2363f2b363ddf0a6f2f46cfa91

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
SOSAW080: Make SdrModel& prerequisite to SdrObjects
Comment 3 Xisco Faulí 2018-06-17 10:16:11 UTC
Created attachment 142814 [details]
gdb backtrace
Comment 4 Julien Nabet 2018-07-02 20:19:20 UTC
Noel: I haven’t tested it yet but think your commit "Pass OutLinerParaObject..." may help here (https://cgit.freedesktop.org/libreoffice/core/commit/?id=50c63e5c2f7962e8893e2d04b0e958209432f4c9)
Comment 5 Xisco Faulí 2018-07-02 23:31:16 UTC
I'm afraid it's still reproducible in

Version: 6.2.0.0.alpha0+
Build ID: 5fce97a58b8f764e35bf98128591c9a89537da05
CPU threads: 4; OS: Linux 4.13; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded
Comment 6 Armin Le Grand (allotropia) 2018-07-03 09:59:03 UTC
Cannot check - when opening writer and try to create a table, I get a hang in debugger on SwWrongList::GetWrongPos where maList obviously is wrong/bad/not initialized. Updating master (hopefully fixed)
Comment 7 Armin Le Grand (allotropia) 2018-07-03 11:01:28 UTC
SdrText::~SdrText is called in Model cleanup after SdrTableObj::~SdrTableObj, so SdrText::mrObject is already dead, but used in Cell::~Cell by using Cell::dispose by using SetOutlinerParaObject.

Shutdown problem -> each Cell remembers the SdrObject it is based upon. Need a mechanism to safely discover that the SdrObject is already dead. Note: calling Cell::SetOutlinerParaObject in dispose() called from destructor is not good anyways - that's a virtual function.

Problem detected, but how to solve that...?

Compared with libreoffice-6-0: Difference is that mpOutlinerParaObject in SdrText is already destroyed in SdrText::~SdrText() and in SdrText::SetOutlinerParaObject a check

    if( mpOutlinerParaObject != pTextObject )

is used, so the critical access to the already destroyed SdrObject does not happen. A second diff is that mpOutlinerParaObject in master is a std::unique_ptr<OutlinerParaObject> now.

Thiinking about it...
Comment 8 Noel Grandin 2018-07-03 13:38:30 UTC
@Alg, something like:
   https://gerrit.libreoffice.org/#/c/56873/
?
which fixes it for me
Comment 9 Armin Le Grand (allotropia) 2018-07-03 16:26:00 UTC
Used
  ::tools::WeakReference< SdrTextObj > mxObject;
in class SdrText - that works, but seems complicated. Have to secure all mxObject usages then (and not sure that SdrTableObj gets deleted). Looking for something mmore simple.
Comment 10 Armin Le Grand (allotropia) 2018-07-03 16:32:07 UTC
@Noel - yes - something like that :)
But I would prefer to solve the basic problem - that the local SdrTextObj& mrObject in class SdrText is no longer valid. Other accesses to it will/may cause the same crash to reappear anytime...
Comment 11 Noel Grandin 2018-07-03 16:44:32 UTC
@Alg, at destruction time, we only need to care about what goes on inside the destructor, and it is only at destruction time that the mrObject reference becomes invalid, unless I am missing something?
Comment 12 Armin Le Grand (allotropia) 2018-07-03 17:38:53 UTC
@Noel: Cannot answer that :-) Don't know all your thoughts about it ;-)

Problem is that dispose() is executed multiple times on a Cell instance, last from the Undo-Stack cleanup when SdrModel goes down (btw why does the clipboard-model have an undo steck...?). Latest there, the SdrTableObj is *really* dead and it's mem freed. OTOH SdrTableObj cleans up principally nicely, already disposing as needed.
Thus - secure double dispose and do not access SdrTextObj when disposed. No bool member needed, mpProperties is safe for this. Also avoid resetting OutlinerParaObject when call is from edstructor - this is expensive and superfuous. And when at it - make dispose really private and add users as friends (surprise - there are other direct callers I could identify doing this).

What about https://gerrit.libreoffice.org/#/c/56896/ ..?
Comment 13 Commit Notification 2018-07-04 10:16:57 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=ea39c41fdf63191579d25f327db81db14862251c

tdf#118199 avoid double dispose actions

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 14 Buovjaga 2018-07-04 11:05:05 UTC
Verified. Thanks.

Arch Linux 64-bit
Version: 6.2.0.0.alpha0+
Build ID: ea39c41fdf63191579d25f327db81db14862251c
CPU threads: 8; OS: Linux 4.17; UI render: default; VCL: gtk3; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group threaded
Built on July 4th 2018
Comment 15 Xisco Faulí 2018-10-03 15:56:10 UTC
ahhh, reproduced in

Version: 6.1.3.0.0+
Build ID: 316281cf1d888a4bac9f1e40e92c66405480249c
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group threaded

Cherry-picked to 6.1 -> https://gerrit.libreoffice.org/#/c/61318/1
Comment 16 Commit Notification 2018-10-04 09:05:40 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=7239716e4307a348d818606340e34b4fc6ceb282&h=libreoffice-6-1

tdf#118199 avoid double dispose actions

It will be available in 6.1.3.

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.