64-bit crash in mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed()

RESOLVED FIXED in Firefox 41

Status

()

--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: philipp, Assigned: aklotz)

Tracking

({crash})

41 Branch
mozilla43
Unspecified
Windows NT
crash
Points:
---

Firefox Tracking Flags

(firefox41+ fixed, firefox42+ fixed, firefox43+ fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
This bug was filed from the Socorro interface and is 
report bp-77603c0b-889d-4a1b-88b9-6281e2150902.
=============================================================
Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed() 	dom/plugins/ipc/PluginAsyncSurrogate.cpp
1 	xul.dll 	mozilla::plugins::PluginModuleParent::OnInitFailure() 	dom/plugins/ipc/PluginModuleParent.cpp
2 	xul.dll 	mozilla::plugins::PluginModuleChromeParent::OnProcessLaunched(bool) 	dom/plugins/ipc/PluginModuleParent.cpp
3 	xul.dll 	mozilla::plugins::PluginProcessParent::RunLaunchCompleteTask() 	dom/plugins/ipc/PluginProcessParent.cpp
4 	xul.dll 	mozilla::plugins::PluginProcessParent::WaitUntilConnected(int) 	dom/plugins/ipc/PluginProcessParent.cpp
5 	xul.dll 	mozilla::plugins::PluginAsyncSurrogate::WaitForInit() 	dom/plugins/ipc/PluginAsyncSurrogate.cpp
6 	xul.dll 	mozilla::plugins::PluginAsyncSurrogate::ScriptableHasProperty(NPObject*, void*) 	dom/plugins/ipc/PluginAsyncSurrogate.cpp
7 	xul.dll 	NPObjWrapper_Resolve 	dom/plugins/base/nsJSNPRuntime.cpp
8 	xul.dll 	js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) 	js/src/vm/NativeObject.h
9 	xul.dll 	GetPropertyOperation 	js/src/vm/Interpreter.cpp
10 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp
11 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp
12 	xul.dll 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
13 	xul.dll 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
14 	xul.dll 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp
15 	xul.dll 	mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) 	obj-firefox/dom/bindings/FunctionBinding.cpp
16 	xul.dll 	mozilla::dom::Function::Call<nsCOMPtr<nsISupports> >(nsCOMPtr<nsISupports> const&, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) 	obj-firefox/dist/include/mozilla/dom/FunctionBinding.h
17 	xul.dll 	nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) 	dom/base/nsGlobalWindow.cpp
18 	xul.dll 	nsGlobalWindow::RunTimeout(nsTimeout*) 	dom/base/nsGlobalWindow.cpp
19 	xul.dll 	nsGlobalWindow::TimerCallback(nsITimer*, void*) 	dom/base/nsGlobalWindow.cpp
20 	xul.dll 	nsTimerImpl::Fire() 	xpcom/threads/nsTimerImpl.cpp
21 	xul.dll 	nsTimerEvent::Run() 	xpcom/threads/nsTimerImpl.cpp
22 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
23 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
24 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
25 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
26 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
27 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
28 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
29 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
30 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
31 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp
32 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp
33 	firefox.exe 	NS_internal_main(int, char**) 	browser/app/nsBrowserApp.cpp
34 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
35 	firefox.exe 	__tmainCRTStartup 	f:/dd/vctools/crt/crtw32/startup/crt0.c:255
36 	kernel32.dll 	BaseThreadInitThunk 	
37 	ntdll.dll 	RtlUserThreadStart 	
38 	kernel32.dll 	BasepReportFault 	
39 	kernel32.dll 	BasepReportFault

this crash signature seems to surge in firefox 41.0b6, presumably related to the code-change that landed with bug 1198302
(Reporter)

Comment 1

3 years ago
need-infoing aaron klotz about this (as per conversation on irc with dmajor).
Flags: needinfo?(aklotz)

Comment 2

3 years ago
[Tracking Requested - why for this release]: regression/topcrash
tracking-firefox41: --- → ?
tracking-firefox42: --- → ?
tracking-firefox43: --- → ?
|owner| is null in nsPluginInstanceOwner::NotifyHostAsyncInitFailed().

The spike on b6 is entirely on win64 builds.

Comment 4

3 years ago
I see 3 patches in b6 that touched plugin-related stuff: Bug 1177367, bug 1198302, and bug 1185532. The second of those is in PluginAsyncSurrogate::NotifyAsyncInitFailed, which is where we crash here, and the latter is specifically 64bit-related.
Bug 1198302 was releases/mozilla-beta changeset cc07ed95653e and I don't think that it could have caused a null |owner| when there wasn't one before.

Bug 1185532 was releases/mozilla-beta changeset adad025c19a8 which seems much more likely, given the 64-bit correlation. CCing Bob and since he's on PTO, flagging bsmedberg too.
Blocks: 1185532
No longer blocks: 1198302
Flags: needinfo?(benjamin)
Summary: crash in mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed() → 64-bit crash in mozilla::plugins::PluginAsyncSurrogate::NotifyAsyncInitFailed()
(Assignee)

Comment 6

3 years ago
Created attachment 8656643 [details] [diff] [review]
Add owner check

I can't speak for why we're seeing this only on 64-bit, but seeing that it's happening, let's add a proper null check.
Flags: needinfo?(aklotz)
Attachment #8656643 - Flags: review?(jmathies)

Comment 7

3 years ago
Comment on attachment 8656643 [details] [diff] [review]
Add owner check

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

The handler we're skipping here tells content to stop loading the plugin. I guess if owner is null the sandbox broke plugin init and presumably content already knows?
Attachment #8656643 - Flags: review?(jmathies) → review+
(Assignee)

Comment 8

3 years ago
(In reply to Jim Mathies [:jimm] from comment #7)
> Comment on attachment 8656643 [details] [diff] [review]
> Add owner check
> 
> Review of attachment 8656643 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The handler we're skipping here tells content to stop loading the plugin. I
> guess if owner is null the sandbox broke plugin init and presumably content
> already knows?

Yeah if the owner is null then content must have already torn its side down.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 9

3 years ago
Comment on attachment 8656643 [details] [diff] [review]
Add owner check

Approval Request Comment
[Feature/regressing bug #]: sandboxing + async plugin init
[User impact if declined]: crashes if plugin can't load
[Describe test coverage new/current, TreeHerder]: locally
[Risks and why]: None. Simple patch to add null pointer check
[String/UUID change made/needed]: None
Attachment #8656643 - Flags: approval-mozilla-beta?
Attachment #8656643 - Flags: approval-mozilla-aurora?

Updated

3 years ago
Flags: needinfo?(benjamin)
Comment on attachment 8656643 [details] [diff] [review]
Add owner check

Patch seems simple enough and it addresses a top crash on Beta41. Let's uplift before Beta7 goes out to see if it helps.
Attachment #8656643 - Flags: approval-mozilla-beta?
Attachment #8656643 - Flags: approval-mozilla-beta+
Attachment #8656643 - Flags: approval-mozilla-aurora?
Attachment #8656643 - Flags: approval-mozilla-aurora+
Tracked as it's a top crash.
status-firefox42: --- → affected
tracking-firefox41: ? → +
tracking-firefox42: ? → +
https://hg.mozilla.org/releases/mozilla-beta/rev/e9159912a5b3
status-firefox41: affected → fixed
status-firefox43: --- → affected
Keywords: checkin-needed
Keywords: checkin-needed

Updated

3 years ago
Assignee: nobody → aklotz
https://hg.mozilla.org/mozilla-central/rev/a808cd51b754
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
tracking-firefox43: ? → +
You need to log in before you can comment on or make changes to this bug.