380512 - "ASSERTION: JSContext still in threadjscontextstack" and crash removing a <frame> with an http auth prompt for a <script> load

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Tested in Mac trunk debug.

1. Load the testcase.
2. Wait 3 seconds for the blue to disappear.

###!!! ASSERTION: JSContext still in threadjscontextstack!: '!tls->GetJSContextStack() || !tls->GetJSContextStack()-> DEBUG_StackHasJSContext(aJSContext)', mozilla/js/src/xpconnect/src/nsXPConnect.cpp, line 1852

3. Press *Enter* to *accept* the http auth dialog.

(sometimes)
Assertion failure: !cx->fp, at /Users/jruderman/trunk/mozilla/js/src/jsapi.c:4933

There is a related EXC_BAD_INSTRUCTION crash requiring no user interaction, but I don't know how to reproduce it.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Stack

Here's the stack for that assertion.  I also had a breakpoint in
nsWindowWatcher::OpenWindowJSInternal printing out the local
variable 'cx' (blue).  We delete that JSContext while it's
still on the stack of the local event loop of the dialog.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpfe/appshell/src/nsXULWindow.cpp&rev=1.171&root=/cvsroot&mark=383,398,401-402#383

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

FWIW, a kung-fu death grip on the script context fixes the crash...

Comment 3

[Mats Palmgren (inactive)]

Bug 307247 and bug 234013 looks like it could be the same problem.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I suppose that would work; we could also change the Push() callers to pass in the nsIScriptContext unless it would be more work...

We should probably audit other places that push non-null JSContexts.

Comment 5

[Johnny Stenback (:jst)]

Blocking on getting the wip patch reviewed and checked in at least, even if there's more that could be done here.

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 264681 [details] [diff] [review]
Patch rev. 1

Clearing review request. I'd like to investigate this further...

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 264681 [details] [diff] [review]
Patch rev. 1

Sorry for the delay.  I investigated this last week and this path
is the only problem I could find.

Comment 8

[Mats Palmgren (inactive)]

mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp 	1.131 

-> FIXED

Comment 9

[Mats Palmgren (inactive)]

The patch would also apply to branches if we like it there too.
The testcase does not crash branch builds, presumably because someone
else is holding a strong ref, but the patch is low risk so maybe
we should take it just in case...

Comment 10

[Jesse Ruderman]

Now, when I follow the steps in comment 0, I get:

###!!! ASSERTION: null request in stream complete handler: 'request', file /Users/jruderman/trunk/mozilla/content/base/src/nsScriptLoader.cpp, line 791

Is that bad?

Comment 11

[Mats Palmgren (inactive)]

Filed bug 388523 for the assertion.