:Irving Reid (No longer working on Firefox) Reporter
	Description • 12 years ago

The patch for Bug 730208 causes Thunderbird to print many "JavaScript error: , line 0: nothing active on context" messages. I'm assuming 730208 is a security bug, because I don't have permission to view it.

Unfortunately, the Bugzilla security model is such that I can't even make this bug depend on or block 730208, so if that's appropriate, someone else will need to do it.



Here is an example backtrace:

#0	0x0000000104262ccf in JS_ObjectToInnerObject at /Users/ireid/tbird/comm-central/mozilla/js/src/jsobj.cpp:123
#1	0x000000010411378d in js::GetGlobalForScopeChain(JSContext*) ()
#2	0x00000001041136c7 in JS_GetGlobalForScopeChain ()
#3	0x0000000102035569 in xpc_UnmarkGrayContext(JSContext*) ()
#4	0x000000010202f499 in nsJSContext::GetNativeContext() ()
#5	0x000000010205466e in nsGlobalWindow::SetNewDocument(nsIDocument*, nsISupports*, bool) ()
#6	0x000000010167f486 in DocumentViewerImpl::InitInternal(nsIWidget*, nsISupports*, nsIntRect const&, bool, bool, bool) ()
#7	0x000000010167ea84 in DocumentViewerImpl::Init(nsIWidget*, nsIntRect const&) ()
#8	0x000000010293d36f in nsDocShell::SetupNewViewer(nsIContentViewer*) ()
#9	0x0000000102931ad6 in nsDocShell::Embed(nsIContentViewer*, char const*, nsISupports*) ()
#10	0x00000001029361ea in nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, bool) ()
#11	0x00000001029358ea in nsDocShell::EnsureContentViewer() ()
#12	0x0000000102919034 in nsDocShell::GetInterface(nsID const&, void**) ()
#13	0x000000010291a2e2 in non-virtual thunk to nsDocShell::GetInterface(nsID const&, void**) ()
#14	0x00000001037d7190 in nsGetInterface::operator()(nsID const&, void**) const ()
#15	0x0000000101b19f91 in nsCOMPtr<nsIDOMDocument>::assign_from_helper(nsCOMPtr_helper const&, nsID const&) ()
#16	0x0000000101b19f3d in nsCOMPtr<nsIDOMDocument>::nsCOMPtr(nsCOMPtr_helper const&) ()
#17	0x0000000101b17ffd in nsCOMPtr<nsIDOMDocument>::nsCOMPtr(nsCOMPtr_helper const&) ()
#18	0x0000000102058f49 in nsGlobalWindow::GetDocument(nsIDOMDocument**) ()
#19	0x0000000102092cb8 in nsPIDOMWindow::EnsureInnerWindow() ()
#20	0x000000010208ae4c in nsGlobalWindow::WrapObject(JSContext*, JSObject*, bool*) ()
#21	0x000000010208aecf in non-virtual thunk to nsGlobalWindow::WrapObject(JSContext*, JSObject*, bool*) ()
#22	0x00000001027815ac in XPCConvert::NativeInterface2JSObject(XPCLazyCallContext&, JS::Value*, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, XPCNativeInterface**, bool, unsigned int*) ()
#23	0x0000000102780f9e in XPCConvert::NativeData2JS(XPCLazyCallContext&, JS::Value*, void const*, nsXPTType const&, nsID const*, unsigned int*) ()
#24	0x0000000102789f96 in XPCConvert::NativeData2JS(XPCCallContext&, JS::Value*, void const*, nsXPTType const&, nsID const*, unsigned int*) ()
#25	0x00000001027c102c in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) ()
#26	0x00000001027b614b in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) ()
#27	0x000000010389d5b0 in PrepareAndDispatch ()
#28	0x000000010389c04b in SharedStub ()
#29	0x000000010380a196 in nsObserverList::NotifyObservers(nsISupports*, char const*, unsigned short const*) ()
#30	0x000000010380c527 in nsObserverService::NotifyObservers(nsISupports*, char const*, unsigned short const*) ()
#31	0x00000001029c410f in nsWindowWatcher::AddWindow(nsIDOMWindow*, nsIWebBrowserChrome*) ()
#32	0x00000001029c41f7 in non-virtual thunk to nsWindowWatcher::AddWindow(nsIDOMWindow*, nsIWebBrowserChrome*) ()
#33	0x0000000102a2fd77 in nsAppShellService::RegisterTopLevelWindow(nsIXULWindow*) ()
#34	0x0000000102a2f2cf in nsAppShellService::CreateTopLevelWindow(nsIXULWindow*, nsIURI*, unsigned int, int, int, nsIXULWindow**) ()
#35	0x0000000102a50a22 in nsAppStartup::CreateChromeWindow2(nsIWebBrowserChrome*, unsigned int, unsigned int, nsIURI*, bool*, nsIWebBrowserChrome**) ()
#36	0x0000000102a50b9a in non-virtual thunk to nsAppStartup::CreateChromeWindow2(nsIWebBrowserChrome*, unsigned int, unsigned int, nsIURI*, bool*, nsIWebBrowserChrome**) ()
#37	0x00000001029bea2b in nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow*, char const*, char const*, char const*, bool, nsIArray*, bool, nsIDOMWindow**) ()
#38	0x00000001029bd87d in nsWindowWatcher::OpenWindow(nsIDOMWindow*, char const*, char const*, char const*, nsISupports*, nsIDOMWindow**) ()
#39	0x000000010389bb06 in NS_InvokeByIndex_P ()
#40	0x00000001027d2455 in CallMethodHelper::Invoke() ()
#41	0x00000001027d0d73 in CallMethodHelper::Call() ()
#42	0x00000001027cd71b in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) ()
#43	0x00000001027dd43a in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) ()
#44	0x0000000104398f34 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), js::CallArgs const&) ()
#45	0x000000010423b44a in js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) ()
#46	0x000000010422c56e in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) ()
#47	0x000000010421cdc2 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) ()
#48	0x000000010423b5d5 in js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) ()
#49	0x00000001041910a3 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) ()
#50	0x000000010423bd6d in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) ()
#51	0x0000000104125b49 in JS_CallFunctionValue ()
#52	0x00000001027c161f in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) ()
#53	0x00000001027b614b in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) ()
#54	0x000000010389d5b0 in PrepareAndDispatch ()
#55	0x000000010389c04b in SharedStub ()
#56	0x0000000102a441bb in EnumRun(nsICommandLineHandler*, nsICommandLine*, void*) ()
#57	0x0000000102a438a0 in nsCommandLine::EnumerateHandlers(unsigned int (*)(nsICommandLineHandler*, nsICommandLine*, void*), void*) ()
#58	0x0000000102a440ff in nsCommandLine::Run() ()
#59	0x000000010125b8bf in XREMain::XRE_mainRun() ()
#60	0x000000010125c1a6 in XREMain::XRE_main(int, char**, nsXREAppData const*) ()
#61	0x000000010125c5bc in XRE_main ()
#62	0x0000000100001c32 in do_main(char const*, int, char**) ()
#63	0x00000001000016e3 in main ()

	Steve Fink [:sfink] [:s:] Assignee
	Comment 1 • 12 years ago

Ugh. I didn't realize that path could throw an exception.

Well, fixing it is easy. There are several possible fixes that would all work.

Figuring out which one is the right one is going to take some effort.

	Steve Fink [:sfink] [:s:] Assignee
	Comment 2 • 12 years ago

It is possible to have a JSContext with no global object and no frames on the
stack, yet still be within a request. In that case, xpc_UnmarkGrayContext will
trigger an exception "nothing active on context" when it attempts to access the
inner object of the cx's NULL global.

This patch just skips the UnmarkGray if there is no global object. This culd theoretically miss cases where cx->globalObject is NULL but we have frames on the stack, but (1) I don't think that would ever actually happen, (2) cx->globalObject will probably be eliminated or nerfed soon now that we have CPG, and (3) it's not like our UnmarkGray coverage is 100% complete to begin with.

	:Irving Reid (No longer working on Firefox) Reporter
	Comment 3 • 12 years ago

Thanks for jumping on this so quickly.

Based on a quick build and some manual tests, attachment 621682 [details] [diff] [review] makes the warning messages go away and does not cause any unstable behaviour. Do you want a Thunderbird Try build with full tests on this?

	Steve Fink [:sfink] [:s:] Assignee
	Comment 4 • 12 years ago

(In reply to Irving Reid (:irving) from comment #3)
> Thanks for jumping on this so quickly.
> 
> Based on a quick build and some manual tests, attachment 621682 [details] [diff] [review]
> [diff] [review] makes the warning messages go away and does not cause any
> unstable behaviour. Do you want a Thunderbird Try build with full tests on
> this?

I don't think so. If my test turns out to be too inclusive, it just means we're partway back to the way things were before the bug 730208 patch; I'm not worried about introducing random failures from that. And the error you're seeing doesn't seem to get caught by any of the Firefox automated tests, at least, so I doubt the Thunderbird ones will detect it either (other than possibly by spewing warnings into the logs.)

Thanks for noticing and reporting this, though. Apparently it's happening with Firefox too.

	Bobby Holley (:bholley)
	Comment 5 • 12 years ago

Comment on attachment 621682 [details] [diff] [review]
xpc_UnmarkGrayContext should never throw an exception

Delegating to mccr8.

	Andrew McCreight [:mccr8]
	Comment 6 • 12 years ago

Comment on attachment 621682 [details] [diff] [review]
xpc_UnmarkGrayContext should never throw an exception

Would you mind looking at this, Bill?  You reviewed the original code, and I don't really understand what is going on here.  If you don't have time, I can go read up on what a request is.  It is a short patch.

	Steve Fink [:sfink] [:s:] Assignee
	Comment 7 • 12 years ago

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

	Serge Gautherie (:sgautherie)
	Comment 9 • 12 years ago

Comment on attachment 621682 [details] [diff] [review]
xpc_UnmarkGrayContext should never throw an exception

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

::: js/xpconnect/src/xpcpublic.h
@@ +203,5 @@
>  {
>      if (cx) {
>          JSObject *global = JS_GetGlobalObject(cx);
>          xpc_UnmarkGrayObject(global);
> +        if (global && JS_IsInRequest(JS_GetRuntime(cx))) {

Nit: it seems useless to call xpc_UnmarkGrayObject(global); with a null global...

	Joe Drew (not getting mail)
	Comment 10 • 12 years ago

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