Bug 1119579 - Don't GC while iterating compartments in findAllGlobals. r=sfink, a=abillings
authorShu-yu Guo <shu@rfrn.org>
Mon, 26 Jan 2015 18:26:25 -0800
changeset 200543 30322563cab386f676358dfbcc3bcc99082fd58c
parent 200542 50daa62a5cb95a9d98aec1a1713b5324bc4c92d3
child 201063 67231f96efac807b318cadc49fb1024c749e5bc3
push id181
push userryanvm@gmail.com
push dateWed, 28 Jan 2015 14:31:41 +0000
treeherdermozilla-esr31@30322563cab3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, abillings
bugs1119579
milestone31.4.0
Bug 1119579 - Don't GC while iterating compartments in findAllGlobals. r=sfink, a=abillings
js/src/vm/Debugger.cpp
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2820,45 +2820,57 @@ Debugger::findScripts(JSContext *cx, uns
     return true;
 }
 
 bool
 Debugger::findAllGlobals(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGGER(cx, argc, vp, "findAllGlobals", args, dbg);
 
+    AutoObjectVector globals(cx);
+
+    {
+        // Accumulate the list of globals before wrapping them, because
+        // wrapping can GC and collect compartments from under us, while
+        // iterating.
+
+        for (CompartmentsIter c(cx->runtime(), SkipAtoms); !c.done(); c.next()) {
+            if (c->options().invisibleToDebugger())
+                continue;
+
+            c->zone()->scheduledForDestruction = false;
+
+            GlobalObject *global = c->maybeGlobal();
+
+            if (cx->runtime()->isSelfHostingGlobal(global))
+                continue;
+
+            if (global) {
+                /*
+                 * We pulled |global| out of nowhere, so it's possible that it was
+                 * marked gray by XPConnect. Since we're now exposing it to JS code,
+                 * we need to mark it black.
+                 */
+                JS::ExposeGCThingToActiveJS(global, JSTRACE_OBJECT);
+                if (!globals.append(global))
+                    return false;
+            }
+        }
+    }
+
     RootedObject result(cx, NewDenseEmptyArray(cx));
     if (!result)
         return false;
 
-    for (CompartmentsIter c(cx->runtime(), SkipAtoms); !c.done(); c.next()) {
-        if (c->options().invisibleToDebugger())
-            continue;
-
-        c->zone()->scheduledForDestruction = false;
-
-        GlobalObject *global = c->maybeGlobal();
-
-        if (cx->runtime()->isSelfHostingGlobal(global))
-            continue;
-
-        if (global) {
-            /*
-             * We pulled |global| out of nowhere, so it's possible that it was
-             * marked gray by XPConnect. Since we're now exposing it to JS code,
-             * we need to mark it black.
-             */
-            JS::ExposeGCThingToActiveJS(global, JSTRACE_OBJECT);
-
-            RootedValue globalValue(cx, ObjectValue(*global));
-            if (!dbg->wrapDebuggeeValue(cx, &globalValue))
-                return false;
-            if (!NewbornArrayPush(cx, result, globalValue))
-                return false;
-        }
+    for (size_t i = 0; i < globals.length(); i++) {
+        RootedValue globalValue(cx, ObjectValue(*globals[i]));
+        if (!dbg->wrapDebuggeeValue(cx, &globalValue))
+            return false;
+        if (!NewbornArrayPush(cx, result, globalValue))
+            return false;
     }
 
     args.rval().setObject(*result);
     return true;
 }
 
 bool
 Debugger::makeGlobalObjectReference(JSContext *cx, unsigned argc, Value *vp)