585059 - lift restrictions on order of XPCJSContextStack::Push and AutoRequest calls

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Igor Bukanov]

+++ This bug was initially created as a clone of Bug #552266 +++

When working on bug 584673 I notices the following code pattern in xpc_EvalInSandbox, http://hg.mozilla.org/tracemonkey/file/f6c1d5ab87f9/js/src/xpconnect/src/xpccomponents.cpp#l3562 :

{
    JSAutoRequest req(cx);
    ...
}
....
{
   JSAutoRequest req(sandbox_cx);

   if (exception) {
       JSAutoRequest req(cx); // (*)

   }
}

With bug 552266 changes the above would be wrong if on entrance to xpc_EvalInSandbox the cx were in request that outo-suspended some other JSContext instance cx3. In this case the line marked with (*) would assert as it would imply that cx must suspend a request for sandbox_cx in addition for already suspended cx3.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch fixes the request order. It also changes XPCWrapper::RewrapObject called to rewrap the exception object, to use sandcx->GetJSContext(), not the cx, to match XPCWrapper::RewrapObject done for normal call.

Comment 2

[Igor Bukanov]

Comment on attachment 463938 [details] [diff] [review]
v1

jsdStackFrame::Eval has the same no-longer-valid pattern of starting a request before doing Push. I will try to fix all such problematic places in one patch.

Comment 3

[Igor Bukanov]

After trying to fix all the places with violated order of XPCJSContextStack::Push and AutoRequest calls and introducing a not-so-easy-to-find bug in the patch I decided to take another approach. The idea is to lift the restriction on the order  of the methods so Push can be called both before and after JS_BeginRequest and similarly with Pop and JS_EndRequest. This should fully restore the compatibility with the current code.

For that it is enough to provide JS_GetRequestContext API that Push/Pop can use to query the request status of the current thread and to insist in JS_RestoreFrameChain and JS_SaveFrameChain only that the current thread is in a request, not the passed JSContext instance.

Comment 4

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.