If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Crash in mozilla::MediaManager::PostTask

RESOLVED FIXED in Firefox 56

Status

()

Core
WebRTC
P2
critical
Rank:
13
RESOLVED FIXED
13 days ago
a day ago

People

(Reporter: philipp, Assigned: mchiang)

Tracking

({crash, regression})

56 Branch
mozilla57
All
Windows
crash, regression
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 wontfix, firefox56 fixed, firefox57 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

13 days ago
This bug was filed from the Socorro interface and is 
report bp-74786dee-0a1e-4abe-9714-9047a0170912.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::MediaManager::PostTask(already_AddRefed<mozilla::Runnable>) 	dom/media/MediaManager.cpp:2001
1 	xul.dll 	mozilla::MediaManager::EnumerateRawDevices(unsigned __int64, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool) 	dom/media/MediaManager.cpp:1789
2 	xul.dll 	<lambda_8f9699fc120d46dcab1291f5e26e5bf6>::operator() 	dom/media/MediaManager.cpp:2077
3 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1446
4 	xul.dll 	nsThread::Shutdown() 	xpcom/threads/nsThread.cpp:1045
5 	xul.dll 	mozilla::ProcessHangMonitor::~ProcessHangMonitor() 	dom/ipc/ProcessHangMonitor.cpp:1132
6 	xul.dll 	mozilla::ProcessHangMonitor::`scalar deleting destructor'(unsigned int) 	
7 	xul.dll 	mozilla::ProcessHangMonitor::Release() 	dom/ipc/ProcessHangMonitor.cpp:1146
8 	xul.dll 	RefPtr<mozilla::ProcessHangMonitor>::~RefPtr<mozilla::ProcessHangMonitor>() 	obj-firefox/dist/include/mozilla/RefPtr.h:78
9 	xul.dll 	`anonymous namespace'::HangMonitorParent::~HangMonitorParent 	dom/ipc/ProcessHangMonitor.cpp:606
10 	xul.dll 	`anonymous namespace'::HangMonitorParent::`scalar deleting destructor'(unsigned int) 	
11 	xul.dll 	mozilla::ProcessHangMonitor::RemoveProcess(mozilla::PProcessHangMonitorParent*) 	dom/ipc/ProcessHangMonitor.cpp:1295
12 	xul.dll 	mozilla::dom::ContentParent::ActorDestroy(mozilla::ipc::IProtocol::ActorDestroyReason) 	dom/ipc/ContentParent.cpp:1701
13 	xul.dll 	mozilla::dom::PContentParent::OnChannelError() 	obj-firefox/ipc/ipdl/PContentParent.cpp:8437
14 	xul.dll 	mozilla::dom::ContentParent::OnChannelError() 	dom/ipc/ContentParent.cpp:1543
15 	xul.dll 	mozilla::ipc::MessageChannel::NotifyMaybeChannelError() 	ipc/glue/MessageChannel.cpp:2555
16 	xul.dll 	mozilla::ipc::MessageChannel::OnNotifyMaybeChannelError() 	ipc/glue/MessageChannel.cpp:2586
17 	xul.dll 	mozilla::detail::RunnableMethodImpl<mozilla::ipc::MessageChannel* const, void ( mozilla::ipc::MessageChannel::*)(void), 0, 1>::Run() 	xpcom/threads/nsThreadUtils.h:1172
18 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1446
19 	xul.dll 	XPTC__InvokebyIndex 	xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64.asm:97
20 	xul.dll 	JSScript::maybeSweepTypes(js::AutoClearTypeInferenceStateOnOOM*) 	js/src/vm/TypeInference.cpp:4431

this crash signature is regressing in volume during the 56.0b cycle:
https://crash-stats.mozilla.com/signature/?product=Firefox&release_channel=beta&signature=mozilla%3A%3AMediaManager%3A%3APostTask&date=%3E%3D2017-06-13T09%3A41%3A27.000Z#graphs
Looks like OnDeviceChange is calling EnumerateRawDevices in shutdown.

Munro, could you take a look?
Flags: needinfo?(mchiang)
(Assignee)

Updated

13 days ago
Assignee: nobody → mchiang
Flags: needinfo?(mchiang)
Simple fix would be to leak the task and not crash (but complain).  More complex (perhaps not very) would be to return an error and let the caller clean up correctly.  

It's interesting that it spiked in intensity.

P1/13 for the simple patch, which is also upliftable.
Rank: 13
Priority: -- → P1
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Comment hidden (mozreview-request)

Comment 5

12 days ago
mozreview-review
Comment on attachment 8907918 [details]
Bug 1399395 - skip devicechange event handler in shutdown stage.

https://reviewboard.mozilla.org/r/179596/#review184850

This seems all right to fix the spike. Could you also file a bug to fix PostTask per comment 2?
Attachment #8907918 - Flags: review?(apehrson) → review+
(Assignee)

Comment 6

12 days ago
I have filed Bug 1399770 to follow this.
(Assignee)

Updated

12 days ago
Keywords: checkin-needed
See Also: → bug 1399770

Comment 7

12 days ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ec3c4234c98a
skip devicechange event handler in shutdown stage. r=pehrsons
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ec3c4234c98a
Status: NEW → RESOLVED
Last Resolved: 12 days ago
status-firefox57: ? → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Reporter)

Comment 9

11 days ago
this seems to be a fairly simple fix. could you request an uplift to 56 (which is mozilla-release by now) if you deem fit to do so?
Flags: needinfo?(mchiang)
(Assignee)

Comment 10

11 days ago
Comment on attachment 8907918 [details]
Bug 1399395 - skip devicechange event handler in shutdown stage.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1364038
[User impact if declined]: content process may crash in some circumstance
[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
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: small change which fixes the crash
[String changes made/needed]: no
Flags: needinfo?(mchiang)
Attachment #8907918 - Flags: approval-mozilla-beta?
(Assignee)

Comment 11

11 days ago
Comment on attachment 8907918 [details]
Bug 1399395 - skip devicechange event handler in shutdown stage.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1364038
[User impact if declined]: content process may crash in some circumstance
[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
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: small change which fixes the crash
[String changes made/needed]: no
Attachment #8907918 - Flags: approval-mozilla-release?
status-firefox55: --- → wontfix
status-firefox-esr52: --- → unaffected
Comment on attachment 8907918 [details]
Bug 1399395 - skip devicechange event handler in shutdown stage.

Fix for a crash that got significantly worse in 56. 
Let's land this on m-r for the 56 RC build next Monday.
Attachment #8907918 - Flags: approval-mozilla-release?
Attachment #8907918 - Flags: approval-mozilla-release+
Attachment #8907918 - Flags: approval-mozilla-beta?
Attachment #8907918 - Flags: approval-mozilla-beta-

Comment 13

10 days ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/0bfaf562bf40 (FIREFOX_56b13_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/e5b95529a8f8
status-firefox56: affected → fixed
(In reply to Munro Mengjue Chiang [:mchiang] from comment #11)
> [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

Setting qe-verify- based on Munro Mengjue Chiang's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.