Bug 614131 - Don't leave wrapped exception in the context on failure to enter compartment (r=gal)
authorLuke Wagner <lw@mozilla.com>
Fri, 07 Jan 2011 09:13:00 -0800
changeset 60245 63538367f9aa20dd5703b665e4fab0cf27f8d960
parent 60244 97401505439586c55ee8beb84f463a1248c4da97
child 60246 8a1715b0aeaeb2ba6553a5c2701249671032e4a8
push id17896
push usercleary@mozilla.com
push dateSat, 08 Jan 2011 08:51:06 +0000
treeherdermozilla-central@df3c1150dd7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal
bugs614131
milestone2.0b9pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 614131 - Don't leave wrapped exception in the context on failure to enter compartment (r=gal)
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jswrapper.cpp
js/src/jswrapper.h
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -2014,35 +2014,44 @@ JSContext::resetCompartment()
          */
         OBJ_TO_INNER_OBJECT(this, scopeobj);
         if (!scopeobj)
             goto error;
     }
 
     compartment = scopeobj->compartment();
 
-    /*
-     * If wrapException fails, it overrides this->exception and
-     * reports OOM. The upshot is that we silently turn the exception
-     * into an uncatchable OOM error. A bit surprising, but the
-     * caller is just going to return false either way.
-     */
     if (isExceptionPending())
-        (void) compartment->wrapException(this);
+        wrapPendingException();
     return;
 
 error:
 
     /*
      * If we try to use the context without a selected compartment,
      * we will crash.
      */
     compartment = NULL;
 }
 
+/*
+ * Since this function is only called in the context of a pending exception,
+ * the caller must subsequently take an error path. If wrapping fails, we leave
+ * the exception cleared, which, in the context of an error path, will be
+ * interpreted as an uncatchable exception.
+ */
+void
+JSContext::wrapPendingException()
+{
+    Value v = getPendingException();
+    clearPendingException();
+    if (compartment->wrap(this, &v))
+        setPendingException(v);
+}
+
 void
 JSContext::pushSegmentAndFrame(js::StackSegment *newseg, JSFrameRegs &newregs)
 {
     JS_ASSERT(regs != &newregs);
     if (hasActiveSegment())
         currentSegment->suspend(regs);
     newseg->setPreviousInContext(currentSegment);
     currentSegment = newseg;
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -1763,16 +1763,17 @@ struct JSContext
         return !!regs;
     }
 
   public:
     friend class js::StackSpace;
     friend bool js::Interpret(JSContext *, JSStackFrame *, uintN, JSInterpMode);
 
     void resetCompartment();
+    void wrapPendingException();
 
     /* 'regs' must only be changed by calling this function. */
     void setCurrentRegs(JSFrameRegs *regs) {
         JS_ASSERT_IF(regs, regs->fp);
         this->regs = regs;
         if (!regs)
             resetCompartment();
     }
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -348,31 +348,16 @@ JSCompartment::wrap(JSContext *cx, AutoI
     jsint length = props.length();
     for (size_t n = 0; n < size_t(length); ++n) {
         if (!wrapId(cx, &vector[n]))
             return false;
     }
     return true;
 }
 
-bool
-JSCompartment::wrapException(JSContext *cx)
-{
-    JS_ASSERT(cx->compartment == this);
-
-    if (cx->isExceptionPending()) {
-        Value v = cx->getPendingException();
-        cx->clearPendingException();
-        if (wrap(cx, &v))
-            cx->setPendingException(v);
-        return false;
-    }
-    return true;
-}
-
 #if defined JS_METHODJIT && defined JS_MONOIC
 /*
  * Check if the pool containing the code for jit should be destroyed, per the
  * heuristics in JSCompartment::sweep.
  */
 static inline bool
 ScriptPoolDestroyed(JSContext *cx, mjit::JITScript *jit,
                     uint32 releaseInterval, uint32 &counter)
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -322,17 +322,16 @@ struct JS_FRIEND_API(JSCompartment) {
 
     bool wrap(JSContext *cx, js::Value *vp);
     bool wrap(JSContext *cx, JSString **strp);
     bool wrap(JSContext *cx, JSObject **objp);
     bool wrapId(JSContext *cx, jsid *idp);
     bool wrap(JSContext *cx, js::PropertyOp *op);
     bool wrap(JSContext *cx, js::PropertyDescriptor *desc);
     bool wrap(JSContext *cx, js::AutoIdVector &props);
-    bool wrapException(JSContext *cx);
 
     void sweep(JSContext *cx, uint32 releaseInterval);
     void purge(JSContext *cx);
     void finishArenaLists();
     bool arenaListsAreEmpty();
 
   private:
     js::MathCache                *mathCache;
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -343,46 +343,40 @@ AutoCompartment::~AutoCompartment()
 
 bool
 AutoCompartment::enter()
 {
     JS_ASSERT(!entered);
     if (origin != destination) {
         LeaveTrace(context);
 
-#ifdef DEBUG
-        JSCompartment *oldCompartment = context->compartment;
-        context->resetCompartment();
-        wasSane = (context->compartment == oldCompartment);
-#endif
+        if (context->isExceptionPending())
+            return false;
 
         context->compartment = destination;
         JSObject *scopeChain = target->getGlobal();
         JS_ASSERT(scopeChain->isNative());
+
         frame.construct();
-        if (!context->stack().pushDummyFrame(context, *scopeChain, &frame.ref()) ||
-            !destination->wrapException(context)) {
-            frame.destroy();
+        if (!context->stack().pushDummyFrame(context, *scopeChain, &frame.ref())) {
             context->compartment = origin;
             return false;
         }
     }
     entered = true;
     return true;
 }
 
 void
 AutoCompartment::leave()
 {
     JS_ASSERT(entered);
     if (origin != destination) {
         frame.destroy();
         context->resetCompartment();
-        JS_ASSERT_IF(wasSane && context->hasfp(), context->compartment == origin);
-        context->compartment->wrapException(context);
     }
     entered = false;
 }
 
 /* Cross compartment wrappers. */
 
 JSCrossCompartmentWrapper::JSCrossCompartmentWrapper(uintN flags)
   : JSWrapper(CROSS_COMPARTMENT | flags)
@@ -637,18 +631,17 @@ JSCrossCompartmentWrapper::construct(JSC
     for (size_t n = 0; n < argc; ++n) {
         if (!call.destination->wrap(cx, &argv[n]))
             return false;
     }
     if (!JSWrapper::construct(cx, wrapper, argc, argv, rval))
         return false;
 
     call.leave();
-    return call.origin->wrap(cx, rval) &&
-           call.origin->wrapException(cx);
+    return call.origin->wrap(cx, rval);
 }
 
 bool
 JSCrossCompartmentWrapper::hasInstance(JSContext *cx, JSObject *wrapper, const Value *vp, bool *bp)
 {
     AutoCompartment call(cx, wrappedObject(wrapper));
     if (!call.enter())
         return false;
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -163,19 +163,16 @@ class AutoCompartment
     JSCompartment * const origin;
     JSObject * const target;
     JSCompartment * const destination;
   private:
     LazilyConstructed<DummyFrameGuard> frame;
     JSFrameRegs regs;
     AutoStringRooter input;
     bool entered;
-#ifdef DEBUG
-    bool wasSane;
-#endif
 
   public:
     AutoCompartment(JSContext *cx, JSObject *target);
     ~AutoCompartment();
 
     bool enter();
     void leave();