Bug Hunting Session
Bug 114479 - Crash when loading an ods file when threading is enabled
Summary: Crash when loading an ods file when threading is enabled
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.1.0 target:6.0.0.1
Keywords:
Depends on:
Blocks: Calc-Threaded
  Show dependency treegraph
 
Reported: 2017-12-15 05:32 UTC by Dennis Francis
Modified: 2018-06-21 11:09 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
assert failure at ScDocument::GetFormatTable() (backtrace) (4.05 KB, text/plain)
2017-12-15 05:34 UTC, Dennis Francis
Details
TextSearch object sharing b/w threads (backtrace) (4.23 KB, text/plain)
2017-12-15 05:35 UTC, Dennis Francis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dennis Francis 2017-12-15 05:32:03 UTC
Description:
The LO build from master crashes on loading https://bugs.documentfoundation.org/attachment.cgi?id=120128 when formula group threading is enabled in calc.

Steps to Reproduce:
1. Open the file https://bugs.documentfoundation.org/attachment.cgi?id=120128 with calc with threading enabled.

Actual Results:  
Calc crashes on loading

Expected Results:
No crash


Reproducible: Always


User Profile Reset: No



Additional Info:
There are multiple issues with this document from a threading perspective :

1) assert() fails inside ScDocument::GetFormatTable()

2) A SUMIF() implicit range problem.

The offending SUMIF here in the document is :

=SUMIF($ResPer.$G$5:$G$593,"^"&D272,ResPer.$R$5)+SUMIF($ResPer.$G$5:$G$
593,"^"&D272,ResPer.$S$5)+SUMIF($ResPer.$G$5:$G$593,"^"&D272,ResPer.$AD
$5)+SUMIF($ResPer.$G$5:$G$593,"^"&D272,ResPer.$AE$5)


Notice that the third arg to each SUMIF is just a single ref, but
during evaluation, it will be implicitly expanded to the length/size of
the first argument doubleref.

The ScDependantsCalculator::Doit() simply pre-interprets all singlerefs
and doublerefs in the token array before any threading begins, hence
misses this special case requirement of SUMIF.

3) Apart from this, we need to blacklist MATCH() opcode for threading as this one leads to calling of the slot machine like VLOOKUP does.

4) Finally there is the problem of sharing the TextSearch object
between threads without any mutex protection that happens on SUMIF(S), COUNTIF(S) or anything that involves a call to ScInterpreter::IterateParametersIfs().


User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36
Comment 1 Dennis Francis 2017-12-15 05:34:43 UTC
Created attachment 138455 [details]
assert failure at ScDocument::GetFormatTable() (backtrace)
Comment 2 Dennis Francis 2017-12-15 05:35:58 UTC
Created attachment 138456 [details]
TextSearch object sharing b/w threads (backtrace)
Comment 3 Commit Notification 2017-12-15 09:36:18 UTC
Dennis Francis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#114479 : Blacklist MATCH() for threading

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 4 Commit Notification 2017-12-15 11:52:56 UTC
Dennis Francis committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

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

tdf#114479 : Blacklist MATCH() for threading

It will be available in 6.0.0.1.

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 5 Commit Notification 2017-12-19 13:10:42 UTC
Dennis Francis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#114479 : Use the SvNumberFormatter from ScInterpreterContext

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 6 Commit Notification 2017-12-19 14:56:29 UTC
Dennis Francis committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

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

tdf#114479 : Use the SvNumberFormatter from ScInterpreterContext

It will be available in 6.0.0.1.

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 7 Xisco Faulí 2018-01-19 10:04:43 UTC
A polite ping to Dennis Francis: is this bug fixed? if so, could you
please close it as RESOLVED FIXED ? Thanks
Comment 8 Dennis Francis 2018-01-19 10:15:46 UTC
(In reply to Xisco Faulí from comment #7)
> A polite ping to Dennis Francis: is this bug fixed? if so, could you
> please close it as RESOLVED FIXED ? Thanks

This is not yet fixed completely, 2 out of the 4 issues with this sheet are still pending. I'm putting this back to NEW in case someone else has time to fix this sooner than me. If not I'll get back to this in a couple of weeks. Thanks.
Comment 9 Commit Notification 2018-04-28 11:34:56 UTC
Dennis Francis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#114479: compute implicit sum ranges for ocSumIf,ocAverageIf...

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 10 Xisco Faulí 2018-05-29 09:35:40 UTC
A polite ping to Dennis Francis:
Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Otherwise, Could you please explain what's missing?
Thanks
Comment 11 Dennis Francis 2018-05-29 10:28:59 UTC
(In reply to Xisco Faulí from comment #10)
> A polite ping to Dennis Francis:
> Is this bug fixed? if so, could you please close it as RESOLVED FIXED ?
> Otherwise, Could you please explain what's missing?
> Thanks

The 4th issue mentioned in comment#1 is yet to be fixed. All others are done.
Comment 12 Dennis Francis 2018-05-29 10:30:26 UTC
(In reply to Dennis Francis from comment #11)
> (In reply to Xisco Faulí from comment #10)
> > A polite ping to Dennis Francis:
> > Is this bug fixed? if so, could you please close it as RESOLVED FIXED ?
> > Otherwise, Could you please explain what's missing?
> > Thanks
> 
> The 4th issue mentioned in comment#1 is yet to be fixed. All others are done.

Actually it is mentioned in the description, not comment#1.
Comment 13 Dennis Francis 2018-06-21 11:09:18 UTC
I can't reproduce the last issue (#4) anymore. Marking as resolved fixed for now.