Add runtime NS_IsMainThread checks to protect the cycle collector's purple buffer, and make NS_IsMainThread faster on our major platforms. (Bug 521750) r=peterv a1.9.1.5=ss GECKO1914_20091006_RELBRANCH
authorBenjamin Smedberg <benjamin@smedbergs.us>, L. David Baron <dbaron@dbaron.org>
Wed, 28 Oct 2009 10:28:57 -0700
branchGECKO1914_20091006_RELBRANCH
changeset 26517 40f1c74ccf91c71f8c02485e5c7be85f0fb6dffc
parent 26516 4dba03f79519fa39c209987176c80824df420d5c
child 26518 edf189567edc4e7984f111709b9a1c58cf6ea6ed
push id2079
push userdbaron@mozilla.com
push dateMon, 02 Nov 2009 17:11:27 +0000
reviewerspeterv
bugs521750
milestone1.9.1.4
Add runtime NS_IsMainThread checks to protect the cycle collector's purple buffer, and make NS_IsMainThread faster on our major platforms. (Bug 521750) r=peterv a1.9.1.5=ss
xpcom/base/nsCycleCollector.cpp
xpcom/glue/nsISupportsImpl.h
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -736,20 +736,22 @@ public:
     void UnmarkRemainingPurple(Block *b)
     {
         for (nsPurpleBufferEntry *e = b->mEntries,
                               *eEnd = e + NS_ARRAY_LENGTH(b->mEntries);
              e != eEnd; ++e) {
             if (!(PRUword(e->mObject) & PRUword(1))) {
                 // This is a real entry (rather than something on the
                 // free list).
-                nsXPCOMCycleCollectionParticipant *cp;
-                ToParticipant(e->mObject, &cp);
-
-                cp->UnmarkPurple(e->mObject);
+                if (e->mObject) {
+                    nsXPCOMCycleCollectionParticipant *cp;
+                    ToParticipant(e->mObject, &cp);
+
+                    cp->UnmarkPurple(e->mObject);
+                }
 
                 if (--mCount == 0)
                     break;
             }
         }
     }
 
     void SelectPointers(GCGraphBuilder &builder);
