Closed Bug 1342360 Opened 4 years ago Closed 4 years ago

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


(Core :: Networking, defect)

51 Branch
Not set



Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox-esr52 53+ fixed
firefox53 + fixed
firefox54 + fixed
firefox55 + fixed


(Reporter: philipp, Assigned: michal)


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

Crash Data


(1 file)

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]
Attached patch fixSplinter Review
Assignee: nobody → michal.novotny
Attachment #8842845 - Flags: review?(valentin.gosu)
Whiteboard: [necko-active]
Attachment #8842845 - Flags: review?(valentin.gosu) → review+
[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.  :-)
Flags: needinfo?(michal.novotny)
Comment on attachment 8842845 [details] [diff] [review]

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?
Keywords: checkin-needed
> [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.
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8842845 [details] [diff] [review]

ESR52 likes easy stability fixes too :)
Attachment #8842845 - Flags: approval-mozilla-esr52?
Tracking for all branches based on Comment 2.
Comment on attachment 8842845 [details] [diff] [review]

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+
Comment on attachment 8842845 [details] [diff] [review]

null deref fix for esr52
Attachment #8842845 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
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.