Assertion failure: (cx)->requestDepth || (cx)->thread == (cx)->runtime->gcThread, at mozilla/js/src/jsapi.cpp:5196

VERIFIED FIXED in mozilla1.9.2a1

Status

()

--
blocker
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

({assertion, verified1.9.0.12, verified1.9.1})

Trunk
mozilla1.9.2a1
x86_64
Linux
assertion, verified1.9.0.12, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
blocking1.9.0.12 +
wanted1.9.0.x +
blocking1.8.1.next -
wanted1.8.1.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: regression from 461861 [fixed on 1.9.0.12 by 461861])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
I get the assertion when running mochitest on a x86_64 linux debug build.
#7  0x00002aaaaadcd79c in JS_Assert (s=<value optimized out>, file=<value optimized out>, ln=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/js/src/jsutil.cpp:69
#8  0x00002aaaaad26be6 in JS_CallFunctionValue (cx=0x33af790, obj=0x4054740, fval=67461760, argc=0, argv=0x0, 
    rval=0x7fff8f095780) at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/js/src/jsapi.cpp:5196
#9  0x00002aaab0af0a61 in nsXBLProtoImplAnonymousMethod::Execute (this=0x17ee2b0, aBoundElement=0x2a4eb40)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/content/xbl/src/nsXBLProtoImplMethod.cpp:332
#10 0x00002aaab0afe9d5 in nsBindingManager::ProcessAttachedQueue (this=0x328c380, aSkipSize=0)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/content/xbl/src/nsBindingManager.cpp:1015
#11 0x00002aaab0afea6e in nsBindingManager::EndOutermostUpdate (this=0x328c380)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/content/xbl/src/nsBindingManager.cpp:1677
#12 0x00002aaab097abde in nsDocument::EndUpdate (this=0x3539960, aUpdateType=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/content/base/src/nsDocument.cpp:3709
#13 0x00002aaab0a7492f in nsHTMLDocument::EndUpdate (this=0x3ec9, aUpdateType=16073)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/content/html/document/src/nsHTMLDocument.cpp:3045

Comment 1

10 years ago
Can you attach the full stack trace?

Comment 2

10 years ago
Created attachment 377968 [details]
Full stack trace

It crashes consistently by loading the mochitest http://localhost:8888/tests/browser/components/feeds/test/test_bug408328.html

Updated

10 years ago
Blocks: 372581
Keywords: assertion

Updated

10 years ago
Assignee: general → igor

Updated

10 years ago
Hardware: x86 → x86_64

Comment 3

10 years ago
This bug is a regression from the bug 413774. Here is a relevant source of nsXBLProtoImplAnonymousMethod::Execute that triggers this bug, http://hg.mozilla.org/mozilla-central/file/998d6f6e95b7/content/xbl/src/nsXBLProtoImplMethod.cpp#l266

   309   JSAutoRequest ar(cx);
...
   320   // Use nsCxPusher to make sure we call ScriptEvaluated when we're done.
   321   nsCxPusher pusher;
   322   NS_ENSURE_STATE(pusher.Push(aBoundElement));
   ...
   331     ok = ::JS_CallFunctionValue(cx, thisObject, OBJECT_TO_JSVAL(method),

   332                                 0 /* argc */, nsnull /* argv */, &retval);


What happens here is that pusher.Push(aBoundElement) pushes a JSContext instance that is different from the one stored in the cx variable. Since the bug 413774 that lead to suspending the previous top of the stack, that is, the cx.

Now, this bug may not be a true regression from the bug 413774 as the latter could just expose some latent problem. For example, shouldn't the code use the pushed context by pusher.Push(aBoundElement), not the cx locals, when doing JS_CallFunctionValue?
Blocks: 413774
Uh... how do cx and the context the pusher pushes end up different?  Both look like they go through the owner document, then one uses the script global object, the other the script handling object.  Is that the issue?  After that they're identical again.
(Assignee)

Comment 5

10 years ago
bug 413774 did land long time ago. This assertion is something recent.

