Closed Bug 852436 Opened 12 years ago Closed 12 years ago

crash in XPCWrappedNative::GetNewOrUsed @ JSAutoCompartment::JSAutoCompartment

Categories

(Core :: XPConnect, defect)

22 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- unaffected
firefox22 + verified

People

(Reporter: scoobidiver, Assigned: mrbkap)

References

Details

(4 keywords)

Crash Data

Attachments

(3 files)

With the below stack trace, it first showed up in 22.0a1/20130313. There was a hole in 22.0a1/20130316 and 17 because of bug 851806, bug 851807 and 851851. The regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7433bc4545c9&tochange=c1a5c44ae3d8 Signature JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*) More Reports Search UUID 4369d6fe-215f-48fb-b95e-c4c182130318 Date Processed 2013-03-18 18:41:21 Uptime 45 Install Age 45 seconds since version was first installed. Install Time 2013-03-18 18:40:24 Product Firefox Version 22.0a1 Build ID 20130318030947 Release Channel nightly OS Windows NT OS Version 6.1.7601 Service Pack 1 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 37 stepping 5 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0xffffffffdadadada App Notes AdapterVendorID: 0x1002, AdapterDeviceID: 0x68a0, AdapterSubsysID: 107a1462, AdapterDriverVersion: 8.751.0.0 D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ Processor Notes sp-processor05.phx1.mozilla.com_13244:2008 EMCheckCompatibility True Adapter Vendor ID 0x1002 Adapter Device ID 0x68a0 Total Virtual Memory 4294836224 Available Virtual Memory 3756597248 System Memory Use Percentage 46 Available Page File 6154903552 Available Physical Memory 2255372288 Frame Module Signature Source 0 mozjs.dll JSAutoCompartment::JSAutoCompartment js/src/jsapi.cpp:1485 1 xul.dll XPCWrappedNative::GetNewOrUsed js/xpconnect/src/XPCWrappedNative.cpp:530 2 xul.dll XPCConvert::NativeInterface2JSObject js/xpconnect/src/XPCConvert.cpp:925 3 xul.dll nsGlobalChromeWindow::QueryInterface dom/base/nsGlobalWindow.cpp:11090 4 mozglue.dll je_free memory/mozjemalloc/jemalloc.c:6589 5 xul.dll nsJSArgArray::Release dom/base/nsJSEnvironment.cpp:3871 6 xul.dll nsCOMPtr_base::~nsCOMPtr_base obj-firefox/dist/include/nsAutoPtr.h:880 7 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70 8 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1417 9 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:384 10 mozjs.dll js::Interpret js/src/jsinterp.cpp:2397 11 mozjs.dll js::RunScript js/src/jsinterp.cpp:333 12 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:398 13 mozjs.dll js::Invoke js/src/jsinterp.cpp:431 14 mozjs.dll JS_CallFunctionValue js/src/jsapi.cpp:5760 15 xul.dll mozilla::dom::EventHandlerNonNull::Call obj-firefox/dom/bindings/EventHandlerBinding.cpp:51 16 xul.dll mozilla::dom::EventHandlerNonNull::Call<nsISupports*> obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:59 17 xul.dll nsJSEventListener::HandleEvent dom/src/events/nsJSEventListener.cpp:250 18 xul.dll nsEventListenerManager::HandleEventInternal content/events/src/nsEventListenerManager.cpp:994 19 xul.dll nsEventTargetChainItem::HandleEventTargetChain content/events/src/nsEventDispatcher.cpp:328 20 xul.dll nsEventDispatcher::Dispatch content/events/src/nsEventDispatcher.cpp:634 21 mozglue.dll je_malloc memory/mozjemalloc/jemalloc.c:6294 22 xul.dll nsEventDispatcher::Dispatch content/events/src/nsEventDispatcher.cpp:575 23 xul.dll nsEventStateManager::UpdateAncestorState content/events/src/nsEventStateManager.cpp:4761 24 xul.dll testSortCallback content/xul/templates/src/nsXULSortService.cpp:206 25 xul.dll nsEventStateManager::PostHandleEvent content/events/src/nsEventStateManager.cpp:3291 26 xul.dll PresShell::HandleEventInternal layout/base/nsPresShell.cpp:6872 ... More reports at: https://crash-stats.mozilla.com/report/list?signature=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext*%2C+JSObject*%29
This looks bad. Can we try to get STR? Looks like at least one reporter mentions bbc.co.uk.
Keywords: steps-wanted
(In reply to Bobby Holley (:bholley) from comment #1) > Looks like at least one reporter mentions bbc.co.uk. It's about a crash in 19.0.2 with a different stack trace. See instead https://crash-stats.mozilla.com/report/list?version=Firefox:22.0a1&signature=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext*%2C+JSObject*%29
Crash Signature: [@ JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*)] → [@ JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*)] [@ JSAutoCompartment::JSAutoCompartment ]
OS: Windows 7 → All
It might be a dupe of bug 850741.
It's #11 top browser crasher in 22.0a1.
Keywords: topcrash
It seems correlated to Self-Destructing Cookies (https://addons.mozilla.org/firefox/addon/self-destructing-cookies): JSAutoCompartment::JSAutoCompartment|SIGSEGV (13 crashes) 85% (11/13) vs. 18% (11/62) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*)|EXCEPTION_ACCESS_VIOLATION_READ (55 crashes) 38% (21/55) vs. 1% (33/2785) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack
Both crashes I've had with this signature today have been opening an alert service, could be caused by bug 782211
I had 2 crashes with this, both while the only thing going on was downloading update from Help -> About.
Actually, reading comment 6 it is possible what actually caused the crash was Sync trying to open a master password dialog alert.
(In reply to Scoobidiver from comment #3) > It might be a dupe of bug 850741. As we have assigned that one to Bill, we should at least have him CCed here. I'd like to get this one assigned to someone as well, even if just for investigating, who can take it? This now is #5 topcrash on Nightly over the last 7 days, and when looking at crashes from the last 3 days' builds, it's actually #1 by a margin: https://crash-stats.mozilla.com/topcrasher/by_build_date/Firefox/22.0a1/3/browser
Blocks: 854025
(In reply to Scoobidiver from comment #5) > It seems correlated to Self-Destructing Cookies > (https://addons.mozilla.org/firefox/addon/self-destructing-cookies): > JSAutoCompartment::JSAutoCompartment|SIGSEGV (13 crashes) > 85% (11/13) vs. 18% (11/62) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack > > JSAutoCompartment::JSAutoCompartment(JSContext*, > JSObject*)|EXCEPTION_ACCESS_VIOLATION_READ (55 crashes) > 38% (21/55) vs. 1% (33/2785) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack I agree. I started having this problem after I installed self-destructing cookies. I will disable it and see if the crash goes away.
(In reply to webmaster from comment #10) > (In reply to Scoobidiver from comment #5) > > It seems correlated to Self-Destructing Cookies > > (https://addons.mozilla.org/firefox/addon/self-destructing-cookies): > > JSAutoCompartment::JSAutoCompartment|SIGSEGV (13 crashes) > > 85% (11/13) vs. 18% (11/62) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack > > > > JSAutoCompartment::JSAutoCompartment(JSContext*, > > JSObject*)|EXCEPTION_ACCESS_VIOLATION_READ (55 crashes) > > 38% (21/55) vs. 1% (33/2785) jid0-9XfBwUWnvPx4wWsfBWMCm4Jj69E@jetpack > > I agree. I started having this problem after I installed self-destructing > cookies. I will disable it and see if the crash goes away. I've gone from one crash every ~15 mins to none for the past two hours after disabling it. I will continue monitoring.
I'm definitely hitting this frequently after one of the new desktop notifications is shown/interacted with. But not 100% of the time. I've seen it both from Facebook (via SocialAPI integration), and from the Copy ShortURL addon (which shows a notification after being triggered). The latter is especially interesting, because I've been using that addon for a long time.
Assignee: nobody → wmccloskey
It sounds like there's some hope of getting an STR here.
Keywords: qawanted
So I have been able to reproduce with self-destruct cookies add-on. STR: 0) install the self-destruct cookies add-on from amo. enable the add-ons bar 1) visit a random site. I've used yahoo.com, cnn.com and golfchannel.com 2) click the add-on icon in botton right corner and select to remove cookies when a tab is close 3) open another tab with random site. close the tab in step 2. 4) wait some 10-15 seconds. tested results: crash as reported. note: after one one of these crashes, the add-on icon was missing from the add-ons bar. disable and re-enable the add-on fixed that. one of my crashes: https://crash-stats.mozilla.com/report/index/bp-0a0ebf62-d938-4249-a25c-e06702130325
sync calling master password dialog does not cause this crash for me.
Thanks, this is perfect. Those steps work for me.
Attached file stack trace for assertion (deleted) —
Bobby and I looked into this crash for a little while today. I added some printfs and now I have a decent idea of what's going on, but I don't yet know how all the pieces fit together. We're asserting with the stack trace that's attached. The basic problem is that we're ending up with an outer window in nsWindowSH::PreCreate, which is only supposed to get inner windows. I'm not sure which caller in this stack is responsible for ensuring that no outer windows end up here. Bobby thinks maybe it's nsContentUtils::WrapNative. I'm CCing bz in case he knows more. Also, we're not sure if this outer/inner issue is actually contributing to the crash at all. I instrumented the code to figure out the order of events. For the window in question, here's what happens: 1. We set mJSObject to an nsOuterWindowProxy wrapper object during SetNewDocument. 2. Shortly after that, nsGlobalWindow::FinalClose is called on the window. 3. Later on, nsOuterWindowProxy::finalize is called on the JS object in step 1. Crucially, we do not clear the mJSObject field of the window. 4. Then we assert with the stack that's attached. This happens before the destructor is called on the window, so it's still alive somehow. As far as I can tell, failing to clear mJSObject is pretty busted. However, even if we did clear it, we'd still crash with a NULL pointer deref after the PreCreate hook returns a NULL parent. So that seems broken too. Also, there's the matter of this inner/outer assertion.
Keywords: reproducible
Actually, it looks like SetParentToWindow correctly handles the NULL case by returning an error. So setting mJSObject to NULL in nsOuterWindowProxy::finalize would at least stop us from crashing, I think.
(In reply to Bill McCloskey (:billm) from comment #19) > Actually, it looks like SetParentToWindow correctly handles the NULL case by > returning an error. So setting mJSObject to NULL in > nsOuterWindowProxy::finalize would at least stop us from crashing, I think. Yeah, we should definitely do that, unless there's something I'm missing. The other question is what to do about outer windows passed to WrapNative. Comment 17 isn't quite right in that my belief is that it's up the caller to avoid passing an outer nsGlobalWindow to WrapNative. I think mrbkap knows the situation here better than anyone. Blake, can you weigh on on the above two points?
Flags: needinfo?(mrbkap)
After going from crashes around every 10 minutes for weeks when I had the addon to no crashes for four days after I disabled it, I can confidently say that this crash is caused by the Self-Destructing Cookies (https://addons.mozilla.org/en-us/firefox/addon/self-destructing-cookies/) addon. Please notify me if anyone has experienced the crash and does not have the addon installed.
Attached patch Proposed fix (deleted) — Splinter Review
After talking to bz, we determined that this patch is safe. I'll file a new bug to finish cleaning up some of the nsWrapperCache::WrapObject failure cases. The reason for the crash is that outer windows (and not inner windows) implement nsWrapperCache and are "DOM objects" (via IsDOMObject). They are meant to fall into that case and expect that returning null will mean that they fail to wrap (even though they don't set an exception). This patch treats all null returns from WrapObject as failure, preventing us from trying to incorrectly use the scriptable helper hooks on the outer window.
Assignee: wmccloskey → mrbkap
Status: NEW → ASSIGNED
Attachment #730480 - Flags: review?(bzbarsky)
Flags: needinfo?(mrbkap)
Blocks: 851392
Comment on attachment 730480 [details] [diff] [review] Proposed fix r=me, but a pox upon this no-brace style. ;)
Attachment #730480 - Flags: review?(bzbarsky) → review+
Is the mJSObject thing an issue? Even if we're guaranteed never to access it after it's finalized, it would be nice to poison it or something. Also, could we move the IsInnerWindow assertion higher up in the callstack?
(In reply to Bill McCloskey (:billm) from comment #24) > Is the mJSObject thing an issue? Even if we're guaranteed never to access it > after it's finalized, it would be nice to poison it or something. Also, > could we move the IsInnerWindow assertion higher up in the callstack? I can definitely clear mJSObject. Moving the IsInnerWindow assertion higher is more challenging, though. The first place we call into window-specific code in this case is in nsWindowSH::PreCreate, everything above it on the stack deals with either nsISupports or nsWrapperCache. I'll attach a patch tomorrow that clears mJSObject.
Attachment #730820 - Flags: review?(bzbarsky)
Comment on attachment 730820 [details] [diff] [review] Clear the outer window's mJSObject This is orange on try.
Attachment #730820 - Flags: review?(bzbarsky)
I filed bug 855891 on figuring out what to do with the rest of the WrapObject callers.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 730480 [details] [diff] [review] Proposed fix Review of attachment 730480 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCConvert.cpp @@ +845,3 @@ > } > > if (flat) { This check looks awfully dead now.
(In reply to :Ms2ger from comment #32) > This check looks awfully dead now. It isn't in the case that there was a cached DOM object already in the cache.
Could we keep this open until the other patch lands, or else move that to a different bug? I want to make sure we don't lose track of it.
Is this supposed to be fixed in the latest Nightly? I got https://crash-stats.mozilla.com/report/index/bp-ba40a80a-8b5e-454f-89ab-20cb02130401 just a few minutes ago with the 2013-3-31 Nightly and clicking a video from the videos listed after the video has ended.
The buildid (20130330030828) in that crash report indicates you were actually running the 3/30 nightly. Therefore, It did NOT contain the fix here.
(In reply to Bill McCloskey (:billm) from comment #34) > Could we keep this open until the other patch lands, or else move that to a > different bug? I want to make sure we don't lose track of it. I filed bug 856818 on the other patch.
Verified that Firefox 22 RC1 does not crash when using the steps from Comment 14 (verified on Mac OS X 10.7, Windows 7 and Ubuntu 12.10) Checked the Socorro reports and there are around 700 crashes related with the signature:[@ JSAutoCompartment::JSAutoCompartment(JSContext*, JSObject*)] https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=JSAutoCompartment%3A%3AJSAutoCompartment%28JSContext%2A%2C%20JSObject%2A%29 Is this expected in any way?
(In reply to Simona B [QA] from comment #41) > Is this expected in any way? It's only expected in that JSAutoCompartment is a relatively likely place for us to crash if something goes wrong so we tend to collect possibly-unrelated crashes there. The remaining crashes aren't related directly to the bug that was fixed here.
(In reply to Blake Kaplan (:mrbkap) from comment #42) > (In reply to Simona B [QA] from comment #41) > > Is this expected in any way? > > It's only expected in that JSAutoCompartment is a relatively likely place > for us to crash if something goes wrong so we tend to collect > possibly-unrelated crashes there. The remaining crashes aren't related > directly to the bug that was fixed here. Does that mean we should add this frame to the "prefix list" in Socorro and have the next frame added to it to form a signature (which hopefully will be more specific to a certain problem then)?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #44) > Does that mean we should add this frame to the "prefix list" in Socorro and > have the next frame added to it to form a signature (which hopefully will be > more specific to a certain problem then)? Adding the JSAutoCompartment constructor to the prefix list would be a good idea, yes.
Depends on: 885443
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: