Bug 139426 - Update of Contents is freezing LibreOffice
Summary: Update of Contents is freezing LibreOffice
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.1.6.3 release
Hardware: All All
: high major
Assignee: Armin Le Grand
URL:
Whiteboard: target:7.3.0 target:7.2.0.2
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2021-01-05 11:39 UTC by Stefan Parimucha
Modified: 2021-07-28 10:12 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
demo document (2.41 MB, application/vnd.oasis.opendocument.text)
2021-01-05 12:48 UTC, Stefan Parimucha
Details
screenshot of the CPU being at 100% (131.68 KB, image/png)
2021-01-05 17:07 UTC, BogdanB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Parimucha 2021-01-05 11:39:15 UTC
Description:
I have a document with several numbered sections and subsections created by default using styles Heading1 to Heading3, and with the contents register. When I update the index, the Writer froze and has to be killed from the console.

Steps to Reproduce:
1. create a document with several numbered sections and subsections using Header1 to Header3 styles
2. update Contents index


Actual Results:
Writer froze and has to be killed from the console.

Expected Results:
update register with all sections and subsections 


Reproducible: Always


User Profile Reset: Yes


OpenGL enabled: Yes

Additional Info:
update froze also in version 7.0.2.2
Comment 1 BogdanB 2021-01-05 11:57:51 UTC
Please upload here a demo document. Remove any sensitive data from your document and attach here.
Comment 2 Stefan Parimucha 2021-01-05 12:48:39 UTC
Created attachment 168701 [details]
demo document
Comment 3 BogdanB 2021-01-05 17:06:53 UTC
Tested in Version: 7.0.4.2
Build ID: dcf040e67528d9187c66b2379df5ea4407429775
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded

I have 4 cores on my computer, one of them is going to 100% and LibreOffice is freezing.
Comment 4 BogdanB 2021-01-05 17:07:30 UTC
Created attachment 168707 [details]
screenshot of the CPU being at 100%
Comment 5 Timur 2021-06-15 13:01:07 UTC
Confirming bug is of little value, it's lost in thousand of other bugs. 
Key step is tedious determining if regression, and here it is. And finding it. 
No repro 6.0, repro 6.1 and 7.2+.
Comment 6 Timur 2021-06-15 13:40:29 UTC
6.1:
commit aa3056a64b992812f01674ab72c99db0d73fa022
Date:   Fri Dec 22 05:41:30 2017 +0100
    source sha:0b53f794ffb2550288610b9488f11fd21ab85aae
    previous sha:fadbccc3176044b3642716f5b388a793165da05a

author	Armin Le Grand <Armin.Le.Grand@cib.de>	2017-12-20
committer	Armin Le Grand <Armin.Le.Grand@cib.de>	2017-12-20 
commit	0b53f794ffb2550288610b9488f11fd21ab85aae (patch)

Make used Pages alive in AssertFlyPages
There is a case where docs are created/exist that have an empty
2nd page with a single, page-anchored Frame (e.g. graphic), not
using a PageBreak. Persistence works in this case due to method
AssertFlyPages adding the missing page at end, but when multiple of
these docs get serialized (e.g. MailMerge) the problem exists for
in-between pages, too, and needs to be corrected. Also need to
correct the Pages for the Frames when Pages in that situation were
corrected.
Comment 7 Timur 2021-06-18 13:18:55 UTC
Hello Armin. Here is a regression from your "Make used Pages alive in AssertFlyPages". Please write if you'll work, when possible.
Comment 8 Armin Le Grand 2021-07-19 09:02:47 UTC
Not sure if I am involved, but checking shows that loop is in
    void SwLayAction::Action(OutputDevice* pRenderContext)
in sw/source/core/layout/layact.cxx in the loop
    while ( IsAgain() )
    {
        SetAgain(false);
        m_bNextCycle = false;
        InternalAction(pRenderContext);
        if (RemoveEmptyBrowserPages())
            SetAgain(true);
    }
