620937 - Crash [@ nsContentPermissionRequestProxy::GetElement] [@ nsContentPermissionRequestProxy::GetUri]

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect, P5)

Description

[Josh Matthews [:jdm]]

ID: a6d79116-3cf1-4d39-9247-051af2101221

Signature	nsContentPermissionRequestProxy::GetElement
UUID	a6d79116-3cf1-4d39-9247-051af2101221
Time 	2010-12-21 17:27:02.662861
Uptime	118
Last Crash	34854 seconds (9.7 hours) before submission
Install Age	34814 seconds (9.7 hours) since version was first installed.
Product	Fennec
Version	4.0b4pre
Build ID	20101220042014
Branch	2.0
OS	Linux
OS Version	0.0.0 Linux 2.6.35.9-cyanogenmod #1 PREEMPT Wed Dec 15 19:51:23 EST 2010 armv7l
CPU	arm
CPU Info	
Crash Reason	SIGSEGV
Crash Address	0x0
User Comments	
App Notes 	HTC Nexus One
google/passion/passion/mahimahi:2.2.1/FRG83D/75603:user/release-keys
Processor Notes 	
EMCheckCompatibility	False
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	libxul.so 	nsContentPermissionRequestProxy::GetElement 	dom/base/nsContentPermissionHelper.cpp:109
1 	libxul.so 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:199
2 	libxul.so 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3064
3 	libxul.so 	XPC_WN_GetterSetter 	js/src/xpconnect/src/xpcprivate.h:2637
4 	libxul.so 	js::Invoke 	js/src/jscntxtinlines.h:689
5 	libxul.so 	js::ExternalGetOrSet 	js/src/jsinterp.cpp:858
6 	libxul.so 	js_GetPropertyHelper 	js/src/jsobj.cpp:5054
7 	libxul.so 	js::Interpret 	js/src/jsinterp.cpp:4156
8 	libxul.so 	js::Invoke 	js/src/jsinterp.cpp:657
9 	libxul.so 	js::ExternalInvoke 	js/src/jsinterp.cpp:858
10 	libxul.so 	JS_CallFunctionValue 	js/src/jsinterp.h:962
11 	libxul.so 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1694
12 	libxul.so 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:589
13 	libxul.so 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:134
14 	libxul.so 	libxul.so@0x957854 	
15 	libxul.so 	nsContentPermissionRequestProxy::Init 	nsCOMPtr.h:492
16 		@0x505fe72f 	
17 	libxul.so 	mozilla::dom::ContentPermissionRequestParent::Recvprompt 	dom/base/nsContentPermissionHelper.cpp:158
18 	libxul.so 	mozilla::dom::PContentPermissionRequestParent::OnMessageReceived 	PContentPermissionRequestParent.cpp:107
19 	libxul.so 	mozilla::dom::PContentParent::OnMessageReceived 	PContentParent.cpp:604
20 	libxul.so 	mozilla::ipc::AsyncChannel::OnDispatchMessage 	ipc/glue/AsyncChannel.cpp:262
21 	libxul.so 	mozilla::ipc::RPCChannel::OnMaybeDequeueOne 	ipc/glue/RPCChannel.cpp:440
22 	libxul.so 	RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run 	ipc/chromium/src/base/task.h:308
23 	libxul.so 	mozilla::ipc::RPCChannel::DequeueTask::Run 	RPCChannel.h:475
24 	libxul.so 	MessageLoop::RunTask 	ipc/chromium/src/base/message_loop.cc:344
25 	libxul.so 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/message_loop.cc:354
26 	libxul.so 	MessageLoop::DoWork 	ipc/chromium/src/base/message_loop.cc:451
27 	libxul.so 	mozilla::ipc::DoWorkRunnable::Run 	ipc/glue/MessagePump.cpp:71
28 	libxul.so 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:626
29 	libxul.so 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:250
30 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:134
31 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:220
32 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:512
33 	libxul.so 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:198
34 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:192
35 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3693
36 	libxul.so 	GeckoStart 	toolkit/xre/nsAndroidStartup.cpp:131
37 	libc.so 	libc.so@0x11473 	
38 	libc.so 	libc.so@0x10f5f

Comment 1

[Josh Matthews [:jdm]]

So, we're hitting

>109  NS_ADDREF(*aRequestingElement = mParent->mElement);

when mElement is null.  Given that the only time mElement is set is in ContentPermissionRequestParent::ContentPermissionRequestParent:

>248   return new ContentPermissionRequestParent(type, mFrameElement, uri);

we've got a tabParent with a null frame element, ie. it's most likely been destroyed.

Comment 2

[Josh Matthews [:jdm]]

I expect that the GetURI crashes I see occasionally are the same root cause.

Comment 3

[Doug Turner (:dougt)]

i can look soon.  jdm, steal if you want to.

Comment 4

[Josh Matthews [:jdm]]

I've been poking around, and I'm confused, especially in regards to the GetURI crashes.  The URI is sent from the content process, and corresponds to nsGeolocation::mURI.  That's retrieved from the document's node principal, and never changed until nsGeolocation::Shutdown, at which point it's nulled out.  If there's a null URI at the time we assign mURI, we bail.  The only way I can figure we'd end up with a null URI, therefore, is if we are calling RegisterRequestWithPrompt after Shutdown.

