Bug 140901 - EDITING Crash when deleting rows that are referenced by a chart
Summary: EDITING Crash when deleting rows that are referenced by a chart
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.2.8.2 release
Hardware: All All
: high critical
Assignee: Not Assigned
URL:
Whiteboard: target:7.3.0 target:7.2.3 target:7.1.7
Keywords: bibisected, bisected, filter:xls, regression
: 124836 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-03-09 05:45 UTC by txcsharppro
Modified: 2021-10-07 10:13 UTC (History)
3 users (show)

See Also:
Crash report or crash signature: ["ScChart2DataSequence::UpdateTokensFromRanges(ScRangeList%20const%20&)"]


Attachments
sample XLS file created in a prior version of LO (448.00 KB, application/vnd.ms-excel)
2021-03-09 05:45 UTC, txcsharppro
Details
sample screen shot (928.78 KB, image/jpeg)
2021-03-09 06:15 UTC, txcsharppro
Details

Note You need to log in before you can comment on or make changes to this bug.
Description txcsharppro 2021-03-09 05:45:55 UTC
Created attachment 170355 [details]
sample XLS file created in a prior version of LO

When a row or group of rows is deleted, if an existing chart contains a reference to any element in that row(s), Calc will quickly crash when attempting to delete the row(s). Bug occurs in LO 6.4.6.2 but likely exists in other versions, this issue has been around for years and I only just now spent enough time with it to narrow down the problem and write a defect for it.

Has a workaround, but it is clumsy and time-consuming. If the charts are first modified to exclude any references to any of the rows about to be deleted, then the row deletion is successful and the bug does not occur. This is the workaround I've been using, but I often forget to do this until it crashes and I ask myself, "what was that strange thing I had to do the last time it crashed".

STEPS TO REPRODUCE:
1. Open the sample file 'sample-file-for-bug-report - pristine.xls'. (Please forgive how odd it looks, I deleted a lot of text to anonymize the file.) If you look at the chart data ranges (see 'Charts' tab), you'll notice the charts reference data up through row 644 on the 'Sheet13' tab. This is also demonstrated in the screen shot 'Libre-Calc-bug-screen-1.jpg'.
2. Now go to the 'Sheet13' tab. Attempt to do any 'delete row(s)' operation for one or more rows which are referenced by the charts on the 'Charts' tab. Calc will quickly crash. In my testing, I deleted row 631.
3. That action corresponds to crashreport.libreoffice.org/stats/crash_details/abfe5c0d-223c-4c80-ae76-6983ae8f6258  and is reproducible 100% of the time.
4. If you allow the Recovery process to proceed, the file is recovered, and the user has the opportunity to try again with the same row deletion. (The row is still there, as would be expected.) This cycle of crash/recovery can be repeated infinitely.

DEV NOTES:
1. There are some related bugs which I am reporting separately. Those occur when continuing to retry the same row deletion, but since they have additional unexpected results, they are probably separate bugs.
2. This bug deals with deleting a row or group of rows which a chart holds a reference to. (Not deleting the data within a row. This is when removing the row from the spreadsheet.) For reproducing the bug, any kind of row deletion will do. You could delete a single row, delete a group of rows, or do an insert & undo that insert. As long as the row deletion is from inside a range that a chart is referencing, the bug will occur.
3. This might possibly be due to a broken reference error similar to what makes #REF! be displayed in a cell? Makes me wonder if some of the code for that exception handler for cells could be used for chart objects also. Just an idea.
4. Bug occurs whether using top menu, context menu, or keyboard shortcut, and is reproducible 100% of the time.
5. Notably, rows 645-651 are not referenced in the charts, so you can try deleting from among those rows and the bug does not occur. It only occurs when the deleted row(s) are referenced in a chart somewhere.
6. Bug is also reproducible by inserting a row (for example) between rows 630 and 631, then typing Ctrl-Z to undo the insert, then Calc will crash on the Undo operation.
7. It would be interesting to know whether deleting a row which references pure data elsewhere (such as on another tab) would cause a crash when there are NO charts involved. I did not test that scenario.

