crash in mozilla::AbstractMediaDecoder::AutoNotifyDecoded::~AutoNotifyDecoded()

VERIFIED FIXED in Firefox 36

Status

()

defect
P1
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: marcia, Assigned: mattwoodrow)

Tracking

(Blocks 1 bug, {crash, regression})

37 Branch
mozilla37
All
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox36 verified, firefox37 verified)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-dca46c0a-b58c-4b52-9050-f138f2141230.
=============================================================

Seen while looking at crash stats. Link to all crashes: https://crash-stats.mozilla.com/report/list?productFirefox&signature=mozilla::AbstractMediaDecoder::AutoNotifyDecoded::~AutoNotifyDecoded%28%29

Started showing up with build ID=2014112503 and have continued up to 20141230030214. Not a high volume crash but all the URLs seem to be youtube.com. The majority of crashes appear to be Win 7.

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::AbstractMediaDecoder::AutoNotifyDecoded::~AutoNotifyDecoded() 	dom/media/AbstractMediaDecoder.h
1 	xul.dll 	mozilla::MP4Reader::Update(mp4_demuxer::TrackType) 	dom/media/fmp4/MP4Reader.cpp
2 	xul.dll 	nsRunnableMethodImpl<void ( mozilla::CDMProxy::*)(unsigned int), unsigned int, 1>::Run() 	xpcom/glue/nsThreadUtils.h
3 	xul.dll 	mozilla::MediaTaskQueue::Runner::Run() 	dom/media/MediaTaskQueue.cpp
4 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp
5 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
6 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
7 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
8 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
9 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
10 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp
11 	nss3.dll 	PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c
12 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c
13 	msvcr120.dll 	_callthreadstartex 	f:\dd\vctools\crt\crtw32\startup\threadex.c:376
14 	msvcr120.dll 	_threadstartex 	f:\dd\vctools\crt\crtw32\startup\threadex.c:354
15 	kernel32.dll 	BaseThreadInitThunk 	
16 	ntdll.dll 	RtlUserThreadStart 	
17 	kernel32.dll 	BasepReportFault 	
18 	kernel32.dll 	BasepReportFault
Assignee: nobody → matt.woodrow
This crash is still occurring as of yesterday's build id 2015010403.
Looks like we can get a callback to Update() after we've tried to shutdown and called ClearDecoder() (so mDecoder == nullptr).

I haven't been able to reproduce this, so not sure what else we can do other than preventing the crash in this case.
Attachment #8544265 - Flags: review?(karlt)
Comment on attachment 8544265 [details] [diff] [review]
Null check mDecoder

Yes, we need to null check.

MP4Reader::Update() has some other mDecoder dereferences.  It's not clear to
me from a quick look at the code that they will be avoided after reader
shutdown.  I also don't know how much of Update() needs to run if after reader shutdown.

(I thought about delaying setting mDecoder to null until the main thread task,
but that would just make things race.  Setting it to null on the decode task
queue seems the right place as that is the task queue where it will be accessed.)
Attachment #8544265 - Flags: review?(karlt) → review+
https://hg.mozilla.org/mozilla-central/rev/0aaec62e6684
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8544265 [details] [diff] [review]
Null check mDecoder

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Youtube users will experience more crashes, less consistent testing.
[Describe test coverage new/current, TBPL]: Landed on m-c.
[Risks and why]: Risk is low, we just protect against an ininitialized access.
[String/UUID change made/needed]: None.
Attachment #8544265 - Flags: approval-mozilla-beta?
Attachment #8544265 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
No crash reports in Socorro after 2015-01-14 builds. Marking as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.