910744 - Using AutoSafeJSContext in NotifyOffThreadScriptCompletedRunnable::Run causes some tests to time out

Status: RESOLVED DUPLICATE of bug 912719

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

See bug 908699 comment 10.  The mere presence of AutoSafeJSContext in this method seems to cause some tests, such as toolkit/mozapps/extensions/test/browser/browser_bug562797.js, to reliably time out on my 10.8 machine and on tinderbox.  This test mostly completes, then goes out of wonk and sits waiting for events without making progress.

Comment 1

[Bobby Holley (:bholley)]

Hm, that's bad. We should look into this, because we use AutoSafeJSContext all over the place, and the issue probably appears elsewhere as well.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note that this blocks fixing the "XUL scipts no longer report parse errors" bug (which we presumably need to get filed...)

Comment 3

[Bobby Holley (:bholley)]

Bill and I just looked at this. I'm 90% sure that the timeouts come from the fact that the patch in [1] does the following:

AutoSafeJSContext cx;
JSScript *script = JS::FinishOffThreadScript(cx, JS_GetRuntime(cx), mToken);
return mReceiver->OnScriptCompileComplete(script, script ? NS_OK : NS_ERROR_FAILURE);

This is happening in a runnable, so the AutoSafeJSContext will cause us to go from having null as the top of the cx stack (giving us system privileges) to having the SafeJSContext's NullPrincipal at the top (giving us very limited privileges).

You probably want:

{
  AutoSafeJSContext cx;
  JSScript *script = JS::FinishOffThreadScript(cx, JS_GetRuntime(cx), mToken);
}
return mReceiver->OnScriptCompileComplete(script, script ? NS_OK : NS_ERROR_FAILURE);


[1] https://bugzilla.mozilla.org/attachment.cgi?id=796297&action=diff

Comment 4

[Brian Hackett [Laid off!]]

OK, that fix worked.  The above patch was included in bug 912719.