Backed out changeset 81428de4b5dc (Fix for bug 464676 (Cycle collector sometimes unlinks live cycles). r=bent, sr=jst.).

js/src/xpconnect/src/nsXPConnect.cpp

@@ -706,35 +706,6 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
     JSContext *cx = mCycleCollectionContext->GetJSContext();
 
     uint32 traceKind = js_GetGCThingTraceKind(p);
-    JSObject *obj;
-    JSClass *clazz;
-
-    // We do not want to add wrappers to the cycle collector if their native
-    // can't participate because they might be used off of the main thread, the
-    // cycle collector isn't able to deal with such objects. We do want to
-    // explicitly mark them for cycle collection if the wrapper has an
-    // external reference, because the wrapper would mark the JS object if
-    // we did add the wrapper to the cycle collector.
-    JSBool dontTraverse = PR_FALSE;
-    JSBool markJSObject = PR_FALSE;
-    if(traceKind == JSTRACE_OBJECT)
-    {
-        obj = static_cast<JSObject*>(p);
-        clazz = OBJ_GET_CLASS(cx, obj);
-
-        if(clazz == &XPC_WN_Tearoff_JSClass)
-        {
-            XPCWrappedNative *wrapper =
-                (XPCWrappedNative*)xpc_GetJSPrivate(STOBJ_GET_PARENT(obj));
-            dontTraverse = wrapper->DontCycleCollect();
-        }
-        else if(IS_WRAPPER_CLASS(clazz))
-        {
-            XPCWrappedNative *wrapper = (XPCWrappedNative*)xpc_GetJSPrivate(obj);
-            dontTraverse = wrapper->DontCycleCollect();
-            markJSObject = dontTraverse && wrapper->HasExternalReference();
-        }
-    }
 
     CCNodeType type;
 
@@ -755,14 +726,12 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
         // ExplainLiveExpectedGarbage codepath
         PLDHashEntryHdr* entry =
             PL_DHashTableOperate(&mJSRoots, p, PL_DHASH_LOOKUP);
-        type = markJSObject || PL_DHASH_ENTRY_IS_BUSY(entry) ? GCMarked :
-                                                               GCUnmarked;
+        type = PL_DHASH_ENTRY_IS_BUSY(entry) ? GCMarked : GCUnmarked;
     }
     else
     {
         // Normal codepath (matches non-DEBUG_CC codepath).
-        type = !markJSObject && JS_IsAboutToBeFinalized(cx, p) ? GCUnmarked :
-                                                                 GCMarked;
+        type = JS_IsAboutToBeFinalized(cx, p) ? GCUnmarked : GCMarked;
     }
 
     char name[72];
@@ -886,8 +855,7 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
 
     }
 #else
-    type = !markJSObject && JS_IsAboutToBeFinalized(cx, p) ? GCUnmarked :
-                                                             GCMarked;
+    type = JS_IsAboutToBeFinalized(cx, p) ? GCUnmarked : GCMarked;
     cb.DescribeNode(type, 0);
 #endif
 
@@ -908,9 +876,12 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
     JS_TRACER_INIT(&trc, cx, NoteJSChild);
     JS_TraceChildren(&trc, p, traceKind);
 
-    if(traceKind != JSTRACE_OBJECT || dontTraverse)
+    if(traceKind != JSTRACE_OBJECT)
         return NS_OK;
     
+    JSObject *obj = static_cast<JSObject*>(p);
+    JSClass* clazz = OBJ_GET_CLASS(cx, obj);
+
     if(clazz == &XPC_WN_Tearoff_JSClass)
     {
         // A tearoff holds a strong reference to its native object

js/src/xpconnect/src/xpcprivate.h

@@ -2193,29 +2193,11 @@ public:
                                 GetProto()->GetLock() : nsnull;}
 
     XPCNativeSet*
