1342360 - Crash in mozilla::net::WyciwygChannelParent::ActorDestroy

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[[:philipp]]

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]

Comment 1

[Michal Novotny [:michal]]

Attachment: fix

Comment 2

[(no longer active)]

[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.  :-)

Comment 3

[Michal Novotny [:michal]]

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

Comment 4

[Michal Novotny [:michal]]

> [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 5

[Iris Hsiao [:ihsiao]]

pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/503b627f4d4b44ad0cf494491f9e0bb7b6606007

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/503b627f4d4b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8842845 [details] [diff] [review]
fix

ESR52 likes easy stability fixes too :)

Comment 8

[Marcia Knous [:marcia]]

Tracking for all branches based on Comment 2.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8842845 [details] [diff] [review]
fix

Crash fix, let's go for it. This should land for the beta 9 build tomorrow.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9dcd8fb79239

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8842845 [details] [diff] [review]
fix

null deref fix for esr52

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/416681a239ef

Comment 14

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Michal's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 3).