crash in mozilla::GStreamerReader::GetBuffered(mozilla::dom::TimeRanges*)

VERIFIED FIXED in Firefox 38



4 years ago
4 years ago


(Reporter: Usul, Assigned: karlt)



Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox37 unaffected, firefox38 verified, firefox39 verified)


(crash signature)


(1 attachment)



4 years ago
This bug was filed from the Socorro interface and is 
report bp-fe4c5b6c-b398-4fde-921b-ea8a22150305.
0 	mozilla::GStreamerReader::GetBuffered(mozilla::dom::TimeRanges*) 	dom/media/gstreamer/GStreamerReader.cpp
1 	mozilla::MediaDecoder::GetBuffered(mozilla::dom::TimeRanges*) 	dom/media/MediaDecoderStateMachine.h
2 	mozilla::dom::HTMLMediaElement::Buffered() const 	dom/html/HTMLMediaElement.cpp
3 	mozilla::dom::HTMLMediaElementBinding::get_buffered 	obj-firefox/dom/bindings/HTMLMediaElementBinding.cpp
4 	mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp
5 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
6 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
7 	js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
8 	Interpret 	js/src/vm/NativeObject.h
9 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp
10 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
11 	js::CallOrConstructBoundFunction(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp
12 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
13 	Interpret 	js/src/vm/Interpreter.cpp
14 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp
15 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
16 	js::CallOrConstructBoundFunction(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp
17 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
18 	js::fun_call(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp
19 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
20 	Interpret 	js/src/vm/Interpreter.cpp
21 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp
22 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
23 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
24 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp
25 	mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) 	obj-firefox/dom/bindings/EventListenerBinding.cpp
26 	void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) 	obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h
27 	mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) 	dom/events/EventListenerManager.cpp
28 	mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) 	dom/events/EventListenerManager.cpp
29 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventListenerManager.h
30 	mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) 	dom/events/EventDispatcher.cpp
31 	mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) 	dom/events/EventDispatcher.cpp
32 	nsINode::DispatchEvent(nsIDOMEvent*, bool*) 	dom/base/nsINode.cpp
33 	nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*, bool) 	dom/base/nsContentUtils.cpp
34 	nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) 	dom/base/nsContentUtils.cpp
35 	mozilla::dom::HTMLMediaElement::DispatchEvent(nsAString_internal const&) 	dom/html/HTMLMediaElement.cpp
36 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
37 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
38 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
39 	MessageLoop::Run() 	ipc/chromium/src/base/
40 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
41 	XRE_RunAppShell 	toolkit/xre/nsEmbedFunctions.cpp
42 	MessageLoop::Run() 	ipc/chromium/src/base/
43 	XRE_InitChildProcess 	toolkit/xre/nsEmbedFunctions.cpp
44 	js::jit::CreateDerivedTypedObj(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, int) 	js/src/jit/VMFunctions.cpp
45 		@0x7f9b6f231eff 	
46 	_fini
Component: DOM → Video/Audio
Blocks: 1091008
Bug 1091008 _may_ have caused this, or perhaps the issue is something older.
At a guess, mDecoder is null?

Bobby, you've been in this code recently..
Flags: needinfo?(bobbyholley)
I'm swamped, sorry. Karl, do you have the cycles to look at this?
Flags: needinfo?(bobbyholley) → needinfo?(karlt)

Comment 4

4 years ago
It seems either the reader's mDecoder or the decoder's mResource is null.

The reader's mDecoder might be null if the reader has been Shutdown, but the decoder has not been disconnected from the media element, if such a situation can arise.

I don't see how decoder's mResource could be null.

This is more likely a regression from changes in bug 1095251.
I likely won't get to this today, but will chase up next week.
Assignee: nobody → karlt
Blocks: 1095251
No longer blocks: 1091008
Flags: needinfo?(karlt)

Comment 5

4 years ago
(In reply to Karl Tomlinson (:karlt) from comment #4)
> The reader's mDecoder might be null if the reader has been Shutdown, but the
> decoder has not been disconnected from the media element, if such a
> situation can arise.

Each of HTMLMediaElement::NetworkError() and ResetConnectionState() receive
notifications from the MediaDecoder before it calls Shutdown(), but do
nothing to disconnect from the MediaDecoder.

Similarly, I don't see any disconnection on invocation of MDecoder::Shutdown()
from NS_XPCOM_SHUTDOWN_OBSERVER_ID, though the stack trace here is not in xpcom shutdown.

Some options are:
a) a general shutdown notification from the MDecoder to its owner, at which point
   the HTMLMediaElement can clear mDecoder.
b) MediaDecoderStateMachine::GetBuffered() can check DECODER_STATE_SHUTDOWN before
   using mReader.

Comment 6

4 years ago
mReadyState is > HAVE_NOTHING on fatal network errors because the spec doesn't
say that this should be changed.

"Fatal network errors that occur after the user agent has established whether
the current media resource is usable (i.e. once the media element's readyState
attribute is no longer HAVE_NOTHING) must cause the user agent to execute the
following steps:

     1. The user agent should cancel the fetching process.

     2. Set the error attribute to a new MediaError object whose code
        attribute is set to MEDIA_ERR_NETWORK.

     3. Fire a simple event named error at the media element.

     4. Set the element's networkState attribute to the NETWORK_IDLE value.

     5. Set the element's delaying-the-load-event flag to false. This stops
        delaying the load event.

     6. Abort the overall resource selection algorithm."

Comment 7

4 years ago
Also, HTMLMediaElement uses mDecoder for GetCurrentPrincipal(), so I hesitate to clear mDecoder.

Instead, it is simple to check for shutdown before asking the reader for buffered.

Comment 8

4 years ago
Created attachment 8575048 [details] [diff] [review]
check for shutdown before asking the reader for buffered
Attachment #8575048 - Flags: review?(bobbyholley)
Attachment #8575048 - Flags: review?(bobbyholley) → review+

Comment 9

4 years ago

Possibly could make a crashtest if we can generate a network error and then get the buffered attribute.
Flags: in-testsuite?


4 years ago
status-firefox37: --- → unaffected
status-firefox38: --- → affected

Comment 10

4 years ago
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

Comment 11

4 years ago
Comment on attachment 8575048 [details] [diff] [review]
check for shutdown before asking the reader for buffered

Approval Request Comment
[Feature/regressing bug #]: bug 1095251 
[User impact if declined]: null deref
[Describe test coverage new/current, TreeHerder]: no testcase at this stage.
[Risks and why]: simple boolean check, low risk
[String/UUID change made/needed]: none
Attachment #8575048 - Flags: approval-mozilla-aurora?
Attachment #8575048 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Socorro [1] reports zero crashes after this fix landed.

[1] -
status-firefox38: fixed → verified
status-firefox39: fixed → verified
You need to log in before you can comment on or make changes to this bug.