1443110 - Crash in nsDocument::InitCSP via nsHTMLDocument::StartDocumentLoad

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Wayne Mery (:wsmwk)]

I crashed  on win7 - bp-bbf15f66-e83e-41d7-89d6-4a39b0180305.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll nsDocument::InitCSP dom/base/nsDocument.cpp:2935
1 xul.dll nsDocument::StartDocumentLoad dom/base/nsDocument.cpp:2824
2 xul.dll nsHTMLDocument::StartDocumentLoad dom/html/nsHTMLDocument.cpp:583
3 xul.dll nsContentDLF::CreateDocument layout/build/nsContentDLF.cpp:364
4 xul.dll nsContentDLF::CreateInstance layout/build/nsContentDLF.cpp:186
5 xul.dll mozilla::mailnews::MailNewsDLF::CreateInstance C:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/mailnews/base/src/MailNewsDLF.cpp:69
6 xul.dll nsDocShell::NewContentViewerObj docshell/base/nsDocShell.cpp:8971
7 xul.dll nsDocShell::CreateContentViewer docshell/base/nsDocShell.cpp:8760
8 xul.dll nsDSURIContentListener::DoContent docshell/base/nsDSURIContentListener.cpp:196
9 xul.dll nsDocumentOpenInfo::TryContentListener uriloader/base/nsURILoader.cpp:766

=============================================================


THere are two other nightly crashes in past week
bp-742b4dfc-c532-493f-98a7-485530180303 Mac
bp-63997488-c8b3-42ce-b362-a76bc0180301 linux

Comment 1

[Alfred Peters [:infofrommozilla]]

Attachment: callstack.txt

