Bug 765432 - Make JS_IsAboutToBeFinalized indirect; r=bholley,billm
authorTerrence Cole <terrence@mozilla.com>
Thu, 28 Mar 2013 13:27:29 -0700
changeset 126718 88ec6ee2d57a2c62051e13aba2c83bc071ccc47c
parent 126717 36eca1fb1c5c19f78a1383991c99afdba42b9dd9
child 126719 a61aa08ab4ac92a5f909a87f0aa717a05eb36df2
push id25608
push usertcole@mozilla.com
push dateFri, 29 Mar 2013 17:26:54 +0000
treeherdermozilla-inbound@88ec6ee2d57a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, billm
bugs765432
milestone22.0a1
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 765432 - Make JS_IsAboutToBeFinalized indirect; r=bholley,billm
js/public/GCAPI.h
js/src/jsapi-tests/README
js/src/jsapi-tests/testIntern.cpp
js/src/jsapi-tests/testIsAboutToBeFinalized.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jstypedarray.cpp
js/xpconnect/src/XPCMaps.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -199,16 +199,20 @@ class ObjectPtr
     void init(JSObject *obj) { value = obj; }
 
     JSObject *get() const { return value; }
 
     void writeBarrierPre(JSRuntime *rt) {
         IncrementalObjectBarrier(value);
     }
 
+    bool isAboutToBeFinalized() {
+        return JS_IsAboutToBeFinalized(&value);
+    }
+
     ObjectPtr &operator=(JSObject *obj) {
         IncrementalObjectBarrier(value);
         value = obj;
         return *this;
     }
 
     JSObject &operator*() const { return *value; }
     JSObject *operator->() const { return value; }
--- a/js/src/jsapi-tests/README
+++ b/js/src/jsapi-tests/README
@@ -115,33 +115,31 @@ tests.h:
 
         True if v1 and v2 are the same value according to the ES5 SameValue()
         function, to wit:
 
         SameValue(NaN, NaN) is true.
         SameValue(-0, 0) is false.
         Otherwise SameValue(a, b) iff a === b.
 
