Bug 1352430 - Update XPConnect sweeping to handle incrementally finalized objects r=mccr8 r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 26 Apr 2017 11:18:13 +0100
changeset 403180 09be4ae7bbf0217af921b4fdb93360b8389338a6
parent 403179 42dcd77916bf4f07c76218a2a42f1d4a1e472a51
child 403181 0203cc1f2d2f017f58bf00c676d9f8f819546287
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8, sfink
bugs1352430
milestone55.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 1352430 - Update XPConnect sweeping to handle incrementally finalized objects r=mccr8 r=sfink
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/XPCWrappedNativeProto.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -618,16 +618,23 @@ js::ZoneGlobalsAreAllGray(JS::Zone* zone
     for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
         JSObject* obj = comp->unsafeUnbarrieredMaybeGlobal();
         if (!obj || !JS::ObjectIsMarkedGray(obj))
             return false;
     }
     return true;
 }
 
+JS_FRIEND_API(bool)
+js::IsObjectZoneSweepingOrCompacting(JSObject* obj)
+{
+    MOZ_ASSERT(obj);
+    return MaybeForwarded(obj)->zone()->isGCSweepingOrCompacting();
+}
+
 namespace {
 struct VisitGrayCallbackFunctor {
     GCThingCallback callback_;
     void* closure_;
     VisitGrayCallbackFunctor(GCThingCallback callback, void* closure)
       : callback_(callback), closure_(closure)
     {}
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -467,16 +467,19 @@ extern JS_FRIEND_API(void)
 TraceWeakMaps(WeakMapTracer* trc);
 
 extern JS_FRIEND_API(bool)
 AreGCGrayBitsValid(JSContext* cx);
 
 extern JS_FRIEND_API(bool)
 ZoneGlobalsAreAllGray(JS::Zone* zone);
 
+extern JS_FRIEND_API(bool)
+IsObjectZoneSweepingOrCompacting(JSObject* obj);
+
 typedef void
 (*GCThingCallback)(void* closure, JS::GCCellPtr thing);
 
 extern JS_FRIEND_API(void)
 VisitGrayWrapperTargets(JS::Zone* zone, GCThingCallback callback, void* closure);
 
 extern JS_FRIEND_API(JSObject*)
 GetWeakmapKeyDelegate(JSObject* key);
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -823,26 +823,26 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp*
             self->mDoingFinalization = true;
 
             break;
         }
         case JSFINALIZE_GROUP_START:
         {
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
 
-            // Sweep scopes needing cleanup
-            XPCWrappedNativeScope::KillDyingScopes();
-
             MOZ_ASSERT(self->mGCIsRunning, "bad state");
             self->mGCIsRunning = false;
 
             break;
         }
         case JSFINALIZE_GROUP_END:
         {
+            // Sweep scopes needing cleanup
+            XPCWrappedNativeScope::KillDyingScopes();
+
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
             self->mDoingFinalization = false;
 
             break;
         }
         case JSFINALIZE_COLLECTION_END:
         {
             MOZ_ASSERT(!self->mGCIsRunning, "bad state");
@@ -915,17 +915,17 @@ XPCJSRuntime::WeakPointerZonesCallback(J
 {
     // Called before each sweeping slice -- after processing any final marking
     // triggered by barriers -- to clear out any references to things that are
     // about to be finalized and update any pointers to moved GC things.
     XPCJSRuntime* self = static_cast<XPCJSRuntime*>(data);
 
     self->mWrappedJSMap->UpdateWeakPointersAfterGC();
 
-    XPCWrappedNativeScope::UpdateWeakPointersAfterGC();
+    XPCWrappedNativeScope::UpdateWeakPointersInAllScopesAfterGC();
 }
 
 /* static */ void
 XPCJSRuntime::WeakPointerCompartmentCallback(JSContext* cx, JSCompartment* comp, void* data)
 {
     // Called immediately after the ZoneGroup weak pointer callback, but only
     // once for each compartment that is being swept.
     CompartmentPrivate* xpcComp = CompartmentPrivate::Get(comp);
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -142,16 +142,18 @@ public:
         MOZ_ASSERT(!wrapperInMap || wrapperInMap == wrapper,
                    "About to remove a different wrapper with the same "
                    "nsISupports identity! This will most likely cause serious "
                    "problems!");
 #endif
         mTable.Remove(wrapper->GetIdentityObject());
     }
 
+    inline void Clear() { mTable.Clear(); }
+
     inline uint32_t Count() { return mTable.EntryCount(); }
 
     PLDHashTable::Iterator Iter() { return mTable.Iter(); }
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 private:
     Native2WrappedNativeMap();    // no implementation
@@ -361,16 +363,18 @@ public:
     }
 
     inline void Remove(nsIClassInfo* info)
     {
         NS_PRECONDITION(info,"bad param");
         mTable.Remove(info);
     }
 
+    inline void Clear() { mTable.Clear(); }
+
     inline uint32_t Count() { return mTable.EntryCount(); }
 
     PLDHashTable::Iterator Iter() { return mTable.Iter(); }
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 private:
     ClassInfo2WrappedNativeProtoMap();    // no implementation
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -564,24 +564,24 @@ XPCWrappedNative::~XPCWrappedNative()
     Destroy();
 }
 
 void
 XPCWrappedNative::Destroy()
 {
     mScriptable = nullptr;
 
+#ifdef DEBUG
+    // Check that this object has already been swept from the map.
     XPCWrappedNativeScope* scope = GetScope();
     if (scope) {
         Native2WrappedNativeMap* map = scope->GetWrappedNativeMap();
-
-        // Post-1.9 we should not remove this wrapper from the map if it is
-        // uninitialized.
-        map->Remove(this);
+        MOZ_ASSERT(map->Find(GetIdentityObject()) != this);
     }
+#endif
 
     if (mIdentity) {
         XPCJSRuntime* rt = GetRuntime();
         if (rt && rt->GetDoingFinalization()) {
             DeferredFinalize(mIdentity.forget().take());
         } else {
             mIdentity = nullptr;
         }
--- a/js/xpconnect/src/XPCWrappedNativeProto.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeProto.cpp
@@ -98,20 +98,21 @@ XPCWrappedNativeProto::CallPostCreatePro
     return true;
 }
 
 void
 XPCWrappedNativeProto::JSProtoObjectFinalized(js::FreeOp* fop, JSObject* obj)
 {
     MOZ_ASSERT(obj == mJSProtoObject, "huh?");
 
-    // Only remove this proto from the map if it is the one in the map.
+#ifdef DEBUG
+    // Check that this object has already been swept from the map.
     ClassInfo2WrappedNativeProtoMap* map = GetScope()->GetWrappedNativeProtoMap();
-    if (map->Find(mClassInfo) == this)
-        map->Remove(mClassInfo);
+    MOZ_ASSERT(map->Find(mClassInfo) != this);
+#endif
 
     GetRuntime()->GetDyingWrappedNativeProtoMap()->Add(this);
 
     mJSProtoObject.finalize(js::CastToJSFreeOp(fop)->runtime());
 }
 
 void
 XPCWrappedNativeProto::JSProtoObjectMoved(JSObject* obj, const JSObject* old)
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -474,19 +474,16 @@ XPCWrappedNativeScope::~XPCWrappedNative
     // XXX we should assert that we are dead or that xpconnect has shutdown
     // XXX might not want to do this at xpconnect shutdown time???
     mComponents = nullptr;
 
     if (mXrayExpandos.initialized())
         mXrayExpandos.destroy();
 
     JSContext* cx = dom::danger::GetJSContext();
-    mContentXBLScope.finalize(cx);
-    for (size_t i = 0; i < mAddonScopes.Length(); i++)
-        mAddonScopes[i].finalize(cx);
     mGlobalJSObject.finalize(cx);
 }
 
 // static
 void
 XPCWrappedNativeScope::TraceWrappedNativesInAllScopes(JSTracer* trc)
 {
     // Do JS::TraceEdge for all wrapped natives with external references, as
@@ -527,58 +524,118 @@ XPCWrappedNativeScope::SuspectAllWrapper
             for (DOMExpandoSet::Range r = cur->mDOMExpandoSet->all(); !r.empty(); r.popFront())
                 SuspectDOMExpandos(r.front().unbarrieredGet(), cb);
         }
     }
 }
 
 // static
 void
