896242 - crash in nsPluginTag::IsEnabled

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Scoobidiver (away)]

It first showed up in 25.0a1/20130711 and is currently #32 browser crasher in 25.0a1. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=04d8c309fe72&tochange=dde4dcd6fa46
It's likely a regression from bug 855611.

Signature 	nsPluginTag::IsEnabled() More Reports Search
UUID 	d4e601b0-5df8-4ec4-8e32-06c7d2130721
Date Processed	2013-07-21 04:30:12.282516
Uptime	8506
Last Crash	4845939 seconds before submission
Install Age 	50211 since version was first installed.
Install Time 	2013-07-20 14:32:40
Product 	Firefox
Version 	25.0a1
Build ID 	20130720030214
Release Channel 	nightly
OS 	Windows NT
OS Version 	6.1.7601 Service Pack 1
Build Architecture 	x86
Build Architecture Info 	GenuineIntel family 6 model 42 stepping 7 | 4
Crash Reason 	EXCEPTION_ACCESS_VIOLATION_EXEC
Crash Address 	0xffffffff8db7bcf3
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x68fa, AdapterSubsysID: 00000000, AdapterDriverVersion: 8.920.0.0
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 

Frame 	Module 	Signature 	Source
0 		@0x8db7bcf3 	
1 	xul.dll 	nsPluginTag::IsEnabled() 	dom/plugins/base/nsPluginTags.cpp
2 	xul.dll 	nsMimeType::GetEnabledPlugin() 	dom/base/nsMimeTypeArray.cpp
3 	xul.dll 	mozilla::dom::MimeTypeBinding::get_enabledPlugin 	obj-firefox/dom/bindings/MimeTypeBinding.cpp
4 	xul.dll 	mozilla::dom::MimeTypeBinding::genericGetter 	obj-firefox/dom/bindings/MimeTypeBinding.cpp
5 	mozjs.dll 	js::Invoke(JSContext *,JS::Value const &,JS::Value const &,unsigned int,JS::Value *,JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
6 	mozjs.dll 	js::GetPropertyHelper(JSContext *,JS::Handle<JSObject *>,JS::Handle<int>,unsigned int,JS::MutableHandle<JS::Value>) 	js/src/jsobj.cpp
7 	mozjs.dll 	GetPropertyOperation(JSContext *,js::StackFrame *,JS::Handle<JSScript *>,unsigned char *,JS::MutableHandle<JS::Value>,JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
8 	mozjs.dll 	Interpret 	js/src/vm/Interpreter.cpp
9 	mozjs.dll 	js::ion::CanEnter(JSContext *,js::RunState &) 	js/src/ion/Ion.cpp
10 	mozjs.dll 	js::RunScript(JSContext *,js::RunState &) 	js/src/vm/Interpreter.cpp
11 	mozjs.dll 	js::ExecuteKernel(JSContext *,JS::Handle<JSScript *>,JSObject &,JS::Value const &,js::ExecuteType,js::AbstractFramePtr,JS::Value *) 	js/src/vm/Interpreter.cpp
12 	mozjs.dll 	js::Execute(JSContext *,JS::Handle<JSScript *>,JSObject &,JS::Value *) 	js/src/vm/Interpreter.cpp
13 	mozjs.dll 	JS::Evaluate(JSContext *,JS::Handle<JSObject *>,JS::CompileOptions,wchar_t const *,unsigned int,JS::Value *) 	js/src/jsapi.cpp
14 	xul.dll 	nsJSContext::EvaluateString(nsAString_internal const &,JS::Handle<JSObject *>,JS::CompileOptions &,bool,JS::Value *) 	dom/base/nsJSEnvironment.cpp
15 	xul.dll 	nsScriptLoader::EvaluateScript(nsScriptLoadRequest *,nsString const &) 	content/base/src/nsScriptLoader.cpp
...

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=nsPluginTag%3A%3AIsEnabled%28%29

Comment 1

[Benjamin Smedberg]

This looks like we're calling IsEnabled on a freed nsPluginTag. That probably also means that nsMimeType::mPluginElement is a dead element. I don't really understand how it's safe to use a weak pointer in this context:

http://hg.mozilla.org/mozilla-central/annotate/2268ff80683a/dom/base/nsMimeTypeArray.h#l96

Comment 2

[Scoobidiver (away)]

More reports also at:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=nsPluginTag%3A%3AGetPluginState%28%29
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Adom%3A%3AMimeTypeBinding%3A%3Aget_enabledPlugin
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=nsMimeType%3A%3AGetEnabledPlugin%28%29

Comment 3

[Johnny Stenback (:jst)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> This looks like we're calling IsEnabled on a freed nsPluginTag. That
> probably also means that nsMimeType::mPluginElement is a dead element. I
> don't really understand how it's safe to use a weak pointer in this context:
> 
> http://hg.mozilla.org/mozilla-central/annotate/2268ff80683a/dom/base/
> nsMimeTypeArray.h#l96

It should be safe because of http://hg.mozilla.org/mozilla-central/annotate/2268ff80683a/dom/base/nsMimeTypeArray.h#l78 and http://hg.mozilla.org/mozilla-central/annotate/2268ff80683a/dom/base/nsPluginArray.cpp#l299.

Comment 4

[Johnny Stenback (:jst)]

Attachment: Possible fix.

This could fix this crash. If the cycle collector unlinks an nsPluginElement we could be left with nsMimeType objects floating around in memory with a dangling pointer to the nsPluginElement that could've been deleted... I have yet to prove this theory though.

Comment 5

[Andrew McCreight [:mccr8]]

Comment on attachment 780209 [details] [diff] [review]
Possible fix.

Review of attachment 780209 [details] [diff] [review]:
-----------------------------------------------------------------

Can't hurt, I guess.

Comment 6

[Johnny Stenback (:jst)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9e89a88f017

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f9e89a88f017

Comment 8

[Johnny Stenback (:jst)]

Based on crash-stats this crash has been fixed. Closing.

Comment 9

[u279076]

Looks like the signatures in this bug are non-existent in recent crash-stats for Firefox 25 so I'm marking this verified fixed.