Bug 143468 - Writer becames unresponsive opening odt file with multipage table
Summary: Writer becames unresponsive opening odt file with multipage table
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.1.0.0.beta1+
Hardware: All All
: medium major
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: bibisected, bisected, haveBacktrace
Depends on:
Blocks: Writer-Table-Layouting
  Show dependency treegraph
 
Reported: 2021-07-21 10:28 UTC by andres@verot.com
Modified: 2022-07-10 09:10 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
Doc that crashes writer (96.75 KB, application/vnd.oasis.opendocument.text)
2021-07-21 10:28 UTC, andres@verot.com
Details
143468_perf.svg: FlameGraph during the hang (2.87 MB, image/svg+xml)
2021-07-27 06:47 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description andres@verot.com 2021-07-21 10:28:02 UTC
Created attachment 173729 [details]
Doc that crashes writer

The attached document is years old, it has gone through many different versions of Writer and a couple back and froth with MS Office.

When I open it with 7.2.0.1 in both win7 and win10 I only have to try to drag the scroll bar to the end of the document for Writer to become unresponsive.

I also tried in Linux with version 7.1.4.2 and just by opening the document the program stops responding.
Comment 1 Telesto 2021-07-21 12:09:10 UTC
Freeze when scrolling down/ for me at page 27
Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 3d18cae102e16b85fb8787f5ec3b086bfa2bd7b8
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL
Comment 2 Telesto 2021-07-21 12:12:17 UTC
No issue with
Version: 6.4.0.0.beta1+ (x64)
Build ID: 20be5cd0bdc57d812bf34a2debfe48caa51de881
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: default; VCL: win; 
Locale: nl-NL (nl_NL); UI-Language: en-US
Calc: CL
Comment 3 Timur 2021-07-21 13:50:22 UTC
Linux 7.0:
commit 95793203fffed9e0ce6cfc73d479367caeb48d0e
Date:   Wed Jun 3 21:57:43 2020 +0200
    source sha:0fe109a27c9a1b10b0b8a3314cb55926ecf103ae
    previous sha:deee67c566811189ee66d5766d0c9fc644a0120b

author	Justin Luth <justin.luth@collabora.com>	Sat May 30 14:20:02 2020 +0300
Revert "tdf#105478 sw layout: treat minHeight as "do not split row""
This reverts LO7.0 commit aa5dc0b48cd4db6883c9b52c68b16170c9c81d1f,
in order to prevent regressions like tdf#133441.

I don't see how revert is a regression. But it really didn't hang in 6.4.
Comment 4 Telesto 2021-07-22 08:21:54 UTC Comment hidden (no-value)
Comment 5 Justin L 2021-07-26 07:19:19 UTC
I expect that some other change happened after the initial commit and then the revert exposed it. Likely it is a very specific problem instance, and that any change in table content would stop this freezing.

Since my commits were dealing with "minimum row height", I changed the document to remove that, hoping that I could identify that interim commit. However, because the row heights are now different, the document doesn't freeze on loading any more. (Select table, Table - Size - Minimize row height)
Comment 6 Justin L 2021-07-27 06:47:05 UTC
Created attachment 173874 [details]
143468_perf.svg: FlameGraph during the hang

I don't really know what to DO with a FlameGraph, but here is about a minutes worth of data.
Comment 7 Justin L 2021-07-27 07:07:56 UTC
Since these patches never landed in shipping code, I'm removing the regression keyword. Also removing bisected, since the ultimate commit hasn't been identified yet.

I'm also not going to attempt to fix this, because we already know what happens when I touch layout code...
Comment 8 Justin L 2021-07-27 10:47:55 UTC
I found the regression commit.
author	Justin Luth  on 2020-05-19 09:15:19 +0200
commit 8a21d5053d331160e4913dc80c045a454ec84de3
tdf#132642 sw layout: try2 emulate table kept-with-next not splitting

and specifically the part which allowed setting
bTryToSplit = false;

So that leads to checking two other values on the table. The first is the "Text flow" "Keep with Next Paragraph" which amazingly is turned on. Really? We want this 20+ page table to fit on the same page as the following paragraph?
[Turning this off still hangs the document, but in a different way.]

The other setting is "allow row to split". In 6.4, you will notice that the rows never try to split across pages, even though the setting is "Allow row to split across pages". Now, in 7.0 the rows are splitting as the setting is telling them to do. [Turning this off "fixes" the hanging problem as a workaround.]


Once again, I'm not sure the ultimate cause has been found, since this didn't hang in 5.1 (before the original try1 commit). The intention of these changes was to only make affect the layout when emulateTableKeep is true, which should not be true in this 101 row table. Therefore it should not be messing with row split, or keep with next paragraph (even though it mistakenly did from 5.2 - 7.0). So now the code is handling the document just the way the settings tell it to, and that triggers a loop.

I'm guessing the loop is caused by that merged cell around page 27.
Comment 9 Justin L 2021-07-28 09:59:52 UTC
I am assuming that my changes masked the effects of bug 121720, but I wasn't able to verify that.
Comment 10 Buovjaga 2022-04-13 17:28:41 UTC
Still repro

Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: 2f2df626117380427d2e5e8417316f52823f1e6f
CPU threads: 8; OS: Linux 5.17; UI render: default; VCL: kf5 (cairo+xcb)
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded