Bug Hunting Session
Bug 122020 - CRASH in SvTreeList::InvalidateEntry(SvTreeListEntry *)
Summary: CRASH in SvTreeList::InvalidateEntry(SvTreeListEntry *)
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.2 all versions
Hardware: All All
: highest critical
Assignee: Caolán McNamara
URL:
Whiteboard: target:6.3.0 target:6.2.0.2 target:6.1.5
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks:
 
Reported: 2018-12-11 12:16 UTC by Xisco Faulí
Modified: 2019-01-10 08:54 UTC (History)
4 users (show)

See Also:
Crash report or crash signature: ["SvTreeList::InvalidateEntry(SvTreeListEntry%20*)"]


Attachments
gdb backtrace (24.19 KB, text/plain)
2018-12-11 18:04 UTC, Xisco Faulí
Details
backtrace - closing Libreoffice (50.61 KB, text/plain)
2018-12-13 14:44 UTC, Xisco Faulí
Details
gdb bt (16.88 KB, text/plain)
2018-12-15 15:56 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2018-12-11 12:16:16 UTC
Steps to reproduce:
1. Unzip attachment 146893 [details] from bug 121606
2. Add the database to the Registered Databases
3. Make sure Data Sources is displayed ( View - Data sources )
4. Open FieldsFrom_DataOnTwoSheets_PW_Test1234.odt -> authentication dialog is prompted
5. Cancel it or click Ok with the wrong credentials and Ok again
-> Crash!

Reproduced in

Version: 6.3.0.0.alpha0+
Build ID: 0ad2302cf6787cacbbaca081a890a0e356a55297
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded
Comment 1 Xisco Faulí 2018-12-11 12:41:00 UTC
Regression introduced by:

author	Ras-al-Ghul <dipankar1995@gmail.com>	2016-01-07 22:26:34 +0530
committer	Michael Meeks <michael.meeks@collabora.com>	2016-01-08 21:24:16 +0000
commit	47bac0de19fc4ca2c9d469b64fcbffe15bc4a0a3 (patch)
tree	b46bbbcb7eeb0c405d1445150a221e0f6c44c895
parent	774fb6d2e7cf36b677e66c54278225b1256bd40f (diff)
tdf#96888 Kill internal vcl dog-tags ...

Bisected with: bibisect-linux-64-5.2
Comment 2 Xisco Faulí 2018-12-11 18:04:41 UTC
Created attachment 147448 [details]
gdb backtrace
Comment 3 Xisco Faulí 2018-12-11 18:19:48 UTC
Hi Michael, this is related to bug 96888. I thought you could be interested in this issue...
Comment 4 Michael Meeks 2018-12-12 09:23:24 UTC
https://gerrit.libreoffice.org/65005 tdf#122020 - avoid nullptr deref.

Any chance of testing that & chasing it in Xisco ? =) The good news is that holding a VclPtr here turns this from a free-memory-read/write to a simple nullptr de-reference that we can correctly fix =) so ... progress really.

Thanks !
Comment 5 Xisco Faulí 2018-12-12 11:09:16 UTC Comment hidden (obsolete)
Comment 6 Xisco Faulí 2018-12-12 11:17:02 UTC
Hi again,
Actually forget my previous comment about the hang... it no longer hangs with a clean profile.
So the crash is no longer reproducible in this case:

1. Unzip attachment 146893 [details] from bug 121606
2. Add the database to the Registered Databases
3. Make sure Data Sources is displayed ( View - Data sources )
4. Open FieldsFrom_DataOnTwoSheets_PW_Test1234.odt -> authentication dialog is prompted
5. Cancel it
6. Close LibreOffice

but it is with

1. Unzip attachment 146893 [details] from bug 121606
2. Add the database to the Registered Databases
3. Make sure Data Sources is displayed ( View - Data sources )
4. Open FieldsFrom_DataOnTwoSheets_PW_Test1234.odt -> authentication dialog is prompted
5. Click Ok with the wrong credentials
6. Click Ok on the new dialog
7. Close LibreOffice
-> Crash!
Comment 7 Michael Meeks 2018-12-12 17:12:05 UTC
Would to be good to get the commit in if you can (from a gerrit button pressing perspective) - and to get a new trace for the next crash =)

Thanks !
Comment 8 Xisco Faulí 2018-12-12 17:23:19 UTC
(In reply to Michael Meeks from comment #7)
> Would to be good to get the commit in if you can (from a gerrit button
> pressing perspective) - and to get a new trace for the next crash =)
> 
> Thanks !

Sure, good idea! pushing it now. probably I can get tomorrow a new backtrace with a daily build...
Comment 9 Commit Notification 2018-12-12 17:24:04 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/27995e638e1582b443befa93bc5dfd5970a38ef2%5E%21

tdf#122020 - avoid nullptr deref.

It will be available in 6.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í 2018-12-13 14:44:30 UTC
Created attachment 147502 [details]
backtrace -  closing Libreoffice

Hi Michael,
this is the backtrace from the crash while LibreOffice is being closed
Comment 11 Julien Nabet 2018-12-15 15:56:45 UTC
Created attachment 147578 [details]
gdb bt

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Since bt is different, I thought it may help here.
Comment 12 Caolán McNamara 2018-12-20 21:29:23 UTC
what I see is a PostUserEvent of DBTreeListBox::OnResetEntry with a SvTreeListEntry* pEntry as payload, then the DBTreeListBox is disposed and then the UserEvent arrives and the, by now deleted, pEntry is processed by the disposed DBTreeListBox
Comment 13 Caolán McNamara 2018-12-20 21:39:31 UTC
https://gerrit.libreoffice.org/#/c/65512/
Comment 14 Michael Meeks 2018-12-20 22:16:04 UTC
Fair enough. I had hoped that the PostUserEvent - which (these days) can hold a VclPtr on the widget itself - would be able to survive in a valid but disposed state just nicely; (the true bReferenceLink to PostUserEvent) - so checking for isDisposed() or whatever might fly there. But of course cleaning up the event is cool tool.

Thanks Caolan !
Comment 15 Xisco Faulí 2018-12-20 23:20:25 UTC
Hi Caolán, Michael,
Thank you both for providing patches to fix this issue.
Seeing Caolán's patch is already cherry-picked to 6-2 and 6-1 in gerrit, i'm wondering, should michael's patch be cherry-picked as well?
Comment 16 Julien Nabet 2018-12-21 08:03:56 UTC
(In reply to Xisco Faulí from comment #15)
> Hi Caolán, Michael,
> Thank you both for providing patches to fix this issue.
> Seeing Caolán's patch is already cherry-picked to 6-2 and 6-1 in gerrit, i'm
> wondering, should michael's patch be cherry-picked as well?

Considering https://gerrit.libreoffice.org/#/c/65515/, I don't think so.
Comment 17 Commit Notification 2018-12-21 12:18:23 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/b2e7e5ac8d511ada9e254d86b2f16509d4b9b977%5E%21

tdf#122020 crash in SvTreeList::InvalidateEntry

It will be available in 6.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 18 Commit Notification 2018-12-21 12:18:32 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-2":

https://git.libreoffice.org/core/+/404df1bcab4419bc66cafca355d8ed5cd0f07d59%5E%21

tdf#122020 crash in SvTreeList::InvalidateEntry

It will be available in 6.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.
Comment 19 Caolán McNamara 2018-12-21 12:24:30 UTC
backport to 6-1 in gerrit.

yeah, the DBTreeListBox itself survives fine due to the VclPtr, but the SvTreeListEntry* pEntry arg doesn't and is bogus after the dispose. If the operations on it were moved inside the GetModel() check that would presumably have worked fine too.
Comment 20 Xisco Faulí 2018-12-24 14:39:10 UTC
No longer crashes in

Version: 6.3.0.0.alpha0+
Build ID: 9520378e37b97b0a44130c86be482060465b479e
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Caolán, thanks for fixing this!!
Comment 21 Commit Notification 2019-01-10 08:54:54 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

https://git.libreoffice.org/core/+/e9febe48283bf376ecf06cd688e8cef087a41a35%5E%21

tdf#122020 crash in SvTreeList::InvalidateEntry

It will be available in 6.1.5.

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.