Additional notes which might be relevant:
(1) The attached sample spreadsheet is a spreadsheet which I created about 5 years ago in a previous version of LO. I've migrated it up from previous versions, but from its creation I've only ever saved it in XLS format (never in ODF). I do this so I can view it in MS-Office, but I have never edited this document in MS-Office. I'm not sure whether this file having been created on an older version of LO makes any difference, versus creating a new file with a chart, because I did not test that scenario.
(2) The type of chart doesn't seem to be related. This happens on simple charts as well as more complex ones having line + bubble + 8 different data types being tracked - you can repro the defect with any one of the charts in my sample file and deleting the others, and the bug can still be reproduced.
Comment 1 txcsharppro 2021-03-09 06:15:11 UTC
Created attachment 170356 [details]
sample screen shot

Initial bug submission only allowed one attachment, so here is the screen shot also.
Comment 2 Xisco Faulí 2021-04-01 08:54:06 UTC Comment hidden (obsolete)
Comment 3 Timur 2021-09-16 13:01:58 UTC
Thanks for detailed report. 

No repro 4.1 master. Repro 4.2 Linux and 7.3+ Windows for: 
1. open XLS attachment 170355 [details]
2. delete row 631.

Not related to jumbo sheets option. Regression in 42max.
Comment 4 Timur 2021-09-16 13:23:15 UTC
 80b18ee0affc1cfc2ceec3bf32ef65f481922efe is the first bad commit
Date:   Sat Sep 5 2015
    source-hash-5dd2e45e65641134309265db30c9d5f304d0a583
    pre source-hash-25763e59625ce83de4b82927359108f9e7878744    
Author:     Kohei Yoshida <kohei.yoshida@gmail.com>
        Avoid wholesale rebuilding of formula groups at re-calc time.
        And do it once when importing xls, xlsx, and ods documents.
Comment 5 Timur 2021-09-16 13:25:49 UTC Comment hidden (obsolete)
Comment 6 Kohei Yoshida 2021-09-17 22:19:30 UTC
What that likely means is that there is a flaw in how formula groups are made in the XLS import filter code.  What the code did prior to 80b18ee0affc1cfc2ceec3bf32ef65f481922efe was that it would scan the entire column to perform formula re-grouping (and/or repair any incorrect grouping) after the import is complete, which in theory should not be necessary.  So, if that commits breaks this is an indication that there is a logic flaw in the XLS import filter code with respect to formula grouping.

I don't have bandwidth to work on bug fixing for libreoffice these days, so I'll have to pass the torch to someone else for this one. Sorry. But hopefully this will be a helpful tip for anyone willing to dive in.
Comment 7 Timur 2021-09-18 05:50:29 UTC
Thanks for explanation. 
General note: bug started as XLS but should also be checked in MSO, by opening XLS if correct and saving afaim as XLS aand XLSX, to see how that one opens in LO. 
Usually "Xls/x saved in Lo" are not proper samples because source ODS is needed. But they are of they open well in MSO and can be resaved there.
Comment 8 Timur 2021-09-24 11:00:14 UTC
MSO gives some error when opening XLS, but XLS and XLSX saved in MSO both crash in LO. So this sample should be valid for crash.
Comment 9 Commit Notification 2021-10-06 14:32:46 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e7ec79fe36a0f22f10167806da80e3c1f30b36e8

ScRangeList::UpdateReference() join all ranges properly (tdf#140901)

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 10 Xisco Faulí 2021-10-07 07:29:10 UTC
Verified in

Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: e090afc29bdff4303f1235080fb169011220be4a
CPU threads: 16; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: en-GB (en_GB); UI: en-US
Calc: threaded

@Luboš, thanks for fixing this issue!!
Comment 11 Xisco Faulí 2021-10-07 07:40:03 UTC
*** Bug 124836 has been marked as a duplicate of this bug. ***
Comment 12 Commit Notification 2021-10-07 07:43:54 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/d138dd209e99300c4fc0412f726b1058fbb0039d

ScRangeList::UpdateReference() join all ranges properly (tdf#140901)

It will be available in 7.2.3.

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 13 Commit Notification 2021-10-07 10:13:08 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/7f623ef99920e75af22eda95a616aa749f20d1d0

ScRangeList::UpdateReference() join all ranges properly (tdf#140901)

It will be available in 7.1.7.

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.