Crash in mozalloc_abort | NS_DebugBreak | mozilla::ipc::FatalError | mozilla::net::PNeckoChild::SendPWebSocketConstructor

RESOLVED FIXED in Firefox 54

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: marcia, Assigned: junior)

Tracking

({crash})

55 Branch
mozilla55
Unspecified
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [necko-active], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

This bug was filed from the Socorro interface and is 
report bp-f7642eda-91c8-4cb0-86ed-3b4502170328.
=============================================================

Seen while looking at nightly crash stats - feel feel to move to correct component - crashes started on Nightly using 20170308030207 - http://bit.ly/2oMCOqg 

No useful correlations or comments.
Component: IPC → Networking
¡Hola!

Yup! This bite me on Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0 ID:20170426030329 CSet: 0f5ba06c4c5959030a05cb852656d854065e2226 in the form of bp-9c6752f7-a4b6-470d-b67c-992f10170427 

Subscribing...

¡Gracias!
Alex
Shian-Yow - do we have anyone in Taipei who has e10s and/or websockets knowledge that could take a look at this? I've already bugged Michal on a few things lately (my usual websockets go-to), and don't want to overload him :)
Flags: needinfo?(swu)
(In reply to Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) from comment #2)
> Shian-Yow - do we have anyone in Taipei who has e10s and/or websockets
> knowledge that could take a look at this? I've already bugged Michal on a
> few things lately (my usual websockets go-to), and don't want to overload
> him :)

I would like Junior to take a look at this. :)
Assignee: nobody → juhsu
Flags: needinfo?(swu)
Whiteboard: [necko-active]
It has the pattern in Bug 1332054, failed in constructor in a shutdown progress.
Whiteboard: [necko-active]
Whiteboard: [necko-active]
(In reply to Junior[:junior] from comment #4)
> It has the pattern in Bug 1332054, failed in constructor in a shutdown
> progress.

Trying to create IPDL during shutdown phase doesn't seems something we should do. Is it triggered by test case or something else?
Flags: needinfo?(juhsu)
I did a quick investigation that we don't have a intermittent for this before.

Comment 4 is judged by
1. It's triggered from child process
2. Most of them happened in ShutdownProgress:profile-before-change
Flags: needinfo?(juhsu)
FWIW, It's interested that the uptime ranges are < 1min
Posted patch websocket-crash - v1 (obsolete) — Splinter Review
The FatalError occurs in 
PNeckoChild::SendPWebSocketConstructor() ->
(GetIPCChannel())->Send(msg__);

And all the cases are in shutdown progress.

Similar to Bug 1277582, we need to have a bail-out.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bcf9e9366bb8524873061399c4353c5c4a264926
Attachment #8863697 - Flags: review?(hurley)
Comment on attachment 8863697 [details] [diff] [review]
websocket-crash - v1

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

Nit on the commit message: use r=nwgh (or r=hurley, if you're old-school). "nick" is a bit too ambiguous :)

::: netwerk/protocol/websocket/WebSocketChannelChild.cpp
@@ +556,5 @@
>  
> +  ContentChild* cc = static_cast<ContentChild*>(gNeckoChild->Manager());
> +  if (cc->IsShuttingDown()) {
> +    return NS_ERROR_FAILURE;
> +  }

So like in bug 1277582, this should probably be moved above the AddIPDLReference() call further up in the method. Otherwise, this is good to go.
Attachment #8863697 - Flags: review?(hurley)
Also, :billm mentioned over in bug 1277582 that there needs to be a way to prevent this from happening globally. Bill, did that bug ever get filed?
Flags: needinfo?(wmccloskey)
No, although bug 1332054 would probably be that bug. I need to get back to that at some point.
Flags: needinfo?(wmccloskey)
Thanks for your prompt feedback!
Attachment #8863697 - Attachment is obsolete: true
Attachment #8863967 - Flags: review?(hurley)
Comment on attachment 8863967 [details] [diff] [review]
websocket-crash - v2

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

LGTM, thanks!
Attachment #8863967 - Flags: review?(hurley) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/816f2cd29ba6
Ensure that the WebSocket not sending constructor when shutting down, r=nwgh
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/816f2cd29ba6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Beta approval on this when you get a chance. While ESR52 also appears to be affected, the crash rate is miniscule, so I'm setting the status to wontfix. Feel free to change it to affected and nominate the patch for approval if you feel strongly that it warrants consideration for that branch as well.
Flags: needinfo?(juhsu)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)
> Please request Beta approval on this when you get a chance. While ESR52 also
> appears to be affected, the crash rate is miniscule, so I'm setting the
> status to wontfix. Feel free to change it to affected and nominate the patch
> for approval if you feel strongly that it warrants consideration for that
> branch as well.

It's a shut-down crash. IMO it's a wontfix to ESR
Flags: needinfo?(juhsu)
Comment on attachment 8863967 [details] [diff] [review]
websocket-crash - v2

Approval Request Comment
[Feature/Bug causing the regression]: No bug
[User impact if declined]: Have shut-down crashes
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: Not risky
[Why is the change risky/not risky?]: It's a shut-down check and we already have some similar fix
[String changes made/needed]: No
Attachment #8863967 - Flags: approval-mozilla-beta?
Comment on attachment 8863967 [details] [diff] [review]
websocket-crash - v2

Fix a crash. Beta54+. Should be in 54 beta 7.
Attachment #8863967 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1363659
You need to log in before you can comment on or make changes to this bug.