1303967 - Crash in mozalloc_abort | NS_DebugBreak | mozilla::ipc::LogicError | mozilla::dom::PBrowser::Transition

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Ting-Yu Chou [:ting] (away)]

This bug was filed from the Socorro interface and is 
report bp-0fce5ec5-fa63-4446-bcc6-277b62160920.
=============================================================

#6 of 0918 Nightly on Windows, 89 crashes from 50 installations.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

[Tracking Requested - why for this release]:
Over 100 crashes on 51 as well so far.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

Here's the relevant part of the crash stack:

4 	xul.dll 	mozilla::dom::PBrowserChild::SendForcePaintNoOp(unsigned __int64 const&) 	obj-firefox/ipc/ipdl/PBrowserChild.cpp:2452
5 	xul.dll 	mozilla::dom::TabChild::RecvSetDocShellIsActive(bool const&, bool const&, unsigned __int64 const&) 	dom/ipc/TabChild.cpp:2663
6 	xul.dll 	mozilla::dom::TabChild::ForcePaint(unsigned __int64)
...
29 	xul.dll 	nsDocShellTreeOwner::AddChromeListeners() 	embedding/browser/nsDocShellTreeOwner.cpp:976
30 	xul.dll 	nsWebBrowser::Create() 	embedding/browser/nsWebBrowser.cpp:1286
31 	xul.dll 	mozilla::dom::TabChild::Init() 	dom/ipc/TabChild.cpp:762

So we're getting an interrupt callback too soon--before the TabChild has been initialized. When we try to send an IPC message, we crash.

I think it makes sense for mIPCOpen to be false until we can actually send messages. That's how it's typically used. So I delayed setting it to true until TabChild::Init finishes.

I also added an IPCOpen() check in TabChild::ForcePaint. I don't think it makes sense to do any of that stuff if it's false.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a536a062f1d9
Don't set mIPCOpen until TabChild is initialized (r=mrbkap)

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8793104 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1279086
[User impact if declined]: crashes
[Describe test coverage new/current, TreeHerder]: will be on m-c soon
[Risks and why]: pretty low risk, just avoiding some bad code
[String/UUID change made/needed]: none

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a536a062f1d9

Comment 6

[Gerry Chang [:gchang]]

Comment on attachment 8793104 [details] [diff] [review]
patch

Review of attachment 8793104 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes a crash. Take it in 51 aurora.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1456ca7b0683

Comment 8

[Gerry Chang [:gchang]]

Track for 51+ as it has high volume of crash

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature 'mozalloc_abort | NS_DebugBreak | mozilla::ipc::LogicError | mozilla::dom::PBrowser::Transition':
 - nightly (version 52): 260 crashes from 2016-09-19.
 - aurora  (version 51): 404 crashes from 2016-09-19.
 - beta    (version 50): 0 crashes from 2016-09-20.
 - release (version 49): 3 crashes from 2016-09-05.
 - esr     (version 45): 0 crashes from 2016-06-01.

Crash volume on the last weeks (Week N is from 10-03 to 10-09):
            W. N-1  W. N-2
 - nightly      24     236
 - aurora       93     311
 - beta          0       0
 - release       2       1
 - esr           0       0

Affected platforms: Windows, Linux

Crash rank on the last 7 days:
           Browser   Content     Plugin
 - nightly           #49
 - aurora  #678      #16
 - beta
 - release           #3686
 - esr