515546 - Null check mReader in [@ nsOggDecoder::Shutdown]

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

While digging through crash-stats for media related stuff, I found a crash in nsOggDecoder::Shutdown that has happened around 250 times in the past couple of weeks (see bug URL).  It appears to be caused by a null mReader during shutdown, which may happen if Load() returns an error before initializing the reader, or if the reader fails to initialize.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: patch v0

Null check mReader in Shutdown().  Also removes an unnecessary null check of mReader->Stream(), since Stream() will always return a valid pointer if we have an mReader.

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 399649 [details] [diff] [review]
patch v0

Mochitests like this.

Comment 3

[timeless]

Please include one stack trace when filing such bugs, it helps us later determine if a bug you've fixed is the same as a potential duplicate someone is triaging.

Signature	nsOggDecoder::Shutdown()
UUID	d2216041-0600-49c0-8322-2f2532090909
Time 	2009-09-09 16:10:55.809479
Uptime	13
Last Crash	21 seconds before submission
Product	Firefox
Version	3.5.2
Build ID	20090729225027
Branch	1.9.1
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 15 model 3 stepping 4
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x24
User Comments	
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsOggDecoder::Shutdown 	content/media/video/src/nsOggDecoder.cpp:1935
1 	xul.dll 	nsHTMLMediaElement::DestroyContent 	content/html/content/src/nsHTMLMediaElement.cpp:1577
2 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
3 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
4 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
5 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
6 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
7 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
8 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
9 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
10 	xul.dll 	nsGenericElement::DestroyContent 	content/base/src/nsGenericElement.cpp:3495
11 	xul.dll 	nsDocument::Destroy 	content/base/src/nsDocument.cpp:6967
12 	xul.dll 	DocumentViewerImpl::Destroy 	layout/base/nsDocumentViewer.cpp:1501
13 	xul.dll 	DocumentViewerImpl::Show 	layout/base/nsDocumentViewer.cpp:1840
14 	xul.dll 	nsPresContext::EnsureVisible 	layout/base/nsPresContext.cpp:1550
15 	xul.dll 	nsPluginInstanceOwner::Init 	layout/generic/nsObjectFrame.cpp:4491
16 	xul.dll 	nsObjectFrame::PrepareInstanceOwner 	layout/generic/nsObjectFrame.cpp:1746
17 	xul.dll 	nsObjectFrame::Instantiate 	layout/generic/nsObjectFrame.cpp:1801
18 	xul.dll 	nsObjectLoadingContent::Instantiate 	content/base/src/nsObjectLoadingContent.cpp:1768
19 	xul.dll 	nsAsyncInstantiateEvent::Run 	content/base/src/nsObjectLoadingContent.cpp:156
20 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
21 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
22 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:193
23 	nspr4.dll 	PR_GetEnv 	
24 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
25 	firefox.exe 	firefox.exe@0x21a7 	
26 	kernel32.dll 	BaseProcessStart

Comment 4

[Matthew Gregan [:kinetik]]

http://hg.mozilla.org/mozilla-central/rev/e6fd6698d21b

Comment 5

[Matthew Gregan [:kinetik]]

Comment on attachment 399649 [details] [diff] [review]
patch v0

This is a simple and safe change, and fixes one of the most frequent crashes in the media code.  Requesting approval for 1.9.2.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1480bec87afe