Comment 5

[Doug Turner (:dougt)]

could this be called by an extension somehow?

Comment 6

[Josh Matthews [:jdm]]

Any time nsGeolocation::Update is called, if WindowOwnerStillExists returns
false, we call Shutdown.  Since mOwner must be non-null for
RegisterRequestForPrompt to succeed, that means that this block would return
false:

>    PRBool closed = PR_FALSE;
>    window->GetClosed(&closed);
>    if (closed)
>      return PR_FALSE;
>
>    nsPIDOMWindow* outer = window->GetOuterWindow();
>    if (!outer || outer->GetCurrentInnerWindow() != window)
>      return PR_FALSE;

In regards to extensions, I don't see how that would make a difference.

Comment 7

[Josh Matthews [:jdm]]

Bug 627355 is also tracking the GetURI crash, but I'll leave it open until we conclusively prove they're the same issue.

Comment 8

[Josh Matthews [:jdm]]

Interestingly enough, not every GetUri crash is at 0x0.  There are a significant number of 0x12 crash addresses as well.

Comment 9

[Stuart Parmenter]

This crash doesn't appear to be happening very often per our crash reporting system, but would still love to see a fix

Comment 10

[Josh Matthews [:jdm]]

Not fixed. We still see stacks like the one in comment 0.

Comment 11

[Doug Turner (:dougt)]

I wonder why we just don't call:

mProxy->OnParentDestroyed();
                 
from the ContentPermissionRequestParent destructor instead of on ::ActorDestroy.

Comment 12

[Doug Turner (:dougt)]

is this still showing up on crash-stats?

Comment 13

[Josh Matthews [:jdm]]

Still there on 4.0.1, don't see it in 6.0a1 but that doesn't necessarily mean anything. I don't think we ever really saw the signature in nightlies.

Comment 14

[Doug Turner (:dougt)]

ah.  that explains it.  I was looking at the nightly crashes.

Comment 15

[Doug Turner (:dougt)]

jdm,

ContentPermissionRequestParent owns (strong reference) a mElement,

ContentPermissionRequestParent::Recvprompt is called and we create a nsContentPermissionRequestProxy.  This new object is assigned to a nsCOMPtr and held onto for the life of ContentPermissionRequestParent.

Then we create a nsContentPermissionRequestProxy, passing aContentPermissionRequestParent pointer to the proxy.  The proxy holds onto this parent pointer with the assumption that the parent will exist as long as the proxy does.

Now, I think the nsContentPermissionRequestProxy object (mProxy) should exist as long as the parent does.  The only place where the state could be wacked is in ::Recvprompt().  Maybe we are oom, or maybe Init() is failing?  If that happens, mProxy would be valid, but its state not-so-much.

Alternatively, we can make mParent in nsContentPermissionRequestProxy hold a strong reference.  Then in ContentPermissionRequestParent::ActorDestroy clear the cycle.

Comment 16

[Josh Matthews [:jdm]]

Here's the stack we're looking at:

nsContentPermissionRequestProxy::GetElement
NS_InvokeByIndex_P
XPCWrappedNative::CallMethod
XPC_WN_GetterSetter
js::Invoke
js::ExternalGetOrSet
js_GetPropertyHelper
js::Interpret
js::Invoke
js::ExternalInvoke
JS_CallFunctionValue
nsXPCWrappedJSClass::CallMethod
nsXPCWrappedJS::CallMethod
PrepareAndDispatch
nsContentPermissionRequestProxy::Init
mozilla::dom::ContentPermissionRequestParent::Recvprompt

Presumably we're crashing on this line in ContentPermissionPrompt.js: http://mxr.mozilla.org/mozilla-central/source/mobile/components/ContentPermissionPrompt.js#63

The corresponding URI crash that we're also seeing is presumably at
http://mxr.mozilla.org/mozilla-central/source/mobile/components/ContentPermissionPrompt.js#68 or http://mxr.mozilla.org/mozilla-central/source/mobile/components/ContentPermissionPrompt.js#99. Actually, the second one is in a callback, so that's probably fixed by when we added the ActorDestroy last time we looked at this.

That means that when we're inside ContentPermissionPrompt.prompt, request should be valid, and request::mParent should definitely be valid since we shouldn't have had a chance to spin the event loop. Even if we did, the ActorDestroy should take care of any situation that would invalidate the pointer, and we check for NULL. Therefore, I can only conclude that we're inlining the addref call, and that one of ContentPermissionRequestParent::mElement or ContentPermissionRequestParent::mURI  are null.

This indicates that we're either sending a null URI over the wire, or mFrameElement in TabParent is null. That's as far as I've delved so far.

Comment 17

[Christian Riechers]

I've seen this crash with FF12 beta on a Nexus S device running ICS.
bp-493d3403-5c85-4e00-836c-45ec72120423
It's reproducible for me when visiting
https://developer.android.com/guide/developing/tools/adb.html
FF either crashes immediately, or once starting zooming into the page.
Even worse, a song being played in the background with Google Play Music stops. This is reproducible here.

Comment 18

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Closing because no crash reported since 12 weeks.