Closed Bug 1104160 Opened 9 years ago Closed 9 years ago

crash in CPOWTimer::~CPOWTimer()

Categories

(Core :: IPC, defect)

36 Branch
All
Windows NT
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla36
Tracking Status
e10s m4+ ---
firefox36 --- verified

People

(Reporter: jbecerra, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-f3df2777-39d1-4f3a-ba78-3112d2141120.
=============================================================

This is a new signature on Fx36 nightly starting 11/20. It's affecting Windows 7 and Windows 8.1 installations mostly. There are several comments in the reports and a couple of them mention that the plugin container stopped working and causing the browser to crash shortly after, consistently, while using e10s. I haven't tried to reproduce the problem yet. This is a startup crash for some users, and there are a lot of dups.

Not sure where to file it. Please put it in the right bucket. 

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

0 	xul.dll 	CPOWTimer::~CPOWTimer() 	js/ipc/CPOWTimer.cpp
1 	xul.dll 	CPOWProxyHandler::objectClassIs(JS::Handle<JSObject*>, js::ESClassValue, JSContext*) 	js/ipc/WrapperOwner.cpp
2 	xul.dll 	js::Proxy::objectClassIs(JS::Handle<JSObject*>, js::ESClassValue, JSContext*) 	js/src/proxy/Proxy.cpp
3 	xul.dll 	JS_ObjectIsDate(JSContext*, JS::Handle<JSObject*>) 	js/src/jsapi.cpp
4 	xul.dll 	mozilla::dom::MutationObserverInit::Init(JSContext*, JS::Handle<JS::Value>, char const*, bool) 	obj-firefox/dom/bindings/MutationObserverBinding.cpp
5 	xul.dll 	mozilla::dom::MutationObserverBinding::observe 	obj-firefox/dom/bindings/MutationObserverBinding.cpp
6 	xul.dll 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp
7 	xul.dll 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
8 	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
9 	xul.dll 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp
10 	xul.dll 	mozilla::jsipc::WrapperAnswer::RecvCallOrConstruct(mozilla::jsipc::ObjectId const&, nsTArray<mozilla::jsipc::JSParam> const&, bool const&, mozilla::jsipc::ReturnStatus*, mozilla::jsipc::JSVariant*, nsTArray<mozilla::jsipc::JSParam>*) 	js/ipc/WrapperAnswer.cpp
11 	xul.dll 	mozilla::jsipc::JavaScriptBase<mozilla::jsipc::PJavaScriptParent>::RecvCallOrConstruct(unsigned __int64 const&, nsTArray<mozilla::jsipc::JSParam> const&, bool const&, mozilla::jsipc::ReturnStatus*, mozilla::jsipc::JSVariant*, nsTArray<mozilla::jsipc::JSParam>*) 	js/ipc/JavaScriptBase.h
12 	xul.dll 	mozilla::jsipc::PJavaScriptChild::OnMessageReceived(IPC::Message const&, IPC::Message*&) 	obj-firefox/ipc/ipdl/PJavaScriptChild.cpp
13 	xul.dll 	mozilla::layers::PCompositorChild::OnMessageReceived(IPC::Message const&, IPC::Message*&) 	obj-firefox/ipc/ipdl/PCompositorChild.cpp
14 	xul.dll 	mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
15 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
16 	xul.dll 	RunnableMethod<mozilla::ipc::MessageChannel, void ( mozilla::ipc::MessageChannel::*)(void), Tuple0>::Run() 	ipc/chromium/src/base/task.h
17 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
18 	xul.dll 	mozilla::ipc::DoWorkRunnable::Run() 	ipc/glue/MessagePump.cpp
19 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
20 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
21 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
22 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
23 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
24 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
25 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
26 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
27 	xul.dll 	XRE_RunAppShell 	toolkit/xre/nsEmbedFunctions.cpp
28 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
29 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
30 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
31 	xul.dll 	XRE_InitChildProcess 	toolkit/xre/nsEmbedFunctions.cpp
32 	plugin-container.exe 	content_process_main(int, char** const) 	ipc/contentproc/plugin-container.cpp
33 	plugin-container.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
34 	plugin-container.exe 	__tmainCRTStartup 	f:/dd/vctools/crt/crtw32/startup/crt0.c:255
35 	kernel32.dll 	BaseThreadInitThunk 	
36 	ntdll.dll 	RtlUserThreadStart 	
37 	kernel32.dll 	BasepReportFault 	
38 	kernel32.dll 	BasepReportFault
Looks like we need some null checking.
Flags: needinfo?(blassey.bugs)
Blocks: 1103327
Attached patch null_check.patchSplinter Review
Flags: needinfo?(blassey.bugs)
Attachment #8527838 - Flags: review?(wmccloskey)
Comment on attachment 8527838 [details] [diff] [review]
null_check.patch

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

::: js/ipc/CPOWTimer.cpp
@@ +8,5 @@
>  #include "jsfriendapi.h"
>  #include "xpcprivate.h"
>  #include "CPOWTimer.h"
>  
>  CPOWTimer::~CPOWTimer() {

The brace should go on its own line. I mentioned this twice in the review for bug 1074567.

@@ +10,5 @@
>  #include "CPOWTimer.h"
>  
>  CPOWTimer::~CPOWTimer() {
>      /* This is a best effort to find the compartment responsible for this CPOW call */
> +    nsIGlobalObject* global = mozilla::dom::GetIncumbentGlobal();

The * should go with the variable name in js/.

@@ +13,5 @@
>      /* This is a best effort to find the compartment responsible for this CPOW call */
> +    nsIGlobalObject* global = mozilla::dom::GetIncumbentGlobal();
> +    if (!global)
> +        return;
> +    JSObject* obj = global->GetGlobalJSObject();

Same.

@@ +17,5 @@
> +    JSObject* obj = global->GetGlobalJSObject();
> +    if (!obj)
> +        return;
> +    JSCompartment *compartment = js::GetObjectCompartment(obj);
> +    if (!compartment)

No need for this check. Every object has a compartment.

@@ +19,5 @@
> +        return;
> +    JSCompartment *compartment = js::GetObjectCompartment(obj);
> +    if (!compartment)
> +        return;
> +    xpc::CompartmentPrivate* compartmentPrivate = xpc::CompartmentPrivate::Get(compartment);

Same thing about the *.
Attachment #8527838 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/6bc3eb8b5a28
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.