Bug 630072. Fix issue with wrappers holding objects from old scopes alive. r=mrbkap@gmail.com, a=blocker

dom/base/nsJSEnvironment.cpp

@@ -3178,20 +3178,12 @@ nsJSContext::ClearScope(void *aGlobalObj, PRBool aClearFromProtoChain)
       JS_ClearPendingException(mContext);
     }
 
-    // Hack fix for bug 611653. Originally, this always called JS_ClearScope,
+    if (!JS_ClearScope(mContext, obj))
-    // which was required to avoid leaks. But for native objects, the JS
+      JS_ClearPendingException(mContext);
-    // engine has an optimization that requires that permanent properties of
-    // the global object are never deleted. So instead, we call a new special
-    // API that clears the values of the global, thus avoiding leaks without
-    // deleting any properties.
-    if (obj->isNative()) {
-      js_UnbrandAndClearSlots(mContext, obj);
-    } else {
-      JS_ClearScope(mContext, obj);
-    }
 
     if (xpc::WrapperFactory::IsXrayWrapper(obj)) {
-      JS_ClearScope(mContext, &obj->getProxyExtra().toObject());
+      if (!JS_ClearScope(mContext, &obj->getProxyExtra().toObject()))
+        JS_ClearPendingException(mContext);
     }
 
     if (window != JSVAL_VOID) {
@@ -3229,7 +3221,8 @@ nsJSContext::ClearScope(void *aGlobalObj, PRBool aClearFromProtoChain)
       // Clear up obj's prototype chain, but not Object.prototype.
       for (JSObject *o = ::JS_GetPrototype(mContext, obj), *next;
            o && (next = ::JS_GetPrototype(mContext, o)); o = next)
-        ::JS_ClearScope(mContext, o);
+        if (!::JS_ClearScope(mContext, o))
+          JS_ClearPendingException(mContext);
     }
   }
 

ipc/testshell/XPCShellEnvironment.cpp

@@ -524,7 +524,8 @@ Clear(JSContext *cx,
 {
     jsval *argv = JS_ARGV(cx, vp);
     if (argc > 0 && !JSVAL_IS_PRIMITIVE(argv[0])) {
-        JS_ClearScope(cx, JSVAL_TO_OBJECT(argv[0]));
+        if (!JS_ClearScope(cx, JSVAL_TO_OBJECT(argv[0])))
+	    return JS_FALSE;
     } else {
         JS_ReportError(cx, "'clear' requires an object");
         return JS_FALSE;

js/src/jsapi.cpp

@@ -3922,12 +3922,14 @@ JS_DeleteProperty(JSContext *cx, JSObject *obj, const char *name)
     return JS_DeleteProperty2(cx, obj, name, &junk);
 }
 
-JS_PUBLIC_API(void)
+JS_PUBLIC_API(JSBool)
 JS_ClearScope(JSContext *cx, JSObject *obj)
 {
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
 
+    uint32 span = obj->slotSpan();
+
     JSFinalizeOp clearOp = obj->getOps()->clear;
     if (clearOp)
         clearOp(cx, obj);
@@ -3935,16 +3937,36 @@ JS_ClearScope(JSContext *cx, JSObject *obj)
     if (obj->isNative())
         js_ClearNative(cx, obj);
 
+    js_InitRandom(cx);
+
     /* Clear cached class objects on the global object. */
     if (obj->isGlobal()) {
+        if (!obj->unbrand(cx))
+            return false;
+
         for (int key = JSProto_Null; key < JSProto_LIMIT * 3; key++)
             JS_SetReservedSlot(cx, obj, key, JSVAL_VOID);
 
+        /* Clear regexp statics. */
+        RegExpStatics::extractFrom(obj)->clear();
+
         /* Clear the CSP eval-is-allowed cache. */
         JS_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_EVAL_ALLOWED, JSVAL_VOID);
+
+        /*
+         * Compile-and-go scripts might rely on these slots to be present,
+         * so set a bunch of dummy properties to make sure compiled code
+         * doesn't reach into empty space.
+         */
+        uint32 n = 0;
+        while (obj->slotSpan() < span) {
+            if (!JS_DefinePropertyById(cx, obj, INT_TO_JSID(n), JSVAL_VOID, NULL, NULL, 0))
+                return false;
+            ++n;
+        }
     }
 
-    js_InitRandom(cx);
+    return true;
 }
 
 JS_PUBLIC_API(JSIdArray *)

js/src/jsapi.h

@@ -2469,7 +2469,7 @@ JS_DeleteElement(JSContext *cx, JSObject *obj, jsint index);
 extern JS_PUBLIC_API(JSBool)
 JS_DeleteElement2(JSContext *cx, JSObject *obj, jsint index, jsval *rval);
 
-extern JS_PUBLIC_API(void)
+extern JS_PUBLIC_API(JSBool)
 JS_ClearScope(JSContext *cx, JSObject *obj);
 
 extern JS_PUBLIC_API(JSIdArray *)

js/src/jscompartment.cpp

@@ -270,8 +270,14 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
     /* If we already have a wrapper for this value, use it. */
     if (WrapperMap::Ptr p = crossCompartmentWrappers.lookup(*vp)) {
         *vp = p->value;
-        if (vp->isObject())
+        if (vp->isObject()) {
-            vp->toObject().setParent(global);
+            JSObject *obj = &vp->toObject();
+            do {
+                JS_ASSERT(obj->isWrapper());
+                obj->setParent(global);
+                obj = obj->getProto();
+            } while (obj);
+        }
         return true;
     }
 

js/src/jsobj.cpp

@@ -4450,40 +4450,6 @@ JSObject::freeSlot(JSContext *cx, uint32 slot)
     return false;
 }
 
