Closed Bug 1443110 Opened 2 years ago Closed 2 years ago

Crash in nsDocument::InitCSP via nsHTMLDocument::StartDocumentLoad

Categories

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

60 Branch
Unspecified
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 - fixed
firefox61 --- fixed

People

(Reporter: wsmwk, Assigned: ckerschb)

References

Details

(Keywords: crash, regression, topcrash-thunderbird, Whiteboard: [regression:TB60])

Crash Data

Attachments

(2 files)

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
Attached file 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
All three crashes from comment 0 are buildid 20180227030201	

Tony, is this something you can test on nightly?
Flags: needinfo?(antoine.mechelynck)
Keywords: regression
Whiteboard: [regression:TB5?]
#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.
Flags: needinfo?(jorgk)
Flags: needinfo?(infofrommozilla)
OS: Windows 7 → All
Summary: Crash in nsDocument::InitCSP → Crash in nsDocument::InitCSP via nsHTMLDocument::StartDocumentLoad
Whiteboard: [regression:TB5?] → [regression:TB60?]
Version: 58 Branch → 60
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 ;-)
Flags: needinfo?(jorgk)
Flags: needinfo?(ckerschb)
Flags: needinfo?(bugs)
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.
Flags: needinfo?(ckerschb)
Flags: needinfo?(bugs)
Attachment #8957548 - Flags: review?(bugs)
(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
Flags: needinfo?(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. :-)
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.
Flags: needinfo?(antoine.mechelynck)
Whiteboard: [regression:TB60?] → [regression:TB60]
Comment on attachment 8957548 [details] [diff] [review]
bug_1443110_loadinfo_null_check.patch

Looks like we do null check elsewhere in the method too.
Attachment #8957548 - Flags: review?(bugs) → review+
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Keywords: checkin-needed
Component: General → DOM
Product: Thunderbird → Core
Version: 60 → 60 Branch
Jorg, is there likely something in c-c code that needs fixing that should have a followup bug?
Flags: needinfo?(jorgk)
No.
Flags: needinfo?(jorgk)
No firefox crashes here as far as I can tell.
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25ade148226d
Add NullCheck for loadinfo within InitCSP. r=smaug
Keywords: checkin-needed
(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 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.
Attachment #8957548 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/25ade148226d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(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 on attachment 8957548 [details] [diff] [review]
bug_1443110_loadinfo_null_check.patch

null check to fix thunderbird crash, beta60+
Attachment #8957548 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.