-    GetSet() const {XPCAutoLock al(GetLock()); return GetSetNoLock();}
-
-#define XPC_SET_WORD(s)        ((jsword)(s))
-#define XPC_SET_MASK           ((jsword)0x1)
-#define XPC_NOT_CC_PARTICIPANT ((jsword)0x1)
-
-    inline JSBool
-    DontCycleCollect() const
-        {return XPC_SET_WORD(mSet) & XPC_NOT_CC_PARTICIPANT;}
+    GetSet() const {XPCAutoLock al(GetLock()); return mSet;}
 
 private:
-    inline void
-    SetSet(XPCNativeSet* set, PRBool aDontCycleCollect)
-        {mSet = aDontCycleCollect ?
-                (XPCNativeSet*)(XPC_SET_WORD(set) | XPC_NOT_CC_PARTICIPANT) :
-                set;}
-
-    inline void
-    SetSet(XPCNativeSet* set) {SetSet(set, DontCycleCollect());}
-
-    inline XPCNativeSet*
-    GetSetNoLock() const
-        {return (XPCNativeSet*)(XPC_SET_WORD(mSet) & ~XPC_SET_MASK);}
+    void
+    SetSet(XPCNativeSet* set) {XPCAutoLock al(GetLock()); mSet = set;}
 
     inline void
     ExpireWrapper()
