Bug 826650 - GC: Recalculation of GC fullness in EndSweepPhase broken…

… r=billm

js/src/builtin/TestingFunctions.cpp

@@ -469,7 +469,10 @@ GCState(JSContext *cx, unsigned argc, jsval *vp)
     else
         JS_NOT_REACHED("Unobserveable global GC state");
 
-    *vp = StringValue(js_NewStringCopyZ(cx, state));
+    JSString *str = JS_NewStringCopyZ(cx, state);
+    if (!str)
+        return false;
+    *vp = StringValue(str);
     return true;
 }
 

js/src/jsapi-tests/Makefile.in

@@ -39,6 +39,7 @@ CPPSRCS = \
   testFindSCCs.cpp \
   testFuncCallback.cpp \
   testFunctionProperties.cpp \
+  testGCFinalizeCallback.cpp \
   testGCOutOfMemory.cpp \
   testGetPropertyDefault.cpp \
   testHashTable.cpp \

js/src/jsapi-tests/testGCFinalizeCallback.cpp

@@ -0,0 +1,170 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "tests.h"
+
+#include <stdarg.h>
+
+#include "jsfriendapi.h"
+#include "jscntxt.h"
+
+const unsigned BufferSize = 20;
+static unsigned FinalizeCalls = 0;
+static JSFinalizeStatus StatusBuffer[BufferSize];
+static bool IsCompartmentGCBuffer[BufferSize];
+
+static void
+FinalizeCallback(JSFreeOp *fop, JSFinalizeStatus status, JSBool isCompartmentGC)
+{
+    if (FinalizeCalls < BufferSize) {
+        StatusBuffer[FinalizeCalls] = status;
+        IsCompartmentGCBuffer[FinalizeCalls] = isCompartmentGC;
+    }
+    ++FinalizeCalls;
+}
+
+BEGIN_TEST(testGCFinalizeCallback)
+{
+    JS_SetGCParameter(rt, JSGC_MODE, JSGC_MODE_INCREMENTAL);
+    JS_SetFinalizeCallback(rt, FinalizeCallback);
+
+    /* Full GC, non-incremental. */
+    FinalizeCalls = 0;
+    JS_GC(rt);
+    CHECK(rt->gcIsFull);
+    CHECK(checkSingleGroup());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(false));
+
+    /* Full GC, incremental. */
+    FinalizeCalls = 0;
+    js::PrepareForFullGC(rt);
+    js::IncrementalGC(rt, js::gcreason::API, 1000000);
+    CHECK(rt->gcIncrementalState == js::gc::NO_INCREMENTAL);
+    CHECK(rt->gcIsFull);
+    CHECK(checkMultipleGroups());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(false));
+
+    js::AutoObjectRooter global1(cx, createGlobal());
+    js::AutoObjectRooter global2(cx, createGlobal());
+    js::AutoObjectRooter global3(cx, createGlobal());
+    CHECK(global1.object());
+    CHECK(global2.object());
+    CHECK(global3.object());
+
+    /* Compartment GC, non-incremental, single compartment. */
+    FinalizeCalls = 0;
+    js::PrepareCompartmentForGC(global1.object()->compartment());
+    js::GCForReason(rt, js::gcreason::API);
+    CHECK(!rt->gcIsFull);
+    CHECK(checkSingleGroup());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(true));
+
+    /* Compartment GC, non-incremental, multiple compartments. */
+    FinalizeCalls = 0;
+    js::PrepareCompartmentForGC(global1.object()->compartment());
+    js::PrepareCompartmentForGC(global2.object()->compartment());
+    js::PrepareCompartmentForGC(global3.object()->compartment());
+    js::GCForReason(rt, js::gcreason::API);
+    CHECK(!rt->gcIsFull);
+    CHECK(checkSingleGroup());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(true));
+
+    /* Compartment GC, incremental, single compartment. */
+    FinalizeCalls = 0;
+    js::PrepareCompartmentForGC(global1.object()->compartment());
+    js::IncrementalGC(rt, js::gcreason::API, 1000000);
+    CHECK(rt->gcIncrementalState == js::gc::NO_INCREMENTAL);
+    CHECK(!rt->gcIsFull);
+    CHECK(checkSingleGroup());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(true));
+
+    /* Compartment GC, incremental, multiple compartments. */
+    FinalizeCalls = 0;
+    js::PrepareCompartmentForGC(global1.object()->compartment());
+    js::PrepareCompartmentForGC(global2.object()->compartment());
+    js::PrepareCompartmentForGC(global3.object()->compartment());
+    js::IncrementalGC(rt, js::gcreason::API, 1000000);
+    CHECK(rt->gcIncrementalState == js::gc::NO_INCREMENTAL);
+    CHECK(!rt->gcIsFull);
+    CHECK(checkMultipleGroups());
+    CHECK(checkFinalizeStatus());
+    CHECK(checkFinalizeIsCompartmentGC(true));
+
+#ifdef JS_GC_ZEAL
+
+    /* Full GC with reset due to new compartment, becoming compartment GC. */
+
+    FinalizeCalls = 0;
+    JS_SetGCZeal(cx, 9, 1000000);
+    js::PrepareForFullGC(rt);
+    js::GCDebugSlice(rt, true, 1);
+    CHECK(rt->gcIncrementalState == js::gc::MARK);
+    CHECK(rt->gcIsFull);
+
+    js::AutoObjectRooter global4(cx, createGlobal());
+    js::GCDebugSlice(rt, true, 1);
+    CHECK(rt->gcIncrementalState == js::gc::NO_INCREMENTAL);
+    CHECK(!rt->gcIsFull);
+    CHECK(checkMultipleGroups());
+    CHECK(checkFinalizeStatus());
+
+    for (unsigned i = 0; i < FinalizeCalls - 1; ++i)
+        CHECK(!IsCompartmentGCBuffer[i]);
+    CHECK(IsCompartmentGCBuffer[FinalizeCalls - 1]);
+
+    JS_SetGCZeal(cx, 0, 0);
+
+#endif
+
+    JS_SetFinalizeCallback(rt, NULL);
+    return true;
+}
+
+bool checkSingleGroup()
+{
+    CHECK(FinalizeCalls < BufferSize);
+    CHECK(FinalizeCalls == 3);
+    return true;
+}
+
+bool checkMultipleGroups()
+{
+    CHECK(FinalizeCalls < BufferSize);
+    CHECK(FinalizeCalls % 2 == 1);
+    CHECK((FinalizeCalls - 1) / 2 > 1);
+    return true;
+}
+
+bool checkFinalizeStatus()
+{
+    /*
+     * The finalize callback should be called twice for each compartment group
+     * finalized, with status JSFINALIZE_GROUP_START and JSFINALIZE_GROUP_END,
+     * and then once more with JSFINALIZE_COLLECTION_END.
+     */
+
+    for (unsigned i = 0; i < FinalizeCalls - 1; i += 2) {
+        CHECK(StatusBuffer[i] == JSFINALIZE_GROUP_START);
+        CHECK(StatusBuffer[i + 1] == JSFINALIZE_GROUP_END);
+    }
+
+    CHECK(StatusBuffer[FinalizeCalls - 1] == JSFINALIZE_COLLECTION_END);
+
+    return true;
+}
+
+bool checkFinalizeIsCompartmentGC(bool isCompartmentGC)
+{
+    for (unsigned i = 0; i < FinalizeCalls; ++i)
+        CHECK(IsCompartmentGCBuffer[i] == isCompartmentGC);
+
+    return true;
+}
+
+END_TEST(testGCFinalizeCallback)

