Assertion failure: mGlobalObject->GetGlobalJSObject() (Must have an actual JS global for the duration on the stack), at ../../dist/include/mozilla/dom/ScriptSettings.h:79 The assertion failure happens shortly after opening about:addons, disabling a restartless add-on (Whimsy, in my tests), and then navigating away from about:addons. Firefox Trunk build with --enable-debug, --disable-optimize, --enable-tests, --enable-profiling Irvings-MozBook% clang --version Apple LLVM version 5.0 (clang-500.2.75) (based on LLVM 3.3svn) Target: x86_64-apple-darwin12.5.0 Thread model: posix Process: firefox [28906] Path: /Users/USER/*/NightlyDebug.app/Contents/MacOS/firefox Identifier: org.mozilla.nightlydebug Version: 30.0a1 (3014.2.20) Code Type: X86-64 (Native) Parent Process: Python [28894] User ID: 501 Date/Time: 2014-02-21 09:41:49.139 -0500 OS Version: Mac OS X 10.8.5 (12F45) Report Version: 10 Interval Since Last Report: 74390 sec Crashes Since Last Report: 1 Per-App Interval Since Last Report: 967 sec Per-App Crashes Since Last Report: 1 Anonymous UUID: 897C5300-2DBD-1EBF-695C-45E14DF340BB Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000 VM Regions Near 0: --> __TEXT 0000000100000000-000000010000b000 [ 44K] r-x/rwx SM=COW /Users/USER/*/NightlyDebug.app/Contents/MacOS/firefox Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 XUL 0x000000010334cdf6 mozilla::dom::ScriptSettingsStackEntry::ScriptSettingsStackEntry(nsIGlobalObject*, bool) + 262 (ScriptSettings.h:78) 1 XUL 0x000000010332818c mozilla::dom::AutoIncumbentScript::AutoIncumbentScript(nsIGlobalObject*) + 44 (ScriptSettings.cpp:233) 2 XUL 0x000000010332814d mozilla::dom::AutoIncumbentScript::AutoIncumbentScript(nsIGlobalObject*) + 29 (ScriptSettings.cpp:237) 3 XUL 0x0000000102e8638c void mozilla::Maybe<mozilla::dom::AutoIncumbentScript>::construct<nsIGlobalObject*>(nsIGlobalObject* const&) + 156 (Maybe.h:55) 4 XUL 0x0000000102e7e4b6 mozilla::dom::CallbackObject::CallSetup::CallSetup(mozilla::dom::CallbackObject*, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*, bool) + 1222 (CallbackObject.cpp:138) 5 XUL 0x0000000102e7dfe4 mozilla::dom::CallbackObject::CallSetup::CallSetup(mozilla::dom::CallbackObject*, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*, bool) + 68 (CallbackObject.cpp:169) 6 XUL 0x000000010347524c void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, nsDOMEvent&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling) + 124 (EventListenerBinding.h:41) 7 XUL 0x0000000103466de6 nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEvent*, mozilla::dom::EventTarget*) + 326 (nsEventListenerManager.cpp:957) 8 XUL 0x0000000103467142 nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) + 706 (nsEventListenerManager.cpp:1020) 9 XUL 0x000000010347f8e9 nsEventListenerManager::HandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) + 345 (nsEventListenerManager.h:327) 10 XUL 0x0000000103473d0f nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&) + 479 (nsEventDispatcher.cpp:193) 11 XUL 0x000000010345f8e8 nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&) + 296 (nsEventDispatcher.cpp:263) 12 XUL 0x0000000103461221 nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) + 4801 (nsEventDispatcher.cpp:597) 13 XUL 0x00000001034615d8 nsEventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) + 376 (nsEventDispatcher.cpp:658) 14 XUL 0x000000010398b742 nsINode::DispatchEvent(nsIDOMEvent*, bool*) + 226 (nsINode.cpp:1165) 15 XUL 0x0000000103874c85 nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) + 613 (nsContentUtils.cpp:3378) 16 XUL 0x0000000103874a14 nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) + 116 (nsContentUtils.cpp:3348) 17 XUL 0x0000000103825b5e nsDocument::DispatchContentLoadedEvents() + 430 (nsDocument.cpp:4694) 18 XUL 0x0000000103e58a8c mozilla::dom::XULDocument::DoneWalking() + 1356 (XULDocument.cpp:3233) 19 XUL 0x0000000103e4cd4e mozilla::dom::XULDocument::ResumeWalk() + 4302 (XULDocument.cpp:3163) 20 XUL 0x0000000103e599e7 mozilla::dom::XULDocument::OnScriptCompileComplete(JSScript*, tag_nsresult) + 567 (XULDocument.cpp:3631) 21 XUL 0x0000000103e5975b mozilla::dom::XULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, tag_nsresult, unsigned int, unsigned char const*) + 1579 (XULDocument.cpp:3550) 22 XUL 0x0000000103e59b6d non-virtual thunk to mozilla::dom::XULDocument::OnStreamComplete(nsIStreamLoader*, nsISupports*, tag_nsresult, unsigned int, unsigned char const*) + 77 (Unified_cpp_content_xul_document_src0.cpp:3551) 23 XUL 0x00000001017f4ae3 nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) + 211 (nsStreamLoader.cpp:100) 24 XUL 0x000000010177d951 nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) + 257 (nsBaseChannel.cpp:732) 25 XUL 0x000000010177da3d non-virtual thunk to nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) + 61 (Unified_cpp_netwerk_base_src0.cpp:745) 26 XUL 0x00000001017b8753 nsInputStreamPump::OnStateStop() + 1059 (nsInputStreamPump.cpp:704) 27 XUL 0x00000001017b7801 nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) + 433 (nsInputStreamPump.cpp:438) 28 XUL 0x00000001017b887f non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) + 47 (Unified_cpp_netwerk_base_src1.cpp:490) 29 XUL 0x00000001016775a0 nsInputStreamReadyEvent::Run() + 144 (nsStreamUtils.cpp:85) 30 XUL 0x00000001016a1079 nsThread::ProcessNextEvent(bool, bool*) + 1561 (nsThread.cpp:644) 31 XUL 0x00000001015a2c4b NS_ProcessPendingEvents(nsIThread*, unsigned int) + 171 (nsThreadUtils.cpp:210) 32 XUL 0x000000010303d8c9 nsBaseAppShell::NativeEventCallback() + 201 (nsBaseAppShell.cpp:99) 33 XUL 0x0000000102fc9bdc nsAppShell::ProcessGeckoEvents(void*) + 428 (nsAppShell.mm:388) 34 com.apple.CoreFoundation 0x00007fff91783b31 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 35 com.apple.CoreFoundation 0x00007fff91783455 __CFRunLoopDoSources0 + 245 36 com.apple.CoreFoundation 0x00007fff917a67f5 __CFRunLoopRun + 789 37 com.apple.CoreFoundation 0x00007fff917a60e2 CFRunLoopRunSpecific + 290 38 com.apple.HIToolbox 0x00007fff89531eb4 RunCurrentEventLoopInMode + 209 39 com.apple.HIToolbox 0x00007fff89531b94 ReceiveNextEventCommon + 166 40 com.apple.HIToolbox 0x00007fff89531ae3 BlockUntilNextEventMatchingListInMode + 62 41 com.apple.AppKit 0x00007fff8e55f533 _DPSNextEvent + 685 42 com.apple.AppKit 0x00007fff8e55edf2 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 43 XUL 0x0000000102fc8b47 -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 119 (nsAppShell.mm:165) 44 com.apple.AppKit 0x00007fff8e5561a3 -[NSApplication run] + 517 45 XUL 0x0000000102fca6c2 nsAppShell::Run() + 162 (nsAppShell.mm:742) 46 XUL 0x0000000104c73a1c nsAppStartup::Run() + 156 (nsAppStartup.cpp:276) 47 XUL 0x0000000104b2cb6c XREMain::XRE_mainRun() + 6044 (nsAppRunner.cpp:3954) 48 XUL 0x0000000104b2d379 XREMain::XRE_main(int, char**, nsXREAppData const*) + 697 (nsAppRunner.cpp:4021) 49 XUL 0x0000000104b2d7ed XRE_main + 77 (nsAppRunner.cpp:4231) 50 org.mozilla.nightlydebug 0x0000000100002067 do_main(int, char**, nsIFile*) + 1639 (nsBrowserApp.cpp:282) 51 org.mozilla.nightlydebug 0x00000001000015a1 main + 321 (nsBrowserApp.cpp:643) 52 org.mozilla.nightlydebug 0x0000000100001004 start + 52

