Bug 660778 - Deal with stack overflow in UnmarkGrayChildren (r=gal)

js/src/xpconnect/src/nsXPConnect.cpp

@@ -101,6 +101,7 @@ nsXPConnect::nsXPConnect()
         mDefaultSecurityManager(nsnull),
         mDefaultSecurityManagerFlags(0),
         mShuttingDown(JS_FALSE),
+        mNeedGCBeforeCC(JS_TRUE),
         mCycleCollectionContext(nsnull)
 {
     mRuntime = XPCJSRuntime::newXPCJSRuntime(this);
@@ -339,6 +340,12 @@ nsXPConnect::GetInfoForName(const char * name, nsIInterfaceInfo** info)
     return FindInfo(NameTester, name, mInterfaceInfoManager, info);
 }
 
+bool
+nsXPConnect::NeedCollect()
+{
+    return !!mNeedGCBeforeCC;
+}
+
 void
 nsXPConnect::Collect()
 {
@@ -385,6 +392,8 @@ nsXPConnect::Collect()
     // To improve debugging, if DEBUG_CC is defined all JS objects are
     // traversed.
 
+    mNeedGCBeforeCC = JS_FALSE;
+
     XPCCallContext ccx(NATIVE_CALLER);
     if(!ccx.IsValid())
         return;
@@ -574,9 +583,35 @@ xpc_GCThingIsGrayCCThing(void *thing)
     return ADD_TO_CC(kind) && xpc_IsGrayGCThing(thing);
 }
 
+/*
+ * The GC and CC are run independently. Consequently, the following sequence of
+ * events can occur:
+ * 1. GC runs and marks an object gray.
+ * 2. Some JS code runs that creates a pointer from a JS root to the gray
+ *    object. If we re-ran a GC at this point, the object would now be black.
+ * 3. Now we run the CC. It may think it can collect the gray object, even
+ *    though it's reachable from the JS heap.
+ *
+ * To prevent this badness, we unmark the gray bit of an object when it is
+ * accessed by callers outside XPConnect. This would cause the object to go
+ * black in step 2 above. This must be done on everything reachable from the
+ * object being returned. The following code takes care of the recursive
+ * re-coloring.
+ */
 static void
 UnmarkGrayChildren(JSTracer *trc, void *thing, uint32 kind)
 {
+    int stackDummy;
+    if (!JS_CHECK_STACK_SIZE(trc->context->stackLimit, &stackDummy)) {
+        /*
+         * If we run out of stack, we take a more drastic measure: require that
+         * we GC again before the next CC.
+         */
+        nsXPConnect* xpc = nsXPConnect::GetXPConnect();
+        xpc->EnsureGCBeforeCC();
+        return;
+    }
+
     // If this thing is not a CC-kind or already non-gray then we're done.
     if(!ADD_TO_CC(kind) || !xpc_IsGrayGCThing(thing))
         return;

js/src/xpconnect/src/xpcprivate.h

@@ -519,6 +519,8 @@ public:
 
     JSBool IsShuttingDown() const {return mShuttingDown;}
 
+    void EnsureGCBeforeCC() { mNeedGCBeforeCC = JS_TRUE; }
+
     nsresult GetInfoForIID(const nsIID * aIID, nsIInterfaceInfo** info);
     nsresult GetInfoForName(const char * name, nsIInterfaceInfo** info);
 
@@ -553,6 +555,7 @@ public:
     virtual nsresult FinishTraverse();
     virtual nsresult FinishCycleCollection();
     virtual nsCycleCollectionParticipant *ToParticipant(void *p);
+    virtual bool NeedCollect();
     virtual void Collect();
 #ifdef DEBUG_CC
     virtual void PrintAllReferencesTo(void *p);
@@ -602,6 +605,7 @@ private:
     nsIXPCSecurityManager*   mDefaultSecurityManager;
     PRUint16                 mDefaultSecurityManagerFlags;
     JSBool                   mShuttingDown;
+    JSBool                   mNeedGCBeforeCC;
 #ifdef DEBUG_CC
     PLDHashTable             mJSRoots;
 #endif

xpcom/base/nsCycleCollector.cpp

@@ -982,7 +982,6 @@ struct nsCycleCollector
     PRBool mScanInProgress;
     PRBool mFollowupCollection;
     PRUint32 mCollectedObjects;
-    PRBool mFirstCollection;
     TimeStamp mCollectionStart;
 
     nsCycleCollectionLanguageRuntime *mRuntimes[nsIProgrammingLanguage::MAX+1];
@@ -2147,7 +2146,6 @@ nsCycleCollector::nsCycleCollector() :
     mCollectionInProgress(PR_FALSE),
     mScanInProgress(PR_FALSE),
     mCollectedObjects(0),
-    mFirstCollection(PR_TRUE),
     mWhiteNodes(nsnull),
     mWhiteNodeCount(0),
 #ifdef DEBUG_CC
@@ -2554,23 +2552,22 @@ nsCycleCollector::BeginCollection(PRBool aForceGC,
 
     // The cycle collector uses the mark bitmap to discover what JS objects
     // were reachable only from XPConnect roots that might participate in
-    // cycles. If this is the first cycle collection after startup force
-    // a garbage collection, otherwise the GC might not have run yet and
-    // the bitmap is invalid.
-    if (mFirstCollection && mRuntimes[nsIProgrammingLanguage::JAVASCRIPT]) {
-        aForceGC = PR_TRUE;
-        mFirstCollection = PR_FALSE;
-    }
-
-    if (aForceGC && mRuntimes[nsIProgrammingLanguage::JAVASCRIPT]) {
+    // cycles. We ask the JS runtime whether we need to force a GC before
+    // this CC. It returns true on startup (before the mark bits have been set),
+    // and also when UnmarkGray has run out of stack.
+    if (mRuntimes[nsIProgrammingLanguage::JAVASCRIPT]) {
+        nsCycleCollectionJSRuntime* rt =
+            static_cast<nsCycleCollectionJSRuntime*>
+                (mRuntimes[nsIProgrammingLanguage::JAVASCRIPT]);
+        if (rt->NeedCollect() || aForceGC) {
 #ifdef COLLECT_TIME_DEBUG
-        PRTime start = PR_Now();
+            PRTime start = PR_Now();
 #endif
-        static_cast<nsCycleCollectionJSRuntime*>
-            (mRuntimes[nsIProgrammingLanguage::JAVASCRIPT])->Collect();
+            rt->Collect();
 #ifdef COLLECT_TIME_DEBUG
-        printf("cc: GC() took %lldms\n", (PR_Now() - start) / PR_USEC_PER_MSEC);
+            printf("cc: GC() took %lldms\n", (PR_Now() - start) / PR_USEC_PER_MSEC);
 #endif
+        }
     }
 
     if (aListener && NS_FAILED(aListener->Begin())) {

xpcom/base/nsCycleCollector.h

@@ -76,6 +76,11 @@ void nsCycleCollector_shutdown();
 // nsCycleCollector_doCollect directly.
 struct nsCycleCollectionJSRuntime : public nsCycleCollectionLanguageRuntime
 {
+    /**
+     * Should we force a JavaScript GC before a CC?
+     */
+    virtual bool NeedCollect() = 0;
+
     /**
      * Runs the JavaScript GC.
      */