(In reply to comment #4)
> Uh... how do cx and the context the pusher pushes end up different? 
Yeah, I can't see how that could happen.

Comment 6

10 years ago
(In reply to comment #5)
> bug 413774 did land long time ago. This assertion is something recent.
> 
> (In reply to comment #4)
> > Uh... how do cx and the context the pusher pushes end up different? 
> Yeah, I can't see how that could happen.

Right, the context instance is the same, I was too quick to assume that. So the issue is unrelated to the bug 413774.

The real cause is the following code in  XPCJSContextStack::Push, http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcthreadcontext.cpp#134 :

141         XPCJSContextInfo & e = mStack[mStack.Length() - 2];
...
149                     nsIPrincipal* globalObjectPrincipal =
150                         GetPrincipalFromCx(cx);
...
153                         nsIPrincipal* subjectPrincipal = ssm->GetCxSubjectPrincipal(cx);
154                         PRBool equals = PR_FALSE;
155                         globalObjectPrincipal->Equals(subjectPrincipal, &equals);
156                         if(equals)
157                         {
158                             return NS_OK;
159                         }
...
162             }
163 
164             e.frame = JS_SaveFrameChain(e.cx);
165 
166             if(JS_GetContextThread(e.cx))
167                 e.requestDepth = JS_SuspendRequest(e.cx);

Here the check globalObjectPrincipal->Equals returns false leading to fallthrough to the code suspending the e.cx and the current cx. That points directly to the bug 461861.
Blocks: 461861
No longer blocks: 413774
(Assignee)

Comment 7

10 years ago
Ah, fun, I caused this.
Assignee: igor → Olli.Pettay
(Assignee)

Updated

10 years ago
Component: JavaScript Engine → XPConnect
QA Contact: general → xpconnect
(Assignee)

Comment 8

10 years ago
Created attachment 378421 [details] [diff] [review]
not tested
This needs to block 1.9.1.
Flags: blocking1.9.1?
Flags: blocking1.9.1? → blocking1.9.1+

Comment 10

10 years ago
Comment on attachment 378421 [details] [diff] [review]
not tested

>-            if(JS_GetContextThread(e.cx))
>+            if(JS_GetContextThread(e.cx) && e.cx != cx)

I suspect that JS_GetContextThread returning null here would be a bug as no code in the FF codebase calls JS_ClearContextThread. But even if it is possible it is better to avoid this function call that a compiler cannot optimize when e.cx == cx.
(Assignee)

Comment 11

10 years ago
Created attachment 378565 [details] [diff] [review]
patch
Attachment #378421 - Attachment is obsolete: true
Attachment #378565 - Flags: superreview?(mrbkap)
Attachment #378565 - Flags: review?(mrbkap)
Attachment #378565 - Flags: superreview?(mrbkap)
Attachment #378565 - Flags: superreview+
Attachment #378565 - Flags: review?(mrbkap)
Attachment #378565 - Flags: review+
Comment on attachment 378565 [details] [diff] [review]
patch

Sorry, should have spotted this when I reviewed the last patch.
(Assignee)

Comment 13

10 years ago
http://hg.mozilla.org/mozilla-central/rev/0f4de606acd7

Thanks Igor!
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Needed if we take bug 461861
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.12?
Whiteboard: regression from 461861
Olli, I think this works for trunk and 1.9.1 for you now?
Target Milestone: --- → mozilla1.9.2a1
(Assignee)

Comment 17

10 years ago
Yes, I pushed the patch to trunk and 1.9.1
Based on comment 17 everything should be fine now. I can't test on my own due to the lack of a 64bit Linux system. Marking as verified.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.next?
(Assignee)

Comment 19

10 years ago
This was 1.9.0.12fixed in Bug 461861
Keywords: fixed1.9.0.12
Whiteboard: regression from 461861 → regression from 461861 [fixed on 1.9.0.12 by 461861]
Marking verified for 1.9.0.12 based on Bug 461861.
Keywords: fixed1.9.0.12 → verified1.9.0.12
Flags: blocking1.8.1.next? → blocking1.8.1.next-
You need to log in before you can comment on or make changes to this bug.