Ok. So this basically means that the incumbent global stashed on the callback object went away by the time we want to invoke that callback, even though the EventListener is still alive (presumably held alive by the EventTarget). So we have 3 options, all of them kind of crappy: (1) The Callback holds the incumbent global alive in addition to the callable object and its global. (2) If the incumbent global has gone away, we don't fire the callback. (3) If the incumbent global has gone away, we fire the callback without restoring the incumbent (and just use the entry script, which is the global object of the underlying callable). (1) is the simplest, but it's annoying to hold things alive just for the sake of a somewhat-arcane spec concept. The other 2 are non-deterministics, which isn't great. (3) is kind of off the table IMO. I might vote for (1), on the grounds that it's probably pretty rare to get into this situation, and this is the only situation where we'd be holding an additional global alive. Thoughts Boris? Whatever we decide, the spec probably needs to be updated to say something about this.

#1 means we're entraining an entirely new global which is suboptimal from a memory usage perspective. It might be the least bad option though.

Per spec, nothing ever gets observably GCed, so the situation in question can't arise. I'd probably be OK with #1 here; it's pretty rare to have an incumbent global for a callback that doesn't match the global of the callback _or_ the global of the thing that's holding on to the callback.

(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #3) > Per spec, nothing ever gets observably GCed, so the situation in question > can't arise. That's not really the case IIUC. The spec definitely allows globals to get observably GCed. Consider the cases with pageshow vs onload, and the reconstruction of session history entries with dynamic iframes. When we talked about it, Hixie said he thought it was pretty hopeless to avoid that in the spec, and was working on making the interactions as simple as possible. Now, whether that means that this itself is spec-relevant, I don't know. The spec could probably use clarifying on this point at the very least. > I'd probably be OK with #1 here; it's pretty rare to have an incumbent > global for a callback that doesn't match the global of the callback _or_ the > global of the thing that's holding on to the callback. Ok. I'll whip up a patch.

Does this only arise in cases like XPIProvider, where we're explicitly unloading chrome code that may have left callbacks behind, or does it also affect window contents? For chrome code, if it's feasible, it would be helpful to get a warning or exception at unload time telling us we're leaving behind orphaned callbacks.

> Consider the cases with pageshow vs onload, That's not a GC issue. That's a "cached presentation" issue. It no more exposes GC behavior than the HTTP cache does.

> or does it also affect window contents? Seems to me like it can affect any global.

Hm. So in trying to trace the various JSObjects inside nsIGlobalObject implementations, I've run into the snag that TraceCallbacks only takes a JS::Heap<JSObject*>, not a JS::TenuredHeap<JSObject*>. Here are some options I can think of: * Add another overload of Trace(..) to TraceCallbacks. Yuck. * Make TenuredHeap<T> inherit Heap<T> somehow. * reinterpret_cast<Heap<T>>(mJSObject) (is this valid? Seems dicey). Thoughts?

At a lower level, it looks like JS_CallTenuredObjectTracer needs to be called, but that doesn't really help when writing a TRACE thing. I'd guess that adding a new Trace thing to TraceCallbacks is the most sensible option, but I think Terrence or Jon are better people to answer this. (Though it looks like Jon is away at the moment.)

Ok. What do I do about SnowWhiteKiller::Trace? It looks like the PurpleBuffer contains an array of JS::Heap<JSObject*> instances. http://dxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp#2387

I think you should just be able to dig out the JS object and stick it in the array with the rest of them.

Oh, gross, you have to add a new array, I guess.

The only reason TraceCallbacks doesn't have any Tenuredheap<> overloads is that we haven't needed them yet. Let's just update the existing tracing infrastructure to support them. The need for TenuredHeap at all is a bit sadfaces, but that's just the cost of performance. See the comment here for details: http://dxr.mozilla.org/mozilla-central/source/js/public/RootingAPI.h#290

Built a new debug/profiling Trunk with these patches applied; my test no longer crashes, but it has started printing JavaScript error: , line 0: can't access dead object each time I navigate between pages after disabling the Whimsy extension.

Comment on attachment 8383479 [details] [diff] [review] Part 3 - Trace the Incumbent Global from a CallbackObject (but check it too, just to be safe). v1 r=me

Comment on attachment 8383476 [details] [diff] [review] Part 1 - Used a TenuredHeap pointer for SandboxPrivate and BackstagePass. v1 Review of attachment 8383476 [details] [diff] [review]: ----------------------------------------------------------------- These are both weak references, like the global on nsGlobalWindow?

Comment on attachment 8383477 [details] [diff] [review] Part 2 - Add a JS::TenuredHeap<JSObject*> overload to TraceCallbacks. v1 Review of attachment 8383477 [details] [diff] [review]: ----------------------------------------------------------------- Terrence should review the RootingAPI.h change, as small as it is.

Comment on attachment 8383479 [details] [diff] [review] Part 3 - Trace the Incumbent Global from a CallbackObject (but check it too, just to be safe). v1 Review of attachment 8383479 [details] [diff] [review]: ----------------------------------------------------------------- r=me I don't understand the CallbackObject::CallSetup::CallSetup stuff, but presumably bz has that covered. Thanks for hooking up the tenured heap tracing stuff! You've saved somebody else some work in the future.

(In reply to Andrew McCreight [:mccr8] from comment #20) > These are both weak references, like the global on nsGlobalWindow? Correct.

Comment on attachment 8383477 [details] [diff] [review] Part 2 - Add a JS::TenuredHeap<JSObject*> overload to TraceCallbacks. v1 Review of attachment 8383477 [details] [diff] [review]: ----------------------------------------------------------------- The trace callback changes look good to me. r=me

Final all-platform try push: https://tbpl.mozilla.org/?tree=Try&rev=809cc6459d6d

Wow, that totally wasn't all-platform. https://tbpl.mozilla.org/?tree=Try&rev=8d8b86542e25

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/4d8af8698173 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/058ed6c240bb remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1742ecba9c09

https://hg.mozilla.org/mozilla-central/rev/4d8af8698173 https://hg.mozilla.org/mozilla-central/rev/058ed6c240bb https://hg.mozilla.org/mozilla-central/rev/1742ecba9c09