@@ -878,17 +880,17 @@ nsPurpleBuffer::SelectPointers(GCGraphBu
     // Walk through all the blocks.
     for (Block *b = &mFirstBlock; b; b = b->mNext) {
         for (nsPurpleBufferEntry *e = b->mEntries,
                               *eEnd = e + NS_ARRAY_LENGTH(b->mEntries);
             e != eEnd; ++e) {
             if (!(PRUword(e->mObject) & PRUword(1))) {
                 // This is a real entry (rather than something on the
                 // free list).
-                if (AddPurpleRoot(aBuilder, e->mObject)) {
+                if (!e->mObject || AddPurpleRoot(aBuilder, e->mObject)) {
 #ifdef DEBUG_CC
                     mNormalObjects.RemoveEntry(e->mObject);
 #endif
                     --mCount;
                     // Put this entry on the free list in case some
                     // call to AddPurpleRoot fails and we don't rebuild
                     // the free list below.
                     e->mNextInFreeList = (nsPurpleBufferEntry*)
@@ -1102,17 +1104,17 @@ Fault(const char *msg, const void *ptr=n
     // probably a better user experience than crashing. Besides, we
     // *should* never hit a fault.
 
     sCollector->mParams.mDoNothing = PR_TRUE;
 
     // Report to observers off an event so we don't run JS under GC
     // (which is where we might be right now).
     nsCOMPtr<nsIRunnable> ev = new CCRunnableFaultReport(str);
-    NS_DispatchToCurrentThread(ev);
+    NS_DispatchToMainThread(ev);
 }
 
 #ifdef DEBUG_CC
 static void
 Fault(const char *msg, PtrInfo *pi)
 {
     printf("Fault in cycle collector: %s\n"
            "  while operating on pointer %p %s\n",
@@ -1136,17 +1138,25 @@ Fault(const char *msg, PtrInfo *pi)
 #else
 inline void
 Fault(const char *msg, PtrInfo *pi)
 {
     Fault(msg, pi->mPointer);
 }
 #endif
 
-
+static inline bool
+CheckMainThreadIfFast()
+{
+#ifdef NS_TLS
+    return NS_IsMainThread();
+#else
+    return true;
+#endif
+}
 
 static nsISupports *
 canonicalize(nsISupports *in)
 {
     nsCOMPtr<nsISupports> child;
     in->QueryInterface(NS_GET_IID(nsCycleCollectionISupports),
                        getter_AddRefs(child));
     return child.get();
@@ -2152,26 +2162,28 @@ nsCycleCollector_isScanSafe(nsISupports 
 
     return cp != nsnull;
 }
 #endif
 
 PRBool
 nsCycleCollector::Suspect(nsISupports *n)
 {
+    if (!CheckMainThreadIfFast())
+        return PR_FALSE;
+
     // Re-entering ::Suspect during collection used to be a fault, but
     // we are canonicalizing nsISupports pointers using QI, so we will
     // see some spurious refcount traffic here. 
 
     if (mScanInProgress)
         return PR_FALSE;
 
     NS_ASSERTION(nsCycleCollector_isScanSafe(n),
                  "suspected a non-scansafe pointer");
-    NS_ASSERTION(NS_IsMainThread(), "trying to suspect from non-main thread");
 
     if (mParams.mDoNothing)
         return PR_FALSE;
 
 #ifdef DEBUG_CC
     mStats.mSuspectNode++;
 
     if (nsCycleCollector_shouldSuppress(n))
@@ -2191,25 +2203,30 @@ nsCycleCollector::Suspect(nsISupports *n
 
     return mPurpleBuf.PutCompatObject(n);
 }
 
 
 PRBool
 nsCycleCollector::Forget(nsISupports *n)
 {
+    if (!CheckMainThreadIfFast()) {
+        if (!mParams.mDoNothing) {
+            Fault("Forget called off main thread");
+        }
+        return PR_TRUE; // it's as good as forgotten
+    }
+
     // Re-entering ::Forget during collection used to be a fault, but
     // we are canonicalizing nsISupports pointers using QI, so we will
     // see some spurious refcount traffic here. 
 
     if (mScanInProgress)
         return PR_FALSE;
 
-    NS_ASSERTION(NS_IsMainThread(), "trying to forget from non-main thread");
-    
     if (mParams.mDoNothing)
         return PR_TRUE; // it's as good as forgotten
 
 #ifdef DEBUG_CC
     mStats.mForgetNode++;
 
 #ifndef __MINGW32__
     if (mParams.mHookMalloc)
@@ -2225,26 +2242,28 @@ nsCycleCollector::Forget(nsISupports *n)
 
     mPurpleBuf.RemoveCompatObject(n);
     return PR_TRUE;
 }
 
 nsPurpleBufferEntry*
 nsCycleCollector::Suspect2(nsISupports *n)
 {
+    if (!CheckMainThreadIfFast())
+        return nsnull;
+
     // Re-entering ::Suspect during collection used to be a fault, but
     // we are canonicalizing nsISupports pointers using QI, so we will
     // see some spurious refcount traffic here. 
 
     if (mScanInProgress)
         return nsnull;
 
     NS_ASSERTION(nsCycleCollector_isScanSafe(n),
                  "suspected a non-scansafe pointer");
-    NS_ASSERTION(NS_IsMainThread(), "trying to suspect from non-main thread");
 
     if (mParams.mDoNothing)
         return nsnull;
 
 #ifdef DEBUG_CC
     mStats.mSuspectNode++;
 
     if (nsCycleCollector_shouldSuppress(n))
@@ -2265,25 +2284,26 @@ nsCycleCollector::Suspect2(nsISupports *
     // Caller is responsible for filling in result's mRefCnt.
     return mPurpleBuf.Put(n);
 }
 
 
 PRBool
 nsCycleCollector::Forget2(nsPurpleBufferEntry *e)
 {
+    if (!CheckMainThreadIfFast())
+        return PR_FALSE;
+
     // Re-entering ::Forget during collection used to be a fault, but
     // we are canonicalizing nsISupports pointers using QI, so we will
     // see some spurious refcount traffic here. 
 
     if (mScanInProgress)
         return PR_FALSE;
 
-    NS_ASSERTION(NS_IsMainThread(), "trying to forget from non-main thread");
-    
 #ifdef DEBUG_CC
     mStats.mForgetNode++;
 
 #ifndef __MINGW32__
     if (mParams.mHookMalloc)
         InitMemHook();
 #endif
 
--- a/xpcom/glue/nsISupportsImpl.h
+++ b/xpcom/glue/nsISupportsImpl.h
@@ -178,16 +178,18 @@ public:
       nsPurpleBufferEntry *e = NS_CCAR_TAGGED_TO_PURPLE_ENTRY(mTagged);
       NS_ASSERTION(e->mObject == owner, "wrong entry");
       refcount = e->mRefCnt;
       --refcount;
       
       if (NS_UNLIKELY(refcount == 0)) {
         if (NS_UNLIKELY(!NS_CycleCollectorForget2(e))) {
           NS_NOTREACHED("forget should not fail when reference count hits 0");
+          // Clear the entry's pointer to us.
+          e->mObject = nsnull;
         }
         mTagged = NS_CCAR_REFCNT_TO_TAGGED(refcount);
       } else {
         e->mRefCnt = refcount;
       }
     } else {
       refcount = NS_CCAR_TAGGED_TO_REFCNT(mTagged);
       --refcount;