where local m_bAgain gets re-set to true in InternalAction call and never ends -> death loop. Checking if removal of that quite long-time done stuff removes this...
Comment 9 Armin Le Grand 2021-07-19 13:04:30 UTC
Very strange - I wish I knew more about SW layouting :-(
Findings:
- taking out the block in sw/source/core/layout/pagechg.cxx lines 572 to 591 makes the problem not happen
- is active, that block is triggered again and again, with always new this ptr
- if block is in, local var m_bAgain at SwLayAction in sw/source/core/layout/layact.cxx is set back to true between
392        InternalAction(pRenderContext);
393        if (RemoveEmptyBrowserPages())
- It seems as if that menu command layouting removes that added lower SwBodyFrame again and it gets added again...?
- This does not happen at initial layout after loading - why?

The code at sw/source/core/layout/pagechg.cxx lines 572 to 591 is AFAIR intended for correcting once after loading...?
Not sure what to do here - maybe check if that happened already directly with my older fix added?
Comment 10 Justin L 2021-07-19 19:50:57 UTC
Confirmed the bibisect.
Also saw 2019 bug 123098 which also freezes (on fileopen) from the same commit.

Unfortunately, simply reverting won't work since I get a SIGSEGV, Segmentation fault unit test failure in CppunitTest_sw_layoutwriter.
#0  0x00007fffe5b1703a in std::__cxx1998::vector<SwAnchoredObject*, std::allocator<SwAnchoredObject*> >::size() const (this=0x18)
    at /usr/include/c++/9/bits/stl_vector.h:916
#1  0x00007fffe5da9acc in SwSortedObjs::size() const (this=0x0) at sw/source/core/layout/sortedobjs.cxx:45
#2  0x00007fffebb7ae5e in lcl_getVisibleFlyObjRect(SwWrtShell*) (pWrtShell=0x55555bb9e460) at sw/qa/extras/layout/layout.cxx:3485
Comment 11 Armin Le Grand 2021-07-20 13:56:11 UTC
I confirm the bibisect too.

The problem is that the current implementation tries to not create at a new SwPageFrame a contained SwBodyFrame immediately, but tries to do that on-demand e.g. when PageFormat changed (case RES_FMT_CHG: in SwPageFrame::UpdateAttr_). So the fix
  0b53f794ffb2550288610b9488f11fd21ab85aae
tries to do this correction when needed.

I experimented with removing the block in case RES_FMT_CHG: in SwPageFrame::UpdateAttr_ which solves the loop. It is just unfortunate to create this on-demand while in layout - that restarts the layout and we get the loop.

It may also be feasible to add a loop-deathwach-counter to SwLayAction::Action so that when between
392        InternalAction(pRenderContext);
393        if (RemoveEmptyBrowserPages())
m_bAgain is reset to true to hard break this after a defined number of tries, but better would be if that would not be needed. It *may* be needed/be safer anyways one day...

So next I tried to patch the on-demand creation of a SwBodyFrame to a tooling method at SwPageFrame, move the constructor and SwPageFrame::UpdateAttr_ to use this. This leads to surprisingly many places where loops and accesses to data create nullptr-exceptions -> it is just at many places not accepted to not have a Lower() frame in any form (e.g. an instance of SwBodyFrame).

After fixing quite some places/exceptions of that kind I gave up and tried next to just always create a SwBodyFrame as Lower() in SwPageFrame constructor. This should make the adaption in SwPageFrame::UpdateAttr_ case RES_FMT_CHG obsolete and solve the problem.

Only cost I can see is that maybe in some situations not needed temporary instances of a SwBodyFrame get created and replaced/destroyed again when something is added to the page. Due to so many places not checking for nullptr and thus relying on Lower() to exist this seems a good solution.

Need to check what the UnitTest would say to that, preparing possible fix...
Comment 12 Armin Le Grand 2021-07-20 17:31:39 UTC
Argh - error in CppunitTest_sw_mailmerge. That's exactly what the original fix fixed - AFAIR. Thus, need to check deeper.
Comment 13 Armin Le Grand 2021-07-21 11:22:41 UTC
Tried other solutions, e.g. take out m_pRoot->AssertFlyPages() in SwLayAction::InternalAction - that triggers the adaption from the original fix. It shows that I cannot really do that, loading the testdoc already loops. This shows that orig fix is needed/essential, so need to find a way to fix the special layout action that gets triggered by that menu command...
Comment 14 Armin Le Grand 2021-07-21 12:43:34 UTC
Experimenting with suppressing AssertFlyPages when layout is executed from SwEditShell::UpdateTableOf. Looks good, but needs checking...
Comment 15 Armin Le Grand 2021-07-21 13:02:23 UTC
Solution on gerrit (https://gerrit.libreoffice.org/c/core/+/119324), also checked that CppunitTest_sw_mailmerge still works with this.
Comment 16 Commit Notification 2021-07-22 07:30:52 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/0bac5c7e7d71658c5056c4bf0b71fbfb51b92ca0

tdf#139426 Supress AssertFlyPages in Tabe re-layout

It will be available in 7.3.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 17 Armin Le Grand 2021-07-23 15:15:07 UTC
Done AFAIK, please have a look.
Comment 18 Justin L 2021-07-26 06:18:04 UTC
(In reply to Armin Le Grand from comment #17)
> Done AFAIK, please have a look.

This document doesn't hang for me any more either. So I'll mark as verified. The related bug 123098 still hangs on file-open though, so the core issue hasn't been solved yet.
Comment 19 Commit Notification 2021-07-28 10:12:40 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/2b99db827e5bb5d43ca51b5fb90a054975361a7c

tdf#139426 Supress AssertFlyPages in Tabe re-layout

It will be available in 7.2.0.2.

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

Affected users are encouraged to test the fix and report feedback.