Crash in mozilla::net::WyciwygChannelParent::ActorDestroy

RESOLVED FIXED in Firefox -esr52

Status

()

Core
Networking
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philipp, Assigned: michal)

Tracking

({crash})

51 Branch
mozilla55
crash
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox51 wontfix, firefox52 wontfix, firefox-esr5253+ fixed, firefox53+ fixed, firefox54+ fixed, firefox55+ fixed)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

a year ago
This bug was filed from the Socorro interface and is 
report bp-14782aef-89d0-45ac-8c96-478fb2170217.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::net::WyciwygChannelParent::ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) 	netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:42
1 	xul.dll 	mozilla::net::PRtspControllerParent::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) 	obj-firefox/ipc/ipdl/PRtspControllerParent.cpp:556
2 	xul.dll 	mozilla::net::PWyciwygChannelParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PWyciwygChannelParent.cpp:267
3 	xul.dll 	mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentParent.cpp:3402
4 	xul.dll 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp:1662
5 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message&&) 	ipc/glue/MessageChannel.cpp:1600
6 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp:1567
7 	nss3.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:328
8 	winmm.dll 	timeGetTime 	
9 	xul.dll 	mozilla::detail::RunnableMethodImpl<bool ( mozilla::ipc::MessageChannel::*)(void), 0, 1>::Run() 	obj-firefox/dist/include/nsThreadUtils.h:768
10 	xul.dll 	mozilla::ipc::MessageChannel::DequeueTask::Run() 	obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:559
11 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067

this cross-platform crash signature has been around for a while already. it spiked up though since 2017-02-15 to around 100 daily crash reports, some of the recent user comments mention the guardian newspaper site as source of the crashes.

some Correlations for Firefox Release:
(100.0% in signature vs 10.49% overall) address = 0x0
(99.75% in signature vs 55.77% overall) e10s_enabled = 1 [99.75% vs 36.52% if startup_crash = 0]
(34.48% in signature vs 07.85% overall) cpu_microcode_version = null [91.58% vs 24.41% if cpu_arch = amd64]
(68.23% in signature vs 28.83% overall) e10s_cohort = test [68.23% vs 18.37% if startup_crash = 0]
(Assignee)

Comment 1

a year ago
Created attachment 8842845 [details] [diff] [review]
fix
Assignee: nobody → michal.novotny
Attachment #8842845 - Flags: review?(valentin.gosu)
(Assignee)

Updated

a year ago
Whiteboard: [necko-active]
Attachment #8842845 - Flags: review?(valentin.gosu) → review+

Comment 2

a year ago
[Tracking Requested - why for this release]: This is #31 top crasher on beta, and it has an r+ed patch which adds a null check and is totally upliftable, can we land and uplift this please?  It would be a nice stability win.  :-)
tracking-firefox52: --- → ?
Flags: needinfo?(michal.novotny)
(Assignee)

Comment 3

a year ago
Comment on attachment 8842845 [details] [diff] [review]
fix

Approval Request Comment
[Feature/Bug causing the regression]: unknown
[User impact if declined]: possible crash
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no, exact STR are not known
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple null check
[String changes made/needed]: none
Flags: needinfo?(michal.novotny)
Attachment #8842845 - Flags: approval-mozilla-beta?
Attachment #8842845 - Flags: approval-mozilla-aurora?
(Reporter)

Updated

a year ago
Keywords: checkin-needed
(Assignee)

Comment 4

a year ago
> [Has the fix been verified in Nightly?]: yes

Sorry, I completely overlooked that I didn't land it. So it hasn't been verified on nightly.

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/503b627f4d4b
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox51: affected → wontfix
status-firefox52: affected → wontfix
status-firefox54: ? → affected
status-firefox-esr52: --- → affected
tracking-firefox52: ? → ---
tracking-firefox53: --- → ?
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
tracking-firefox-esr52: --- → ?
Comment on attachment 8842845 [details] [diff] [review]
fix

ESR52 likes easy stability fixes too :)
Attachment #8842845 - Flags: approval-mozilla-esr52?
Tracking for all branches based on Comment 2.
tracking-firefox53: ? → +
tracking-firefox54: ? → +
tracking-firefox55: ? → +
tracking-firefox-esr52: ? → 52+
Comment on attachment 8842845 [details] [diff] [review]
fix

Crash fix, let's go for it. This should land for the beta 9 build tomorrow.
Attachment #8842845 - Flags: approval-mozilla-beta?
Attachment #8842845 - Flags: approval-mozilla-beta+
Attachment #8842845 - Flags: approval-mozilla-aurora?
Attachment #8842845 - Flags: approval-mozilla-aurora+
tracking-firefox-esr52: 52+ → ?

Comment 10

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/9dcd8fb79239
status-firefox54: affected → fixed

Comment 11

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/f0bf3f31114a
status-firefox53: affected → fixed
Comment on attachment 8842845 [details] [diff] [review]
fix

null deref fix for esr52
Attachment #8842845 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
tracking-firefox-esr52: ? → 53+

Comment 13

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/416681a239ef
status-firefox-esr52: affected → fixed
Setting qe-verify- based on Michal's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 3).
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.