Closed Bug 635873 Opened 9 years ago Closed 9 years ago
Assertion failure: compartment == rt->gc
1. Install Spider http://bclary.com/projects/spider/spider/spider.xpi 2. firefox -spider -url https://www.citibank.hu/HUGCB/jba/exp/ProcessPaymentInputScreen.do -start -quit 3. Assertion failure: compartment == rt->gcCurrentCompartment on shutdown Spider is required. SS since it involves shapes+compartments+GC. Operating system: Windows NT 5.1.2600 Service Pack 3 CPU: x86 GenuineIntel family 6 model 44 stepping 2 1 CPU Crash reason: EXCEPTION_ACCESS_VIOLATION_WRITE Crash address: 0x0 Thread 0 (crashed) 0 mozjs.dll!JS_Assert [jsutil.cpp : 73 + 0x0] eip = 0x0084c2aa esp = 0x0012abe0 ebp = 0x0012abe0 ebx = 0x00000001 esi = 0x12b6b820 edi = 0xffff0007 eax = 0x00000000 ecx = 0x8ef81597 edx = 0x003b3d38 efl = 0x00010212 Found by: given as instruction pointer in context 1 mozjs.dll!js::Shape::trace(JSTracer *) [jsscope.cpp : 1488 + 0x30] eip = 0x0081a045 esp = 0x0012abe8 ebp = 0x0012abfc Found by: call frame info 2 mozjs.dll!js::Bindings::trace(JSTracer *) [jsscript.cpp : 294 + 0xb] eip = 0x0081c380 esp = 0x0012ac04 ebp = 0x0012ac10 Found by: call frame info 3 mozjs.dll!js_TraceScript(JSTracer *,JSScript *) [jsscript.cpp : 1709 + 0xe] eip = 0x0082036a esp = 0x0012ac18 ebp = 0x0012ac34 Found by: call frame info 4 mozjs.dll!js_TraceStackFrame(JSTracer *,JSStackFrame *) [jsgc.cpp : 1489 + 0x11] eip = 0x0073d124 esp = 0x0012ac3c ebp = 0x0012ac44 Found by: call frame info 5 mozjs.dll!js::StackSpace::mark(JSTracer *) [jscntxt.cpp : 242 + 0xc] eip = 0x006e46eb esp = 0x0012ac4c ebp = 0x0012ac6c Found by: call frame info 6 mozjs.dll!JSThreadData::mark(JSTracer *) [jscntxt.cpp : 536 + 0xe] eip = 0x006e6116 esp = 0x0012ac74 ebp = 0x0012ac7c Found by: call frame info 7 mozjs.dll!js::MarkRuntime(JSTracer *) [jsgc.cpp : 1739 + 0x12] eip = 0x0073e08b esp = 0x0012ac84 ebp = 0x0012acd0 Found by: call frame info 8 mozjs.dll!MarkAndSweepCompartment [jsgc.cpp : 2292 + 0x8] eip = 0x007400e4 esp = 0x0012acd8 ebp = 0x0012ad68 Found by: call frame info 9 mozjs.dll!GCUntilDone [jsgc.cpp : 2748 + 0x10] eip = 0x0073fbf3 esp = 0x0012ad70 ebp = 0x0012ada8 Found by: call frame info 10 mozjs.dll!js_GC(JSContext *,JSCompartment *,JSGCInvocationKind) [jsgc.cpp : 2819 + 0x10] eip = 0x0073f8fc esp = 0x0012adb0 ebp = 0x0012add8 Found by: call frame info 11 mozjs.dll!js::MaybeGC(JSContext *) [jsgc.cpp : 1838 + 0x20] eip = 0x0073e7a7 esp = 0x0012ade0 ebp = 0x0012adf4 Found by: call frame info 12 mozjs.dll!JS_MaybeGC [jsapi.cpp : 2575 + 0x8] eip = 0x006a79b5 esp = 0x0012adfc ebp = 0x0012ae00 Found by: call frame info 13 xul.dll!nsJSContext::ScriptEvaluated(int) [nsJSEnvironment.cpp : 3173 + 0xc] eip = 0x10d640ef esp = 0x0012ae08 ebp = 0x0012ae24 Found by: call frame info 14 xul.dll!nsJSContext::ScriptExecuted() [nsJSEnvironment.cpp : 3245 + 0x28] eip = 0x10d6432c esp = 0x0012ae2c ebp = 0x0012ae30 Found by: call frame info 15 xul.dll!AutoScriptEvaluate::~AutoScriptEvaluate() [xpcwrappedjsclass.cpp : 122 + 0x18] eip = 0x112a5567 esp = 0x0012ae38 ebp = 0x0012ae4c Found by: call frame info 16 xul.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS *,unsigned short,XPTMethodDescriptor const *,nsXPTCMiniVariant *) [xpcwrappedjsclass.cpp : 1911 + 0x29] eip = 0x112aa834 esp = 0x0012ae54 ebp = 0x0012b28c Found by: call frame info 17 xul.dll!nsXPCWrappedJS::CallMethod(unsigned short,XPTMethodDescriptor const *,nsXPTCMiniVariant *) [xpcwrappedjs.cpp : 588 + 0x29] eip = 0x112a4413 esp = 0x0012b294 ebp = 0x0012b2cc Found by: call frame info 18 xul.dll!PrepareAndDispatch [xptcstubs.cpp : 114 + 0x20] eip = 0x11581306 esp = 0x0012b2d4 ebp = 0x0012b3a0 Found by: call frame info 19 xul.dll!SharedStub [xptcstubs.cpp : 141 + 0x4] eip = 0x11580fd6 esp = 0x0012b3a8 ebp = 0x0012b3bc Found by: call frame info 20 xul.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct *,nsIDOMEventListener *,nsIDOMEvent *,nsPIDOMEventTarget *,unsigned int,nsCxPusher *) [nsEventListenerManager.cpp : 1127 + 0x11] eip = 0x10be1cef esp = 0x0012b3c4 ebp = 0x0012b3bc Found by: call frame info with scanning
This might be a bad bug. Bill can you please investigate?
So far it doesn't look like this can cause anything bad to happen. Right now, the only code that stops us from crossing compartments while marking are some checks in the marking code for GC things (objects, strings, etc.). In the stack in comment 0, we don't pass through any of those checks. That's why the assertion triggers. The danger when marking outside your compartment is that, if the mark bit isn't cleared at the end of the GC, then the next time around you'll mark too little and crash. Right now, the mark bits are not being reset at the end of GC, which is bad. Looking back at the patch history in bug 609104, I accidentally took this code out while fixing some review comments. However, the marking code for shapes doesn't check if they're already marked; it scans them unconditionally. So, at worst, we may mark a few extra shapes. But we won't crash. I'll post a patch in a bit. I don't think this should block, but we might want to take the patch anyway. I'll try to make it real safe.
This removes the assertion and puts back the code to clear the mark bits after a per-compartment GC. I also fixes some post-checkin nits that Brendan had in bug 609104 comment 67. It's too bad to lose the assertion. However, when we start GC allocating shapes, this will all change for the better. Until then, I think this fix is the simplest.
Attachment #514228 - Flags: approval2.0? → approval2.0+
Comment on attachment 514228 [details] [diff] [review] fix >+ /* >+ * Unmarked compartments containing marked objects don't get deleted, >+ * except LAST_CONTEXT GC is performed. >+ */ Need "when" after "except". > if ((!compartment->isMarked() && compartment->arenaListsAreEmpty()) > || gckind == GC_LAST_CONTEXT) Nit: || and && go at end of line. >+ /* >+ * Unmark all shapes. Even a per-compartment GC can mark shapes in other >+ * compartments, and we need to clear these bits. >+ */ Which other compartments? Sorry if I'm missing something obvious. A cross-compartment object-to-shape reference seems like a bug to me. /be
(In reply to comment #4) > Which other compartments? Sorry if I'm missing something obvious. A > cross-compartment object-to-shape reference seems like a bug to me. As far as I can tell, there are no cross-compartment object-to-shape references here (although I haven't been able to reproduce the crash myself, so I don't know for sure). When we do a per-compartment GC, we start tracing through the entire runtime; if we hit an object in the wrong compartment, we don't bother to trace through it. The problem here is that there aren't enough checks to ensure that we don't trace out of the compartment. You can see on the stack that we're going through a script and then a bindings object.
http://hg.mozilla.org/tracemonkey/rev/0c398bad4c99 Also, there's no need for this to be security-sensitive.
Why are we performing a per-compartment GC during shutdown?
(In reply to comment #7) > Why are we performing a per-compartment GC during shutdown? Hopefully this was just a GC that happened around when we were about to shut down. The stack seems to say that it wasn't the last GC.
(In reply to comment #8) > (In reply to comment #7) > > Why are we performing a per-compartment GC during shutdown? > > Hopefully this was just a GC that happened around when we were about to shut > down. The stack seems to say that it wasn't the last GC. Yeah you are right. That is a normal per-compartment GC. It seems the marked shape comes from js_TraceScript. We should avoid marking things there if the script is from another compartment and we perform per-compartment GC. Another bug for this?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.