Bug 630072. Fix issue with wrappers holding objects from old scopes alive. r=mrbkap@gmail.com, a=blocker
☠☠ backed out by 4c16f0da0671 ☠ ☠
authorAndreas Gal <gal@uci.edu>
Fri, 11 Feb 2011 16:36:48 -0800
changeset 62444 3fb25cc2c04034010077e2381bbf4e816853acad
parent 62443 b5b177ec4d5de4eec95e7acdd42d3ce4d29332ad
child 62445 44d893bbb66d805e157f8925f540089392539db5
child 62446 4c16f0da0671f329b676212edddd044b0e3ebf80
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
reviewersmrbkap, blocker
bugs630072
milestone2.0b12pre
Bug 630072. Fix issue with wrappers holding objects from old scopes alive. r=mrbkap@gmail.com, a=blocker
dom/base/nsJSEnvironment.cpp
ipc/testshell/XPCShellEnvironment.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jscompartment.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsscope.h
js/src/shell/js.cpp
js/src/xpconnect/loader/mozJSComponentLoader.h
js/src/xpconnect/shell/xpcshell.cpp
js/src/xpconnect/src/dom_quickstubs.qsconf
js/src/xpconnect/tests/TestXPC.cpp
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -3173,30 +3173,22 @@ nsJSContext::ClearScope(void *aGlobalObj
     // nothing else does.
     jsval window;
     if (!JS_GetProperty(mContext, obj, "window", &window)) {
       window = JSVAL_VOID;
 
       JS_ClearPendingException(mContext);
     }
 
-    // Hack fix for bug 611653. Originally, this always called JS_ClearScope,
-    // which was required to avoid leaks. But for native objects, the JS
-    // 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 (!JS_ClearScope(mContext, obj))
+      JS_ClearPendingException(mContext);
 
     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) {
       if (!JS_DefineProperty(mContext, obj, "window", window,
                              JS_PropertyStub, JS_StrictPropertyStub,
                              JSPROP_ENUMERATE | JSPROP_READONLY |
                              JSPROP_PERMANENT)) {
         JS_ClearPendingException(mContext);
@@ -3224,17 +3216,18 @@ nsJSContext::ClearScope(void *aGlobalObj
     // chain when we're clearing an outer window whose current inner we
     // still want.
     if (aClearFromProtoChain) {
       nsWindowSH::InvalidateGlobalScopePolluter(mContext, obj);
 
       // 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);
     }
   }
 
   if (stack) {
     stack->Pop(nsnull);
   }
 }
 
--- a/ipc/testshell/XPCShellEnvironment.cpp
+++ b/ipc/testshell/XPCShellEnvironment.cpp
@@ -519,17 +519,18 @@ DumpHeap(JSContext *cx,
 
 static JSBool
 Clear(JSContext *cx,
       uintN argc,
       jsval *vp)
 {
     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_SET_RVAL(cx, vp, JSVAL_VOID);
     return JS_TRUE;
 }
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3917,39 +3917,61 @@ JS_DeleteElement(JSContext *cx, JSObject
 
 JS_PUBLIC_API(JSBool)
 JS_DeleteProperty(JSContext *cx, JSObject *obj, const char *name)
 {
     jsval junk;
     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);
 
     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_Enumerate(JSContext *cx, JSObject *obj)
 {
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2464,17 +2464,17 @@ extern JS_PUBLIC_API(JSBool)
 JS_SetElement(JSContext *cx, JSObject *obj, jsint index, jsval *vp);
 
 extern JS_PUBLIC_API(JSBool)
 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_Enumerate(JSContext *cx, JSObject *obj);
 
 /*
  * Create an object to iterate over enumerable properties of obj, in arbitrary
  * property definition order.  NB: This differs from longstanding for..in loop
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -265,18 +265,24 @@ JSCompartment::wrap(JSContext *cx, Value
             JS_ASSERT(outer && outer == obj);
         }
 #endif
     }
 
     /* If we already have a wrapper for this value, use it. */
     if (WrapperMap::Ptr p = crossCompartmentWrappers.lookup(*vp)) {
         *vp = p->value;
-        if (vp->isObject())
-            vp->toObject().setParent(global);
+        if (vp->isObject()) {
+            JSObject *obj = &vp->toObject();
+            do {
+                JS_ASSERT(obj->isWrapper());
+                obj->setParent(global);
+                obj = obj->getProto();
+            } while (obj);
+        }
         return true;
     }
 
     if (vp->isString()) {
         Value orig = *vp;
         JSString *str = vp->toString();
         const jschar *chars = str->getChars(cx);
         if (!chars)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -4445,50 +4445,16 @@ JSObject::freeSlot(JSContext *cx, uint32
             last = slot;
             return true;
         }
     }
     vref.setUndefined();
     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"
 
 /*
  * Convert string indexes that convert to int jsvals as ints to save memory.
  * Care must be taken to use this macro every time a property name is used, or
  * else double-sets, incorrect property cache misses, or other mistakes could
  * occur.
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1763,25 +1763,16 @@ js_SetPropertyHelper(JSContext *cx, JSOb
 /*
  * Change attributes for the given native property. The caller must ensure
  * that obj is locked and this function always unlocks obj on return.
  */
 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 {
 
 /*
  * If obj has a data property methodid which is a function object for the given
  * native, return that function object. Otherwise, return NULL.
  */
 extern JSObject *
 HasNativeMethod(JSObject *obj, jsid methodid, Native native);
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -292,17 +292,16 @@ CastAsPropertyOp(js::Class *clasp)
 
 struct Shape : public JSObjectMap
 {
     friend struct ::JSObject;
     friend struct ::JSFunction;
     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
      * search() call.  Once numLinearSearches reaches MAX_LINEAR_SEARCHES
      * (which is a small integer), the table is created on the next search()
      * call, and the table pointer will be easily distinguishable from a small
      * integer.  The table can also be created when hashifying for dictionary
      * mode.
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2728,20 +2728,21 @@ BuildDate(JSContext *cx, uintN argc, jsv
     return JS_TRUE;
 }
 
 static JSBool
 Clear(JSContext *cx, uintN argc, jsval *vp)
 {
     JSObject *obj;
     if (argc != 0 && !JS_ValueToObject(cx, JS_ARGV(cx, vp)[0], &obj))
-        return JS_FALSE;
-    JS_ClearScope(cx, obj);
+        return false;
+    if (!JS_ClearScope(cx, obj))
+        return false;
     JS_SET_RVAL(cx, vp, JSVAL_VOID);
-    return JS_TRUE;
+    return true;
 }
 
 static JSBool
 Intern(JSContext *cx, uintN argc, jsval *vp)
 {
     JSString *str = JS_ValueToString(cx, argc == 0 ? JSVAL_VOID : vp[2]);
     if (!str)
         return false;
@@ -4679,17 +4680,18 @@ split_setup(JSContext *cx, JSBool evalcx
         arguments = JS_NewArrayObject(cx, 0, NULL);
         if (!arguments ||
             !JS_DefineProperty(cx, inner, "arguments", OBJECT_TO_JSVAL(arguments),
                                NULL, NULL, 0)) {
             return NULL;
         }
     }
 
-    JS_ClearScope(cx, outer);
+    if (!JS_ClearScope(cx, outer))
+        return NULL;
 
 #ifndef LAZY_STANDARD_CLASSES
     if (!JS_InitStandardClasses(cx, inner))
         return NULL;
 #endif
 
     return inner;
 }
--- a/js/src/xpconnect/loader/mozJSComponentLoader.h
+++ b/js/src/xpconnect/loader/mozJSComponentLoader.h
@@ -173,17 +173,18 @@ class mozJSComponentLoader : public mozi
             getfactoryobj = NULL;
 
             if (global) {
                 JSAutoRequest ar(sSelf->mContext);
 
                 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);
             }
 
             if (location)
                 NS_Free(location);
 
             global = NULL;
             location = NULL;
--- a/js/src/xpconnect/shell/xpcshell.cpp
+++ b/js/src/xpconnect/shell/xpcshell.cpp
@@ -674,17 +674,18 @@ DumpHeap(JSContext *cx, uintN argc, jsva
 }
 
 #endif /* DEBUG */
 
 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;
     }
     JS_SET_RVAL(cx, vp, JSVAL_VOID);
     return JS_TRUE;
 }
 
@@ -2014,17 +2015,18 @@ main(int argc, char **argv)
 
 #ifdef TEST_CALL_ON_WRAPPED_JS_AFTER_SHUTDOWN
             // test of late call and release (see below)
             nsCOMPtr<nsIJSContextStack> bogus;
             xpc->WrapJS(cx, glob, NS_GET_IID(nsIJSContextStack),
                         (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);
             NS_ASSERTION(oldcx == cx, "JS thread context push/pop mismatch");
             cxstack = nsnull;
             JS_GC(cx);
         } //this scopes the JSAutoCrossCompartmentCall
         JS_DestroyContext(cx);
--- a/js/src/xpconnect/src/dom_quickstubs.qsconf
+++ b/js/src/xpconnect/src/dom_quickstubs.qsconf
@@ -553,18 +553,19 @@ nsIDOMHTMLDocument_Write_customMethodCal
       str.Append(next_arg);
     }
 
     rv = self->%s(arg0);
 """
 
 nsIDOMStorage_Clear_customMethodCallCode = """
     rv = self->Clear();
-    if (NS_SUCCEEDED(rv))
-        JS_ClearScope(cx, obj);
+    if (NS_SUCCEEDED(rv)) {
+        rv = JS_ClearScope(cx, obj) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
+    }
 """
 
 CUSTOM_QS = {
     'skipgen': True,
     'traceable': False
 }
 
 CUSTOM_QS_TN = {
--- a/js/src/xpconnect/tests/TestXPC.cpp
+++ b/js/src/xpconnect/tests/TestXPC.cpp
@@ -851,17 +851,18 @@ int main()
 
         /**********************************************/
 
         if(NS_FAILED(cxstack->Pop(nsnull)))
             DIE("FAILED to pop the current jscontext from the nsThreadJSContextStack service!\n");
 
         {
             JSAutoRequest ar(jscontext);
-            JS_ClearScope(jscontext, glob);
+            if (!JS_ClearScope(jscontext, glob))
+                DIE("FAILED to clear scope");
             JS_GC(jscontext);
             JS_GC(jscontext);
         }
         JS_DestroyContext(jscontext);
         xpc->DebugDump(4);
 
         cxstack = nsnull;   // release service held by nsCOMPtr
         xpc     = nsnull;   // release service held by nsCOMPtr