Closed
Bug 585059
Opened 14 years ago
Closed 6 years ago
lift restrictions on order of XPCJSContextStack::Push and AutoRequest calls
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
INACTIVE
People
(Reporter: igor, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
+++ 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.
Reporter | ||
Comment 1•14 years ago
|
||
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.
Attachment #463938 -
Flags: review?(mrbkap)
Reporter | ||
Comment 2•14 years ago
|
||
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.
Attachment #463938 -
Flags: review?(mrbkap)
Reporter | ||
Comment 3•14 years ago
|
||
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.
Summary: xpc_EvalInSandbox can violate one request cx per thread assumption → lift restrictions on order of XPCJSContextStack::Push and AutoRequest calls
Reporter | ||
Updated•13 years ago
|
Assignee: igor → nobody
Comment 4•6 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
You need to log in
before you can comment on or make changes to this bug.
Description
•