Bug 1352430 - Update XPConnect sweeping to handle incrementally finalized objects r=mccr8 r=sfink

--HG--
extra : rebase_source : 10c974bedd003cf23de9e32ad7ae202441c92db9

js/src/jsfriendapi.cpp

@@ -623,6 +623,13 @@ js::ZoneGlobalsAreAllGray(JS::Zone* zone)
     return true;
 }
 
+JS_FRIEND_API(bool)
+js::IsObjectZoneSweepingOrCompacting(JSObject* obj)
+{
+    MOZ_ASSERT(obj);
+    return MaybeForwarded(obj)->zone()->isGCSweepingOrCompacting();
+}
+
 namespace {
 struct VisitGrayCallbackFunctor {
     GCThingCallback callback_;

js/src/jsfriendapi.h

@@ -472,6 +472,9 @@ 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);
 

js/xpconnect/src/XPCJSRuntime.cpp

@@ -828,9 +828,6 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp* fop,
         {
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
 
-            // Sweep scopes needing cleanup
-            XPCWrappedNativeScope::KillDyingScopes();
-
             MOZ_ASSERT(self->mGCIsRunning, "bad state");
             self->mGCIsRunning = false;
 
@@ -838,6 +835,9 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp* fop,
         }
         case JSFINALIZE_GROUP_END:
         {
+            // Sweep scopes needing cleanup
+            XPCWrappedNativeScope::KillDyingScopes();
+
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
             self->mDoingFinalization = false;
 
@@ -920,7 +920,7 @@ XPCJSRuntime::WeakPointerZonesCallback(JSContext* cx, void* data)
 
     self->mWrappedJSMap->UpdateWeakPointersAfterGC();
 
-    XPCWrappedNativeScope::UpdateWeakPointersAfterGC();
+    XPCWrappedNativeScope::UpdateWeakPointersInAllScopesAfterGC();
 }
 
 /* static */ void

js/xpconnect/src/XPCMaps.h

@@ -147,6 +147,8 @@ public:
         mTable.Remove(wrapper->GetIdentityObject());
     }
 
+    inline void Clear() { mTable.Clear(); }
+
     inline uint32_t Count() { return mTable.EntryCount(); }
 
     PLDHashTable::Iterator Iter() { return mTable.Iter(); }
@@ -366,6 +368,8 @@ public:
         mTable.Remove(info);
     }
 
+    inline void Clear() { mTable.Clear(); }
+
     inline uint32_t Count() { return mTable.EntryCount(); }
 
     PLDHashTable::Iterator Iter() { return mTable.Iter(); }

js/xpconnect/src/XPCWrappedNative.cpp

@@ -569,14 +569,14 @@ 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();

js/xpconnect/src/XPCWrappedNativeProto.cpp

@@ -103,10 +103,11 @@ 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);
 

js/xpconnect/src/XPCWrappedNativeScope.cpp

@@ -479,9 +479,6 @@ XPCWrappedNativeScope::~XPCWrappedNativeScope()
         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);
 }
 
@@ -532,7 +529,7 @@ XPCWrappedNativeScope::SuspectAllWrappers(nsCycleCollectionNoteRootCallback& 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
@@ -540,40 +537,100 @@ XPCWrappedNativeScope::UpdateWeakPointersAfterGC()
     // KillDyingScopes.
     MOZ_ASSERT(!gDyingScopes, "JSGC_MARK_END without JSGC_FINALIZE_END");
 
-    XPCWrappedNativeScope* prev = nullptr;
-    XPCWrappedNativeScope* cur = gScopes;
-
-    while (cur) {
-        // Sweep waivers.
-        if (cur->mWaiverWrapperMap)
-            cur->mWaiverWrapperMap->Sweep();
-
-        XPCWrappedNativeScope* next = cur->mNext;
-
-        if (cur->mContentXBLScope)
-            cur->mContentXBLScope.updateWeakPointerAfterGC();
-        for (size_t i = 0; i < cur->mAddonScopes.Length(); i++)
-            cur->mAddonScopes[i].updateWeakPointerAfterGC();
-
-        // Check for finalization of the global object or update our pointer if
-        // it was moved.
+    XPCWrappedNativeScope** scopep = &gScopes;
+    while (*scopep) {
+        XPCWrappedNativeScope* cur = *scopep;
+        cur->UpdateWeakPointersAfterGC();
         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;
-            }
+            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;
         }
+    }
+}
 
-        if (cur)
-            prev = cur;
-        cur = next;
+static inline void
+AssertSameCompartment(DebugOnly<JSCompartment*>& comp, JSObject* obj)
+{
+    MOZ_ASSERT_IF(obj, js::GetObjectCompartment(obj) == comp);
+}
+
+static inline void
+AssertSameCompartment(DebugOnly<JSCompartment*>& comp, const JS::ObjectPtr& obj)
+{
+#ifdef DEBUG
+    AssertSameCompartment(comp, obj.unbarrieredGet());
+#endif
+}
+
+void
+XPCWrappedNativeScope::UpdateWeakPointersAfterGC()
+{
+    // Sweep waivers.
+    if (mWaiverWrapperMap)
+        mWaiverWrapperMap->Sweep();
+
+    if (!js::IsObjectZoneSweepingOrCompacting(mGlobalJSObject.unbarrieredGet()))
+        return;
+
+    // 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());
+
+#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();
     }
 }
 

js/xpconnect/src/xpcprivate.h

@@ -962,6 +962,9 @@ public:
     SweepAllWrappedNativeTearOffs();
 
     static void
+    UpdateWeakPointersInAllScopesAfterGC();
+
+    void
     UpdateWeakPointersAfterGC();
 
     static void
@@ -1444,6 +1447,9 @@ public:
     JSObject*
     GetJSProtoObject() const { return mJSProtoObject; }
 
+    JSObject*
+    GetJSProtoObjectPreserveColor() const { return mJSProtoObject.unbarrieredGet(); }
+
     nsIClassInfo*
     GetClassInfo()     const {return mClassInfo;}