-JS_FRIEND_API(void)
-js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj)
-{
-    JS_ASSERT(obj->isNative());
-    JS_ASSERT(obj->isGlobal());
-
-    /* This can return false but that doesn't mean it failed. */
-    obj->unbrand(cx);
-
-    /*
-     * Clear the prototype cache. We must not clear the other global
-     * reserved slots, as other code will crash if they are arbitrarily
-     * reset (e.g., regexp statics).
-     */
-    for (int key = JSProto_Null; key < JSRESERVED_GLOBAL_THIS; key++)
-        JS_SetReservedSlot(cx, obj, key, JSVAL_VOID);
-
-    /*
-     * Clear the non-reserved slots.
-     */
-    ClearValueRange(obj->slots + JSCLASS_RESERVED_SLOTS(obj->clasp),
-                    obj->capacity - JSCLASS_RESERVED_SLOTS(obj->clasp),
-                    obj->clasp == &js_ArrayClass);
-
-    /*
-     * We just overwrote all slots to undefined, so the freelist has
-     * been trashed. We need to clear the head pointer or else we will
-     * crash later. This leaks slots but the object is all but dead
-     * anyway.
-     */
-    if (obj->hasPropertyTable())
-        obj->lastProperty()->getTable()->freelist = SHAPE_INVALID_SLOT;
-}
-
 /* JSBOXEDWORD_INT_MAX as a string */
 #define JSBOXEDWORD_INT_MAX_STRING "1073741823"
 

js/src/jsobj.h

@@ -1768,15 +1768,6 @@ extern JSBool
 js_SetNativeAttributes(JSContext *cx, JSObject *obj, js::Shape *shape,
                        uintN attrs);
 