js/src/jsgc.cpp

@@ -3595,6 +3595,19 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC)
     JS_ASSERT(rt->gcMarker.isDrained());
     rt->gcMarker.stop();
 
+    /*
+     * Recalculate whether GC was full or not as this may have changed due to
+     * newly created compartments.  Can only change from full to not full.
+     */
+    if (rt->gcIsFull) {
+        for (CompartmentsIter c(rt); !c.done(); c.next()) {
+            if (!c->isCollecting()) {
+                rt->gcIsFull = false;
+                break;
+            }
+        }
+    }
+
     /*
      * If we found any black->gray edges during marking, we completely clear the
      * mark bits of all uncollected compartments. This is safe, although it may
@@ -3649,19 +3662,11 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC)
     {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_FINALIZE_END);
 
-        bool isFull = true;
-        for (CompartmentsIter c(rt); !c.done(); c.next()) {
-            if (!c->isCollecting()) {
-                rt->gcIsFull = false;
-                break;
-            }
-        }
-
         if (rt->gcFinalizeCallback)
-            rt->gcFinalizeCallback(&fop, JSFINALIZE_COLLECTION_END, !isFull);
+            rt->gcFinalizeCallback(&fop, JSFINALIZE_COLLECTION_END, !rt->gcIsFull);
 
         /* If we finished a full GC, then the gray bits are correct. */
-        if (isFull)
+        if (rt->gcIsFull)
             rt->gcGrayBitsValid = true;
     }
 
@@ -4323,25 +4328,36 @@ js::GCFinalSlice(JSRuntime *rt, JSGCInvocationKind gckind, gcreason::Reason reas
     Collect(rt, true, SliceBudget::Unlimited, gckind, reason);
 }
 
+static bool
+CompartmentsSelected(JSRuntime *rt)
+{
+    for (CompartmentsIter c(rt); !c.done(); c.next()) {
+        if (c->isGCScheduled())
+            return true;
+    }
+    return false;
+}
+
 void
 js::GCDebugSlice(JSRuntime *rt, bool limit, int64_t objCount)
 {
     AssertCanGC();
     int64_t budget = limit ? SliceBudget::WorkBudget(objCount) : SliceBudget::Unlimited;
-    PrepareForDebugGC(rt);
+    if (!CompartmentsSelected(rt)) {
+        if (IsIncrementalGCInProgress(rt))
+            PrepareForIncrementalGC(rt);
+        else
+            PrepareForFullGC(rt);
+    }
     Collect(rt, true, budget, GC_NORMAL, gcreason::DEBUG_GC);
 }
 
 /* Schedule a full GC unless a compartment will already be collected. */
 void
 js::PrepareForDebugGC(JSRuntime *rt)
 {
-    for (CompartmentsIter c(rt); !c.done(); c.next()) {
-        if (c->isGCScheduled())
-            return;
-    }
-
-    PrepareForFullGC(rt);
+    if (!CompartmentsSelected(rt))
+        PrepareForFullGC(rt);
 }
 
 void