-XPCWrappedNativeScope::UpdateWeakPointersAfterGC()
+XPCWrappedNativeScope::UpdateWeakPointersInAllScopesAfterGC()
 {
     // If this is called from the finalization callback in JSGC_MARK_END then
     // JSGC_FINALIZE_END must always follow it calling
     // FinishedFinalizationPhaseOfGC and clearing gDyingScopes in
     // KillDyingScopes.
     MOZ_ASSERT(!gDyingScopes, "JSGC_MARK_END without JSGC_FINALIZE_END");
 
-    XPCWrappedNativeScope* prev = nullptr;
-    XPCWrappedNativeScope* cur = gScopes;
+    XPCWrappedNativeScope** scopep = &gScopes;
+    while (*scopep) {
+        XPCWrappedNativeScope* cur = *scopep;
+        cur->UpdateWeakPointersAfterGC();
+        if (cur->mGlobalJSObject) {
+            scopep = &cur->mNext;
+        } else {
+            // The scope's global is dead so move it to the dying scopes list.
+            *scopep = cur->mNext;
+            cur->mNext = gDyingScopes;
+            gDyingScopes = cur;
+        }
+    }
+}
 
-    while (cur) {
-        // Sweep waivers.
-        if (cur->mWaiverWrapperMap)
-            cur->mWaiverWrapperMap->Sweep();
+static inline void
+AssertSameCompartment(DebugOnly<JSCompartment*>& comp, JSObject* obj)
+{
+    MOZ_ASSERT_IF(obj, js::GetObjectCompartment(obj) == comp);
+}
 
-        XPCWrappedNativeScope* next = cur->mNext;
+static inline void
+AssertSameCompartment(DebugOnly<JSCompartment*>& comp, const JS::ObjectPtr& obj)
+{
+#ifdef DEBUG
+    AssertSameCompartment(comp, obj.unbarrieredGet());
+#endif
+}
 
-        if (cur->mContentXBLScope)
-            cur->mContentXBLScope.updateWeakPointerAfterGC();
-        for (size_t i = 0; i < cur->mAddonScopes.Length(); i++)
-            cur->mAddonScopes[i].updateWeakPointerAfterGC();
+void
+XPCWrappedNativeScope::UpdateWeakPointersAfterGC()
+{
+    // Sweep waivers.
+    if (mWaiverWrapperMap)
+        mWaiverWrapperMap->Sweep();
+
+    if (!js::IsObjectZoneSweepingOrCompacting(mGlobalJSObject.unbarrieredGet()))
+        return;
 
-        // Check for finalization of the global object or update our pointer if
-        // it was moved.
-        if (cur->mGlobalJSObject) {
-            cur->mGlobalJSObject.updateWeakPointerAfterGC();
-            if (!cur->mGlobalJSObject) {
-                // 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;
-            }
-        }
+    // Update our pointer to the global object in case it was moved or
+    // finalized.
+    mGlobalJSObject.updateWeakPointerAfterGC();
+    if (!mGlobalJSObject) {
+        JSContext* cx = dom::danger::GetJSContext();
+        mContentXBLScope.finalize(cx);
+        for (size_t i = 0; i < mAddonScopes.Length(); i++)
+            mAddonScopes[i].finalize(cx);
+        GetWrappedNativeMap()->Clear();
+        mWrappedNativeProtoMap->Clear();
+        return;
+    }
+
+    DebugOnly<JSCompartment*> comp =
+        js::GetObjectCompartment(mGlobalJSObject.unbarrieredGet());
 
-        if (cur)
-            prev = cur;
-        cur = next;
+#ifdef DEBUG
+    // These are traced, so no updates are necessary.
+    if (mContentXBLScope) {
+        JSObject* prev = mContentXBLScope.unbarrieredGet();
+        mContentXBLScope.updateWeakPointerAfterGC();
+        MOZ_ASSERT(prev == mContentXBLScope.unbarrieredGet());
+        AssertSameCompartment(comp, mContentXBLScope);
+    }
+    for (size_t i = 0; i < mAddonScopes.Length(); i++) {
+        JSObject* prev = mAddonScopes[i].unbarrieredGet();
+        mAddonScopes[i].updateWeakPointerAfterGC();
+        MOZ_ASSERT(prev == mAddonScopes[i].unbarrieredGet());
+        AssertSameCompartment(comp, mAddonScopes[i]);
+    }
+#endif
+
+    // Sweep mWrappedNativeMap for dying flat JS objects. Moving has already
+    // been handled by XPCWrappedNative::FlatJSObjectMoved.
+    for (auto iter = GetWrappedNativeMap()->Iter(); !iter.Done(); iter.Next()) {
+        auto entry = static_cast<Native2WrappedNativeMap::Entry*>(iter.Get());
+        XPCWrappedNative* wrapper = entry->value;
+        JSObject* obj = wrapper->GetFlatJSObjectPreserveColor();
+        JS_UpdateWeakPointerAfterGCUnbarriered(&obj);
+        MOZ_ASSERT(!obj || obj == wrapper->GetFlatJSObjectPreserveColor());
+        AssertSameCompartment(comp, obj);
+        if (!obj)
+            iter.Remove();
+    }
+
+    // Sweep mWrappedNativeProtoMap for dying prototype JSObjects. Moving has
+    // already been handled by XPCWrappedNativeProto::JSProtoObjectMoved.
+    for (auto i = mWrappedNativeProtoMap->Iter(); !i.Done(); i.Next()) {
+        auto entry = static_cast<ClassInfo2WrappedNativeProtoMap::Entry*>(i.Get());
+        JSObject* obj = entry->value->GetJSProtoObjectPreserveColor();
+        JS_UpdateWeakPointerAfterGCUnbarriered(&obj);
+        AssertSameCompartment(comp, obj);
+        MOZ_ASSERT(!obj || obj == entry->value->GetJSProtoObjectPreserveColor());
+        if (!obj)
+            i.Remove();
     }
 }
 
 // static
 void
 XPCWrappedNativeScope::SweepAllWrappedNativeTearOffs()
 {
     for (XPCWrappedNativeScope* cur = gScopes; cur; cur = cur->mNext) {
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -957,16 +957,19 @@ public:
 
     static void
     SuspectAllWrappers(nsCycleCollectionNoteRootCallback& cb);
 
     static void
     SweepAllWrappedNativeTearOffs();
 
     static void
+    UpdateWeakPointersInAllScopesAfterGC();
+
+    void
     UpdateWeakPointersAfterGC();
 
     static void
     KillDyingScopes();
 
     static void
     DebugDumpAllScopes(int16_t depth);
 
@@ -1439,16 +1442,19 @@ public:
     GetScope()   const {return mScope;}
 
     XPCJSRuntime*
     GetRuntime() const {return mScope->GetRuntime();}
 
     JSObject*
     GetJSProtoObject() const { return mJSProtoObject; }
 
+    JSObject*
+    GetJSProtoObjectPreserveColor() const { return mJSProtoObject.unbarrieredGet(); }
+
     nsIClassInfo*
     GetClassInfo()     const {return mClassInfo;}
 
     XPCNativeSet*
     GetSet()           const {return mSet;}
 
     nsIXPCScriptable*
     GetScriptable() const { return mScriptable; }