-/*
- * Hack fix for bug 611653: Do not use for any other purpose.
- *
- * Unbrand and set all slot values to undefined (except reserved slots that
- * are not used for cached prototypes).
- */
-JS_FRIEND_API(void)
-js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj);
-
 namespace js {
 
 /*

js/src/jsscope.h

@@ -297,7 +297,6 @@ struct Shape : public JSObjectMap
     friend class js::PropertyTree;
     friend class js::Bindings;
     friend bool IsShapeAboutToBeFinalized(JSContext *cx, const js::Shape *shape);
-    friend JS_FRIEND_API(void) ::js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj);
 
     /* 
      * numLinearSearches starts at zero and is incremented initially on each

js/src/shell/js.cpp

@@ -2733,10 +2733,11 @@ Clear(JSContext *cx, uintN argc, jsval *vp)
 {
     JSObject *obj;
     if (argc != 0 && !JS_ValueToObject(cx, JS_ARGV(cx, vp)[0], &obj))
-        return JS_FALSE;
+        return false;
-    JS_ClearScope(cx, obj);
+    if (!JS_ClearScope(cx, obj))
+        return false;
     JS_SET_RVAL(cx, vp, JSVAL_VOID);
-    return JS_TRUE;
+    return true;
 }
 
 static JSBool
@@ -4684,7 +4685,8 @@ split_setup(JSContext *cx, JSBool evalcx)
         }
     }
 
-    JS_ClearScope(cx, outer);
+    if (!JS_ClearScope(cx, outer))
+        return NULL;
 
 #ifndef LAZY_STANDARD_CLASSES
     if (!JS_InitStandardClasses(cx, inner))

js/src/xpconnect/loader/mozJSComponentLoader.h

@@ -178,7 +178,8 @@ class mozJSComponentLoader : public mozilla::ModuleLoader,
                 JSAutoEnterCompartment ac;
                 ac.enterAndIgnoreErrors(sSelf->mContext, global);
 
-                JS_ClearScope(sSelf->mContext, global);
+                if (!JS_ClearScope(sSelf->mContext, global))
+                    JS_ClearPendingException(sSelf->mContext);
                 JS_RemoveObjectRoot(sSelf->mContext, &global);
             }
 

js/src/xpconnect/shell/xpcshell.cpp

@@ -679,7 +679,8 @@ static JSBool
 Clear(JSContext *cx, uintN argc, jsval *vp)
 {
     if (argc > 0 && !JSVAL_IS_PRIMITIVE(JS_ARGV(cx, vp)[0])) {
-        JS_ClearScope(cx, JSVAL_TO_OBJECT(JS_ARGV(cx, vp)[0]));
+        if (!JS_ClearScope(cx, JSVAL_TO_OBJECT(JS_ARGV(cx, vp)[0])))
+            return JS_FALSE;
     } else {
         JS_ReportError(cx, "'clear' requires an object");
         return JS_FALSE;
@@ -2019,7 +2020,8 @@ main(int argc, char **argv)
                         (void**) getter_AddRefs(bogus));
 #endif
             JSPRINCIPALS_DROP(cx, gJSPrincipals);
-            JS_ClearScope(cx, glob);
+            if (!JS_ClearScope(cx, glob))
+                NS_ERROR("clearing scope failed");
             JS_GC(cx);
             JSContext *oldcx;
             cxstack->Pop(&oldcx);

js/src/xpconnect/src/dom_quickstubs.qsconf

@@ -558,8 +558,9 @@ nsIDOMHTMLDocument_Write_customMethodCallCode = """
 
 nsIDOMStorage_Clear_customMethodCallCode = """
     rv = self->Clear();
-    if (NS_SUCCEEDED(rv))
+    if (NS_SUCCEEDED(rv)) {
-        JS_ClearScope(cx, obj);
+        rv = JS_ClearScope(cx, obj) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
+    }
 """
 
 CUSTOM_QS = {

js/src/xpconnect/tests/TestXPC.cpp

@@ -856,7 +856,8 @@ int main()
 
         {
             JSAutoRequest ar(jscontext);
-            JS_ClearScope(jscontext, glob);
+            if (!JS_ClearScope(jscontext, glob))
+                DIE("FAILED to clear scope");
             JS_GC(jscontext);
             JS_GC(jscontext);
         }