Bug 890048 - Fix rooting of the findReferences shell command; r=jimb,billm

js/public/GCAPI.h

@@ -181,6 +181,9 @@ DisableIncrementalGC(JSRuntime *rt);
 extern JS_FRIEND_API(void)
 DisableGenerationalGC(JSRuntime *rt);
 
+extern JS_FRIEND_API(void)
+EnableGenerationalGC(JSRuntime *rt);
+
 extern JS_FRIEND_API(bool)
 IsIncrementalBarrierNeeded(JSRuntime *rt);
 

js/src/jit-test/tests/gc/bug-890048.js

@@ -0,0 +1,3 @@
+if (typeof findReferences == 'function') {
+    findReferences([schedulegc(1)])
+}

js/src/jsfriendapi.cpp

@@ -879,11 +879,22 @@ JS::DisableGenerationalGC(JSRuntime *rt)
 {
     rt->gcGenerationalEnabled = false;
 #ifdef JSGC_GENERATIONAL
+    MinorGC(rt, JS::gcreason::API);
     rt->gcNursery.disable();
     rt->gcStoreBuffer.disable();
 #endif
 }
 
+extern JS_FRIEND_API(void)
+JS::EnableGenerationalGC(JSRuntime *rt)
+{
+    rt->gcGenerationalEnabled = true;
+#ifdef JSGC_GENERATIONAL
+    rt->gcNursery.enable();
+    rt->gcStoreBuffer.enable();
+#endif
+}
+
 JS_FRIEND_API(bool)
 JS::IsIncrementalBarrierNeeded(JSRuntime *rt)
 {

js/src/shell/jsheaptools.cpp

@@ -59,7 +59,8 @@ using mozilla::MoveRef;
  * they are destroyed. So you don't need to worry about nodes going away while
  * you're using them.
  */
-class HeapReverser : public JSTracer {
+class HeapReverser : public JSTracer, public JS::CustomAutoRooter
+{
   public:
     struct Edge;
 
@@ -83,6 +84,11 @@ class HeapReverser : public JSTracer {
             return *this;
         }
 
+        void trace(JSTracer *trc) {
+            for (Edge *e = incoming.begin(); e != incoming.end(); e++)
+                e->trace(trc);
+        }
+
         /* What kind of Cell this is. */
         JSGCTraceKind kind;
 
@@ -97,8 +103,8 @@ class HeapReverser : public JSTracer {
         bool marked;
 
       private:
-        Node(const Node &);
-        Node &operator=(const Node &);
+        Node(const Node &) MOZ_DELETE;
+        Node &operator=(const Node &) MOZ_DELETE;
     };
 
     /* Metadata for a heap edge we have traversed. */
@@ -121,6 +127,11 @@ class HeapReverser : public JSTracer {
             return *this;
         }
 
+        void trace(JSTracer *trc) {
+            if (origin)
+                gc::MarkGCThingRoot(trc, &origin, "HeapReverser::Edge");
+        }
+
         /* The name of this heap edge. Owned by this Edge. */
         char *name;
 
@@ -142,8 +153,17 @@ class HeapReverser : public JSTracer {
     Map map;
 
     /* Construct a HeapReverser for |context|'s heap. */
-    HeapReverser(JSContext *cx) : rooter(cx, 0, NULL), parent(NULL) {
-        JS_TracerInit(this, JS_GetRuntime(cx), traverseEdgeWithThis);
+    HeapReverser(JSContext *cx)
+      : JS::CustomAutoRooter(cx),
+        runtime(JS_GetRuntime(cx)),
+        parent(NULL)
+    {
+        JS_TracerInit(this, runtime, traverseEdgeWithThis);
+        JS::DisableGenerationalGC(runtime);
+    }
+
+    ~HeapReverser() {
+        JS::EnableGenerationalGC(runtime);
     }
 
     bool init() { return map.init(); }
@@ -152,27 +172,8 @@ class HeapReverser : public JSTracer {
     bool reverseHeap();
 
   private:
-    /*
-     * Once we've produced a reversed map of the heap, we need to keep the
-     * engine from freeing the objects we've found in it, until we're done using
-     * the map. Even if we're only using the map to construct a result object,
-     * and not rearranging the heap ourselves, any allocation could cause a
-     * garbage collection, which could free objects held internally by the
-     * engine (for example, JaegerMonkey object templates, used by jit scripts).
-     *
-     * So, each time reverseHeap reaches any object, we add it to 'roots', which
-     * is cited by 'rooter', so the object will stay alive long enough for us to
-     * include it in the results, if needed.
-     *
-     * Note that AutoArrayRooters must be constructed and destroyed in a
-     * stack-like order, so the same rule applies to this HeapReverser. The
-     * easiest way to satisfy this requirement is to only allocate HeapReversers
-     * as local variables in functions, or in types that themselves follow that
-     * rule. This is kind of dumb, but JSAPI doesn't provide any less restricted
-     * way to register arrays of roots.
-     */
-    Vector<jsval, 0, SystemAllocPolicy> roots;
-    AutoArrayRooter rooter;
+    /* A runtime pointer for use by the destructor. */
+    JSRuntime *runtime;
 
     /*
      * Return the name of the most recent edge this JSTracer has traversed. The
@@ -240,18 +241,23 @@ class HeapReverser : public JSTracer {
         JSObject *object = static_cast<JSObject *>(cell);
         return OBJECT_TO_JSVAL(object);
     }
+
+    /* Keep all tracked objects live across GC. */
+    virtual void trace(JSTracer *trc) MOZ_OVERRIDE {
+        if (!map.initialized())
+            return;
+        for (Map::Enum e(map); !e.empty(); e.popFront()) {
+            gc::MarkGCThingRoot(trc, const_cast<void **>(&e.front().key), "HeapReverser::map::key");
+            e.front().value.trace(trc);
+        }
+        for (Child *c = work.begin(); c != work.end(); ++c)
+            gc::MarkGCThingRoot(trc, &c->cell, "HeapReverser::Child");
+    }
 };
 
 bool
 HeapReverser::traverseEdge(void *cell, JSGCTraceKind kind)
 {
-    jsval v = nodeToValue(cell, kind);
-    if (v.isObject()) {
-        if (!roots.append(v))
-            return false;
-        rooter.changeArray(roots.begin(), roots.length());
-    }
-
     /* Capture this edge before the JSTracer members get overwritten. */
     char *edgeDescription = getEdgeDescription();
     if (!edgeDescription)