Bug 83260 - CRASH: autocorrect with change tracking halts LibreOffice
Summary: CRASH: autocorrect with change tracking halts LibreOffice
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
3.5.7.2 release
Hardware: Other All
: high critical
Assignee: Fyodor
URL:
Whiteboard: target:6.1.0 target:6.0.2
Keywords: haveBacktrace, preBibisect, regression
Depends on:
Blocks: Auto-Correct-Complete
  Show dependency treegraph
 
Reported: 2014-08-29 21:37 UTC by László Németh
Modified: 2018-02-16 22:30 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
test file (9.20 KB, application/vnd.oasis.opendocument.text)
2014-08-29 21:37 UTC, László Németh
Details
Possibly related backtrace (66.16 KB, text/plain)
2014-09-03 00:14 UTC, Matthew Francis
Details
GDB trace of crash with master (21.74 KB, text/plain)
2017-11-01 16:46 UTC, Buovjaga
Details

Note You need to log in before you can comment on or make changes to this bug.
Description László Németh 2014-08-29 21:37:06 UTC
Created attachment 105445 [details]
test file

Steps:

Open the attached test document, type "includ", and press space to halt LibreOffice (includ->include is part of the default English autocorrect list). If change tracking is disabled or Edit->Changes->Show is enabled, LibreOffice will not halt.

Edit->Changes->Show shows the tracked changes of the test file: a removed paragraph with a single letter and a removed letter in the actual paragraph.

Bjoern: maybe this problem is related to the changes mentioned in your letter on the development list.
Comment 1 Matthew Francis 2014-09-03 00:14:27 UTC
Created attachment 105644 [details]
Possibly related backtrace

OSX 10.9.4/ 4.4 master:

This doesn't crash for me after the autocomplete, but does choke on an assertion if I undo a couple of times afterwards. This does not occur if tracked changes is turned off, so I assume it may still be related.

Backtrace attached.