-And see class jsvalRoot, also defined in tests.h.
-
 
 --- Custom test setup
 
 Before executing each test, the test framework calls the tests' init() member
 function, which populates the rt, cx, and global member variables.
 
 A test can customize the test setup process by overloading virtual member
 functions, like this:
 
     JSClass globalClassWithResolve = { ... };
 
     BEGIN_TEST(testGlobalResolveHook)
         {
-            jsvalRoot v;
-            EVAL("v", v.addr());
+            RootedValue v;
+            EVAL("v", v.address());
             CHECK_SAME(v, JSVAL_VOID);
             return true;
         }
 
         // Other class members can go here.
 
         // This one overloads a base-class method.
         virtual JSClass *getGlobalJSClass() {
--- a/js/src/jsapi-tests/testIntern.cpp
+++ b/js/src/jsapi-tests/testIntern.cpp
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "tests.h"
 #include "jsatom.h"
 
+#include "gc/Marking.h"
 #include "vm/String.h"
 
 using mozilla::ArrayLength;
 
 BEGIN_TEST(testAtomizedIsNotInterned)
 {
     /* Try to pick a string that won't be interned by other tests in this runtime. */
     static const char someChars[] = "blah blah blah? blah blah blah";
@@ -26,17 +27,17 @@ struct StringWrapper
     JSString *str;
     bool     strOk;
 } sw;
 
 void
 FinalizeCallback(JSFreeOp *fop, JSFinalizeStatus status, JSBool isCompartmentGC)
 {
     if (status == JSFINALIZE_GROUP_START)
-        sw.strOk = !JS_IsAboutToBeFinalized(sw.str);
+        sw.strOk = js::gc::IsStringMarked(&sw.str);
 }
 
 BEGIN_TEST(testInternAcrossGC)
 {
     sw.str = JS_InternString(cx, "wrapped chars that another test shouldn't be using");
     sw.strOk = false;
     CHECK(sw.str);
     JS_SetFinalizeCallback(rt, FinalizeCallback);
deleted file mode 100644
--- a/js/src/jsapi-tests/testIsAboutToBeFinalized.cpp
+++ /dev/null
@@ -1,140 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: set ts=8 sw=4 et tw=99:
- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-
-#include "tests.h"
-#include "jsstr.h"
-
-static JSGCCallback oldGCCallback;
-
-static void **checkPointers;
-static unsigned checkPointersLength;
-static size_t checkPointersStaticStrings;
-
-static JSBool
-TestAboutToBeFinalizedCallback(JSContext *cx, JSGCStatus status)
-{
-    if (status == JSGC_MARK_END && checkPointers) {
-        for (unsigned i = 0; i != checkPointersLength; ++i) {
-            void *p = checkPointers[i];
-            JS_ASSERT(p);
-            if (JS_IsAboutToBeFinalized(p))
-                checkPointers[i] = NULL;
-        }
-    }
-
-    return !oldGCCallback || oldGCCallback(cx, status);
-}
-
-/*
- * NativeFrameCleaner write to sink so a compiler nwould not be able to optimze out
- * the buffer memset there.
- */
-volatile void *ptrSink;
-
-static JS_NEVER_INLINE void
-NativeFrameCleaner()
-{
-    char buffer[1 << 16];
-    memset(buffer, 0, sizeof buffer);
-    ptrSink = buffer;
-}
-
-BEGIN_TEST(testIsAboutToBeFinalized_bug528645)
-{
-    /*
-     * Due to the conservative GC we use separated never-inline function to
-     * test rooted elements.
-     */
-    CHECK(createAndTestRooted());
-    NativeFrameCleaner();
-
-    JS_GC(cx);
-
-    /* Everything is unrooted except unit strings. */
-    for (unsigned i = 0; i != checkPointersLength; ++i) {
-        void *p = checkPointers[i];
-        if (p) {
-            CHECK(JSString::isStatic(p));
-            CHECK(checkPointersStaticStrings != 0);
-            --checkPointersStaticStrings;
-        }
-    }
-    CHECK_EQUAL(checkPointersStaticStrings, 0);
-
-    free(checkPointers);
-    checkPointers = NULL;
-    JS_SetGCCallback(cx, oldGCCallback);
-
-    return true;
-}
-
-JS_NEVER_INLINE bool createAndTestRooted();
-
-END_TEST(testIsAboutToBeFinalized_bug528645)
-
-JS_NEVER_INLINE bool
-cls_testIsAboutToBeFinalized_bug528645::createAndTestRooted()
-{
-    jsvalRoot root(cx);
-
-    /*
-     * Check various types of GC things against JS_IsAboutToBeFinalized.
-     * Make sure to include unit and numeric strings to the set.
-     */
-    EVAL("var x = 1.1; "
-         "[''+x, 'a', '123456789', 'something'.substring(1), "
-         "{}, [], new Function('return 10;')];",
-         root.addr());
-
-    JSObject *array = JSVAL_TO_OBJECT(root.value());
-    JS_ASSERT(JS_IsArrayObject(cx, array));
-
-    JSBool ok = JS_GetArrayLength(cx, array, &checkPointersLength);
-    CHECK(ok);
-
-    checkPointers = (void **) malloc(sizeof(void *) * checkPointersLength);
-    CHECK(checkPointers);
-
-    checkPointersStaticStrings = 0;
-    for (unsigned i = 0; i != checkPointersLength; ++i) {
-        jsval v;
-        ok = JS_GetElement(cx, array, i, &v);
-        CHECK(ok);
-        JS_ASSERT(JSVAL_IS_GCTHING(v));
-        JS_ASSERT(!JSVAL_IS_NULL(v));
-        checkPointers[i] = JSVAL_TO_GCTHING(v);
-        if (JSString::isStatic(checkPointers[i]))
-            ++checkPointersStaticStrings;
-    }
-
-    oldGCCallback = JS_SetGCCallback(cx, TestAboutToBeFinalizedCallback);
-    JS_GC(cx);
-
-    /*
-     * All GC things are rooted via the root holding the array containing them
-     * and TestAboutToBeFinalizedCallback must keep them as is.
-     */
-    for (unsigned i = 0; i != checkPointersLength; ++i)
-        CHECK(checkPointers[i]);
-
-    /*
-     * Overwrite the registers and stack with new GC things to avoid false
-     * positives with the finalization test.
-     */
-    EVAL("[]", root.addr());
-
-    array = JSVAL_TO_OBJECT(root.value());
-    JS_ASSERT(JS_IsArrayObject(cx, array));
-
-    uint32_t tmp;
-    CHECK(JS_GetArrayLength(cx, array, &tmp));
-    CHECK(ok);
-
-    return true;
-}
-
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -2902,22 +2902,19 @@ JS_SetGCCallback(JSRuntime *rt, JSGCCall
 JS_PUBLIC_API(void)
 JS_SetFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb)
 {
     AssertHeapIsIdle(rt);
     rt->gcFinalizeCallback = cb;
 }
 
 JS_PUBLIC_API(JSBool)
-JS_IsAboutToBeFinalized(void *thing)
-{
-    gc::Cell *t = static_cast<gc::Cell *>(thing);
-    bool isDying = IsCellAboutToBeFinalized(&t);
-    JS_ASSERT(t == thing);
-    return isDying;
+JS_IsAboutToBeFinalized(JSObject **obj)
+{
+    return IsObjectAboutToBeFinalized(obj);
 }
 
 JS_PUBLIC_API(void)
 JS_SetGCParameter(JSRuntime *rt, JSGCParamKey key, uint32_t value)
 {
     switch (key) {
       case JSGC_MAX_BYTES: {
         JS_ASSERT(value >= rt->gcBytes);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2623,18 +2623,34 @@ extern JS_PUBLIC_API(void)
 JS_SetGCCallback(JSRuntime *rt, JSGCCallback cb);
 
 extern JS_PUBLIC_API(void)
 JS_SetFinalizeCallback(JSRuntime *rt, JSFinalizeCallback cb);
 
 extern JS_PUBLIC_API(JSBool)
 JS_IsGCMarkingTracer(JSTracer *trc);
 
+/*
+ * JS_IsAboutToBeFinalized checks if the given object is going to be finalized
+ * at the end of the current GC. When called outside of the context of a GC,
+ * this function will return false. Typically this function is used on weak
+ * references, where the reference should be nulled out or destroyed if the
+ * given object is about to be finalized.
+ *
+ * The argument to JS_IsAboutToBeFinalized is an in-out param: when the
+ * function returns false, the object being referenced is still alive, but the
+ * garbage collector might have moved it. In this case, the reference passed
+ * to JS_IsAboutToBeFinalized will be updated to the object's new location.
+ * Callers of this method are responsible for updating any state that is
+ * dependent on the object's address. For example, if the object's address is
+ * used as a key in a hashtable, then the object must be removed and
+ * re-inserted with the correct hash.
+ */
 extern JS_PUBLIC_API(JSBool)
-JS_IsAboutToBeFinalized(void *thing);
+JS_IsAboutToBeFinalized(JSObject **obj);
 
 typedef enum JSGCParamKey {
     /* Maximum nominal heap before last ditch GC. */
     JSGC_MAX_BYTES          = 0,
 
     /* Number of JS_malloc bytes before last ditch GC. */
     JSGC_MAX_MALLOC_BYTES   = 1,
 
--- a/js/src/jstypedarray.cpp
+++ b/js/src/jstypedarray.cpp
@@ -718,17 +718,17 @@ ArrayBufferObject::sweep(JSCompartment *
 
         // Rebuild the list of views of the ArrayBuffer, discarding dead views.
         // If there is only one view, it will have already been marked.
         JSObject *prevLiveView = NULL;
         JSObject *view = *views;
         while (view) {
             JS_ASSERT(buffer->compartment() == view->compartment());
             JSObject *nextView = NextView(view);
-            if (!JS_IsAboutToBeFinalized(view)) {
+            if (!IsObjectAboutToBeFinalized(&view)) {
                 view->setFixedSlot(BufferView::NEXT_VIEW_SLOT, PrivateValue(prevLiveView));
                 prevLiveView = view;
             }
             view = nextView;
         }
         *views = prevLiveView;
 
         buffer = nextBuffer;
--- a/js/xpconnect/src/XPCMaps.cpp
+++ b/js/xpconnect/src/XPCMaps.cpp
@@ -84,20 +84,18 @@ void
 JSObject2WrappedJSMap::FindDyingJSObjects(nsTArray<nsXPCWrappedJS*>* dying)
 {
     for (Map::Range r = mTable.all(); !r.empty(); r.popFront()) {
         nsXPCWrappedJS* wrapper = r.front().value;
         NS_ASSERTION(wrapper, "found a null JS wrapper!");
 
         // walk the wrapper chain and find any whose JSObject is to be finalized
         while (wrapper) {
-            if (wrapper->IsSubjectToFinalization()) {
-                if (JS_IsAboutToBeFinalized(wrapper->GetJSObjectPreserveColor()))
-                    dying->AppendElement(wrapper);
-            }
+            if (wrapper->IsSubjectToFinalization() && wrapper->IsObjectAboutToBeFinalized())
+                dying->AppendElement(wrapper);
             wrapper = wrapper->GetNextWrapper();
         }
     }
 }
 
 void
 JSObject2WrappedJSMap::ShutdownMarker(JSRuntime* rt)
 {
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -637,18 +637,21 @@ public:
         NS_PRECONDITION(key,"bad param");
         mTable.remove(key);
     }
 
     inline uint32_t Count() { return mTable.count(); }
 
     void Sweep() {
         for (Map::Enum e(mTable); !e.empty(); e.popFront()) {
-            if (JS_IsAboutToBeFinalized(e.front().key) || JS_IsAboutToBeFinalized(e.front().value))
+            JSObject *updated = e.front().key;
+            if (JS_IsAboutToBeFinalized(&updated) || JS_IsAboutToBeFinalized(&e.front().value))
                 e.removeFront();
+            else if (updated != e.front().key)
+                e.rekeyFront(updated);
         }
     }
 
     void Reparent(JSContext *aCx, JSObject *aNewInner) {
         for (Map::Enum e(mTable); !e.empty(); e.popFront()) {
             /*
              * We reparent wrappers that have as their parent an inner window
              * whose outer has the new inner window as its current inner.
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1271,17 +1271,17 @@ XPCWrappedNative::FlatJSObjectFinalized(
     // JSObjects are about to be finalized too.
 
     XPCWrappedNativeTearOffChunk* chunk;
     for (chunk = &mFirstChunk; chunk; chunk = chunk->mNextChunk) {
         XPCWrappedNativeTearOff* to = chunk->mTearOffs;
         for (int i = XPC_WRAPPED_NATIVE_TEAROFFS_PER_CHUNK-1; i >= 0; i--, to++) {
             JSObject* jso = to->GetJSObjectPreserveColor();
             if (jso) {
-                NS_ASSERTION(JS_IsAboutToBeFinalized(jso), "bad!");
+                NS_ASSERTION(JS_IsAboutToBeFinalized(&jso), "bad!");
                 JS_SetPrivate(jso, nullptr);
                 to->JSObjectFinalized();
             }
 
             // We also need to release any native pointers held...
             nsISupports* obj = to->GetNative();
             if (obj) {
 #ifdef XP_WIN
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -470,32 +470,29 @@ XPCWrappedNativeScope::StartFinalization
 
     while (cur) {
         // Sweep waivers.
         if (cur->mWaiverWrapperMap)
             cur->mWaiverWrapperMap->Sweep();
 
         XPCWrappedNativeScope* next = cur->mNext;
 
-        if (cur->mGlobalJSObject &&
-            JS_IsAboutToBeFinalized(cur->mGlobalJSObject)) {
+        if (cur->mGlobalJSObject && cur->mGlobalJSObject.isAboutToBeFinalized()) {
             cur->mGlobalJSObject.finalize(fop->runtime());
             // Move this scope from the live list to the dying list.
             if (prev)
                 prev->mNext = next;
             else
                 gScopes = next;
             cur->mNext = gDyingScopes;
             gDyingScopes = cur;
             cur = nullptr;
         } else {
-            if (cur->mPrototypeNoHelper &&
-                JS_IsAboutToBeFinalized(cur->mPrototypeNoHelper)) {
+            if (cur->mPrototypeNoHelper && JS_IsAboutToBeFinalized(&cur->mPrototypeNoHelper))
                 cur->mPrototypeNoHelper = nullptr;
-            }
         }
         if (cur)
             prev = cur;
         cur = next;
     }
 }
 
 // static
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -3181,16 +3181,17 @@ public:
 
     // This is used by XPCJSRuntime::GCCallback to find wrappers that no
     // longer root their JSObject and are only still alive because they
     // were being used via nsSupportsWeakReference at the time when their
     // last (outside) reference was released. Wrappers that fit into that
     // category are only deleted when we see that their corresponding JSObject
     // is to be finalized.
     JSBool IsSubjectToFinalization() const {return IsValid() && mRefCnt == 1;}
+    JSBool IsObjectAboutToBeFinalized() {return JS_IsAboutToBeFinalized(&mJSObj);}
 
     JSBool IsAggregatedToNative() const {return mRoot->mOuter != nullptr;}
     nsISupports* GetAggregatedNativeObject() const {return mRoot->mOuter;}
 
     void SetIsMainThreadOnly() {
         MOZ_ASSERT(mMainThread);
         mMainThreadOnly = true;
     }