| 2935 if (loadInfo->GetAllowDocumentToBeAgnosticToCSP()) {

I do see this crash in my private local build (Line 2939 here corresponds to line 2942).

STR:
General requirements:

This is a timing problem. That's why you need a news account with a relatively large group. I use a group of 140k articles.

Move the group to the top of the list.

If you select an article in that group, switch to another group and go back, the prior selected article should be shown.

Download all headers off that group.

Here we go:
1. Restart TB
2. Open that first group
3. Select any article
4. Select the Server
5. Type [F5] (On Windows that's 'Get New Messages for'->'Current account')
6. Immediately click into that Group (before TB steps to the next group)

Wait a few seconds. And when TB tries to show the old article: Crash

Comment 2

[Wayne Mery (:wsmwk)]

All three crashes from comment 0 are buildid 20180227030201	

Tony, is this something you can test on nightly?

Comment 3

[Wayne Mery (:wsmwk)]

#1 crash for nightly.  

Just crashed again in newsgroup bp-fffbf8e9-39c6-4107-8947-29d9e0180309 bp-fbd0787f-ad7b-4db4-94d8-8027a0180309 using newsgroups ~30k articles.  So I'd say this is semi-reproducible using comment 1. And it must be a regression. I haven't attempted to test builds older than 20180227030201

Alfred, can you see if slightly older buildids also crash - perhaps even find the "oldest" one that crashes?

Or perhaps something on the stack rings a bell with Jorg.

Comment 4

[Jorg K (CEST = GMT+2)]

Well, the stack shows mainly M-C code. According to the crash IDs from the previous comment, it crashes in dom/base/nsDocument.cpp:2935 in code introduced here on 2018-02-18:
https://hg.mozilla.org/mozilla-central/annotate/bd6e200b5a6b2f9e5a9b31fcc92285aa7fc9afcc/dom/base/nsDocument.cpp#l2935
https://hg.mozilla.org/mozilla-central/rev/93dcf59ff87e#l2.15

So we should ask the author and reviewer of this code ;-)

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1443110_loadinfo_null_check.patch

mozilla-central should not contain any channels that do not have a loadinfo attached to the channel, so I assume that channel must originate somehwere within TB code. Anyway, adding a nullcheck fixes the problem.

Comment 6

[Alfred Peters [:infofrommozilla]]

(In reply to Wayne Mery (:wsmwk) from comment #3)
> #1 crash for nightly.  
> 
> Just crashed again in newsgroup bp-fffbf8e9-39c6-4107-8947-29d9e0180309
> bp-fbd0787f-ad7b-4db4-94d8-8027a0180309 using newsgroups ~30k articles.  So
> I'd say this is semi-reproducible using comment 1.

Yes, I also see that in normal reading. But only rarely. However, it always occurs when the newsgroup is updated and an article is to be displayed.

>  And it must be a
> regression. I haven't attempted to test builds older than 20180227030201
> 
> Alfred, can you see if slightly older buildids also crash - perhaps even
> find the "oldest" one that crashes?

I've a gap in my builds. But with official dailies I found: 
First bad:
Thunderbird (60.0a1) - Thunderbird Daily BuildID=20180219030201
Rev: Comm-Central:2d16fee9404f / Mozilla-Central:d0d3693d9bef

Last good:
Thunderbird (60.0a1) - Thunderbird Daily BuildID=20180218030201
Rev: Comm-Central:8f403ab50774 / Mozilla-Central:e7438140bb20

Comment 7

[Alfred Peters [:infofrommozilla]]

(In reply to Alfred Peters from comment #6)
> First bad:
> Thunderbird (60.0a1) - Thunderbird Daily BuildID=20180219030201
> Rev: Comm-Central:2d16fee9404f / Mozilla-Central:d0d3693d9bef
> Last good:
> Thunderbird (60.0a1) - Thunderbird Daily BuildID=20180218030201
> Rev: Comm-Central:8f403ab50774 / Mozilla-Central:e7438140bb20

(In reply to Jorg K (GMT+1) from comment #4)
> https://hg.mozilla.org/mozilla-central/rev/93dcf59ff87e#l2.15

This change from 01 Feb 2018 lies in between. So nothing new. :-)

Comment 8

[Wayne Mery (:wsmwk)]

Thanks all, and Alfred, for the quick action.

I crashed again and wasn't even trying bp-3ac88ead-c6ba-4c30-a144-b2e170180309. So I have no doubt this would become a topcrash in a 60 beta.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8957548 [details] [diff] [review]
bug_1443110_loadinfo_null_check.patch

Looks like we do null check elsewhere in the method too.

Comment 10

[Wayne Mery (:wsmwk)]

Jorg, is there likely something in c-c code that needs fixing that should have a followup bug?

Comment 11

[Jorg K (CEST = GMT+2)]

No.

Comment 12

[Julien Cristau [:jcristau]]

No firefox crashes here as far as I can tell.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25ade148226d
Add NullCheck for loadinfo within InitCSP. r=smaug

Comment 14

[Wayne Mery (:wsmwk)]

(In reply to Julien Cristau [:jcristau] from comment #12)
> No firefox crashes here as far as I can tell.

correct, a topcrash only for Thunderbird

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8957548 [details] [diff] [review]
bug_1443110_loadinfo_null_check.patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1432358.
[User impact if declined]: Thunderbird topcrash :-((
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: Only this.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Simple null check was added.
[String changes made/needed]: None.

The patch adds a simple null check to avoid a topcrash seen in Thunderbird. The patch just missed the merge date by a few hours.

Comment 16

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/25ade148226d

Comment 17

[Wayne Mery (:wsmwk)]

(In reply to Jorg K (GMT+1) from comment #15)
> Comment on attachment 8957548 [details] [diff] [review]
> bug_1443110_loadinfo_null_check.patch
> 
> Approval Request Comment
> [Feature/Bug causing the regression]: Bug 1432358.
> [User impact if declined]: Thunderbird topcrash :-((
> [Is this code covered by automated tests?]: No.
> [Has the fix been verified in Nightly?]: No.
If it matters, I can test nightly build when it becomes available.  But more importantly...

> ...
> The patch adds a simple null check ...

... which already exists in another section of code (comment 9)

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 8957548 [details] [diff] [review]
bug_1443110_loadinfo_null_check.patch

null check to fix thunderbird crash, beta60+

Comment 19

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e96ddf050a56