@@ -2329,7 +2311,7 @@ public:
                                          nsresult* pError = nsnull);
     void Mark() const
     {
-        GetSetNoLock()->Mark();
+        mSet->Mark();
         if(mScriptableInfo) mScriptableInfo->Mark();
         if(HasProto()) GetProto()->Mark();
     }

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -653,19 +653,15 @@ XPCWrappedNative::GetUsedOnly(XPCCallContext& ccx,
 XPCWrappedNative::XPCWrappedNative(nsISupports* aIdentity,
                                    XPCWrappedNativeProto* aProto)
     : mMaybeProto(aProto),
+      mSet(aProto->GetSet()),
       mFlatJSObject((JSObject*)JSVAL_ONE), // non-null to pass IsValid() test
       mScriptableInfo(nsnull),
       mWrapper(nsnull)
 {
     NS_ADDREF(mIdentity = aIdentity);
 
-    nsCycleCollectionParticipant* cp;
-    PRBool noCC = !aProto->ClassIsMainThreadOnly() &&
-                  NS_FAILED(CallQueryInterface(aIdentity, &cp));
-    SetSet(aProto->GetSet(), noCC);
-
     NS_ASSERTION(mMaybeProto, "bad ctor param");
-    NS_ASSERTION(GetSetNoLock(), "bad ctor param");
+    NS_ASSERTION(mSet, "bad ctor param");
 
     DEBUG_TrackNewWrapper(this);
 }
@@ -676,16 +672,13 @@ XPCWrappedNative::XPCWrappedNative(nsISupports* aIdentity,
                                    XPCNativeSet* aSet)
 
     : mMaybeScope(TagScope(aScope)),
+      mSet(aSet),
       mFlatJSObject((JSObject*)JSVAL_ONE), // non-null to pass IsValid() test
       mScriptableInfo(nsnull),
       mWrapper(nsnull)
 {
     NS_ADDREF(mIdentity = aIdentity);
 
-    nsCycleCollectionParticipant* cp;
-    PRBool noCC = NS_FAILED(CallQueryInterface(aIdentity, &cp));
-    SetSet(aSet, noCC);
-
     NS_ASSERTION(aScope, "bad ctor param");
     NS_ASSERTION(aSet, "bad ctor param");
 
@@ -1140,12 +1133,11 @@ XPCWrappedNative::SystemIsBeingShutDown(JSContext* cx)
     {
         printf("Removing root for still-live XPCWrappedNative %p wrapping:\n",
                static_cast<void*>(this));
-        XPCNativeSet* set = GetSetNoLock();
-        for(PRUint16 i = 0, i_end = set->GetInterfaceCount(); i < i_end; ++i)
+        for(PRUint16 i = 0, i_end = mSet->GetInterfaceCount(); i < i_end; ++i)
         {
             nsXPIDLCString name;
-            set->GetInterfaceAt(i)->GetInterfaceInfo()
-               ->GetName(getter_Copies(name));
+            mSet->GetInterfaceAt(i)->GetInterfaceInfo()
+                ->GetName(getter_Copies(name));
             printf("  %s\n", name.get());
         }
     }
@@ -1495,16 +1487,16 @@ JSBool
 XPCWrappedNative::ExtendSet(XPCCallContext& ccx, XPCNativeInterface* aInterface)
 {
     // This is only called while locked (during XPCWrappedNative::FindTearOff).
-    XPCNativeSet* set = GetSetNoLock();
-    if(!set->HasInterface(aInterface))
+
+    if(!mSet->HasInterface(aInterface))
     {
         AutoMarkingNativeSetPtr newSet(ccx);
-        newSet = XPCNativeSet::GetNewOrUsed(ccx, set, aInterface,
-                                            set->GetInterfaceCount());
+        newSet = XPCNativeSet::GetNewOrUsed(ccx, mSet, aInterface,
+                                            mSet->GetInterfaceCount());
         if(!newSet)
             return JS_FALSE;
 
-        SetSet(newSet);
+        mSet = newSet;
 
         DEBUG_ReportShadowedMembers(newSet, this, GetProto());
     }
@@ -1636,11 +1628,10 @@ XPCWrappedNative::InitTearOff(XPCCallContext& ccx,
 
     // If the scriptable helper forbids us from reflecting additional
     // interfaces, then don't even try the QI, just fail.
-    XPCNativeSet* set = GetSetNoLock();
     if(mScriptableInfo &&
        mScriptableInfo->GetFlags().ClassInfoInterfacesOnly() &&
-       !set->HasInterface(aInterface) &&
-       !set->HasInterfaceWithAncestor(aInterface))
+       !mSet->HasInterface(aInterface) &&
+       !mSet->HasInterfaceWithAncestor(aInterface))
     {
         return NS_ERROR_NO_INTERFACE;
     }
@@ -1794,7 +1785,7 @@ XPCWrappedNative::InitTearOff(XPCCallContext& ccx,
     // because we unlocked and called out in the interim and the result of the
     // previous call might not be correct anymore.
 
-    if(!set->HasInterface(aInterface) && !ExtendSet(ccx, aInterface))
+    if(!mSet->HasInterface(aInterface) && !ExtendSet(ccx, aInterface))
     {
         NS_RELEASE(obj);
         aTearOff->SetInterface(nsnull);
@@ -2791,11 +2782,10 @@ NS_IMETHODIMP XPCWrappedNative::DebugDump(PRInt16 depth)
         else
             XPC_LOG_ALWAYS(("Scope @ %x", GetScope()));
 
-        XPCNativeSet* set = GetSetNoLock();
-        if(depth && set)
-            set->DebugDump(depth);
+        if(depth && mSet)
+            mSet->DebugDump(depth);
         else
-            XPC_LOG_ALWAYS(("mSet @ %x", set));
+            XPC_LOG_ALWAYS(("mSet @ %x", mSet));
 
         XPC_LOG_ALWAYS(("mFlatJSObject of %x", mFlatJSObject));
         XPC_LOG_ALWAYS(("mIdentity of %x", mIdentity));

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -395,7 +395,8 @@ WrappedNativeSuspecter(JSDHashTable *table, JSDHashEntryHdr *hdr,
 {
     SuspectClosure* closure = static_cast<SuspectClosure*>(arg);
     XPCWrappedNative* wrapper = ((Native2WrappedNativeMap::Entry*)hdr)->value;
-    if(wrapper->IsValid())
+    XPCWrappedNativeProto* proto = wrapper->GetProto();
+    if(proto && proto->ClassIsMainThreadOnly() && wrapper->IsValid())
     {
         NS_ASSERTION(NS_IsMainThread(), 
                      "Suspecting wrapped natives from non-main thread");