(I haven't run a bisect specifically for this, but based on an existing build I had sitting around, this still occurs at least as far back as commit ef8293485adbf6554569ca37b8c1bf8cce955842 (Fri May 30 15:11:51 2014 +0530))
Comment 2 Matthew Francis 2014-12-03 16:23:22 UTC
On Linux, this crash occurs all the way back to the start of the 43all repository, so it's pre-bibisect

On OSX, the latter symptom (crash on undo) occurs all the way back to 3.3.0

Reserving judgement about whether these two are the same issue until the code has been investigated, but in either case setting:

-> Whiteboard:preBibisect
Comment 3 Björn Michaelsen 2015-01-27 17:59:31 UTC
set version as per comment 2
Comment 4 Robinson Tryon (qubit) 2015-12-14 05:40:04 UTC Comment hidden (obsolete)
Comment 5 Xisco Faulí 2016-09-14 15:45:52 UTC Comment hidden (obsolete)
Comment 6 QA Administrators 2017-10-23 14:08:38 UTC Comment hidden (obsolete)
Comment 7 Timur 2017-11-01 16:41:19 UTC
I can't reproduce with 4.4 in Windows and Linux.
Comment 8 Buovjaga 2017-11-01 16:46:16 UTC
Created attachment 137425 [details]
GDB trace of crash with master

I repro the crash.

Arch Linux 64-bit, KDE Plasma 5
Version: 6.0.0.0.alpha1+
Build ID: af318eeb4e23694e17b09b902afb98ddf9da9b7b
CPU threads: 8; OS: Linux 4.13; UI render: default; VCL: kde4; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group
Built on November 1st 2017
Comment 9 Xisco Faulí 2017-11-01 22:21:50 UTC
Interestingly, I can reproduce it with a local build

Version: 6.0.0.0.alpha1+
Build ID: fc4f2f1b0f8c286e9ae259c44fb249261a8ac47f
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

but not with a daily build

Version: 6.0.0.0.alpha1+
Build ID: e90329ff3896cf33528b47830f49fcd05590bd6a
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 10 Fyodor 2017-12-18 07:37:20 UTC
I cannot reproduce halt, but can reproduce crush during Undo. Crush happens during "second" undo. Firstly autocorrection undone, then during undoing insertion of "includ" crush happens.

This is due to invalid node indexes (I think this is most frequent cause of crushes in undo/redo, due to their dependency of absolute node indexes...). 
Indexes itself made invalid during previous SwUndoInsert (autocorrection), because of redline paragraphs (one removed and one containing one removed letter) moved to special section of SwDoc for deleted redlines. This in its turn happens due to redline flags are modified during autocorrection execution. (In SwUndo redline flags set to 0n17 but diring autocorrection they set to 0n49). As a result RedlineGuard calls functions to move nodes outside of document which invalidates indexes stored in Undo.

I suspect that modification of redline flags done here https://opengrok.libreoffice.org/xref/core/sw/source/core/doc/DocumentContentOperationsManager.cxx#3900

I'll investigate this bug further.
Comment 11 Fyodor 2017-12-21 07:14:15 UTC
I've removed line https://opengrok.libreoffice.org/xref/core/sw/source/core/doc/DocumentContentOperationsManager.cxx#3901 and corresponding line https://opengrok.libreoffice.org/xref/core/sw/source/core/doc/DocumentContentOperationsManager.cxx#4001
Bug in Undo is gone (as expected).
Also I've start experiencing crush mentioned in https://bugs.documentfoundation.org/show_bug.cgi?id=83260#c8. (I think it only happens if formatting marks are OFF, but not sure). This crush also happens due to manipulation of redlining flags in DocumentContentOperationsManager::ReplaceRangeImpl and also gone with correction made.

For me is absolutely unclear the reason why someone changed redlining flags in ReplaceRangeImpl method... (changed and than set back). I didn't found any code which uses these flags (only Undo stores them and when replaying crushes LO)...
Comment 12 Fyodor 2017-12-22 06:40:25 UTC
This bug is tricky than seemed before...

Redlining implementation is very interesting. If you change Edit->Changes->Show option (either turn show redlining ON of OFF), deleted paragraphs (if so) moved either to special section or to document itself. So this option modifies document itself (model in MVC paradigm), but not how document shown. (In my opinion, Show redlining should only change how doc is shown and not document itself). As a result indexes, stored in Undo may be invalidated and Undo will crush LO.

I'll ask on dev list what to do with this issue...
Comment 13 Fyodor 2018-01-10 00:36:19 UTC
I've submitted patch https://gerrit.libreoffice.org/47686
Comment 14 Commit Notification 2018-02-15 08:56:50 UTC
Fyodor Yemelyanenko committed a patch related to this issue.
It has been pushed to "master":

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

tdf#83260 editeng sw: avoid accessing dead nodes in AutoCorrect

It will be available in 6.1.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 15 Commit Notification 2018-02-15 08:58:07 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1c8efde4daea648204e3ba19f8edc01ef3e548bd

tdf#83260 sw: call CompressRedlines() in ODF import

It will be available in 6.1.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 16 Michael Stahl 2018-02-15 09:01:06 UTC
this should be fixed now on master, thanks Fyodor for the great work!
Comment 17 Commit Notification 2018-02-15 10:29:37 UTC
Fyodor Yemelyanenko committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=aff321ae3c872ad881252979c9f10ab1a01c0ea0&h=libreoffice-6-0

tdf#83260 editeng sw: avoid accessing dead nodes in AutoCorrect

It will be available in 6.0.2.

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 18 Commit Notification 2018-02-16 09:25:28 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e7c67eb093d1e099225eaccdaaf29eff1a4c9325&h=libreoffice-6-0

tdf#83260 sw: call CompressRedlines() in ODF import

It will be available in 6.0.2.

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 19 Michael Stahl 2018-02-16 22:30:38 UTC
addendum: the problem of the file import creating redlines that CompressRedlines() will merge can apparently happen only in the ODF import,
because it creates the redlines in their special nodes-array section
(i.e. as hidden);
the DOC/DOCX/RTF import will insert the redlines in the body text
instead (i.e. as shown), and then the AppendRedline function will
already merge adjacent redlines.

other formats don't have redlines.