Bug 1180536 - Use mozilla::ScopeExit to clean up Debugger::addDebuggeeGlobal's consistency on failure; r=sfink

js/src/vm/Debugger.cpp

@@ -7,6 +7,7 @@
 #include "vm/Debugger-inl.h"
 
 #include "mozilla/DebugOnly.h"
+#include "mozilla/ScopeExit.h"
 
 #include "jscntxt.h"
 #include "jscompartment.h"
@@ -46,6 +47,7 @@ using JS::dbg::Builder;
 using js::frontend::IsIdentifier;
 using mozilla::ArrayLength;
 using mozilla::DebugOnly;
+using mozilla::MakeScopeExit;
 using mozilla::Maybe;
 using mozilla::UniquePtr;
 
@@ -3298,75 +3300,86 @@ Debugger::addDebuggeeGlobal(JSContext* cx, Handle<GlobalObject*> global)
     /*
      * For global to become this js::Debugger's debuggee:
      *
-     * 1. global must be in this->debuggees,
-     * 2. this js::Debugger must be in global->getDebuggers(),
+     * 1. this js::Debugger must be in global->getDebuggers(),
+     * 2. global must be in this->debuggees,
      * 3. it must be in zone->getDebuggers(),
-     * 4. JSCompartment::isDebuggee()'s bit must be set, and
+     * 4. the debuggee's zone must be in this->debuggeeZones,
      * 5. if we are tracking allocations, the SavedStacksMetadataCallback must be
-     *    installed for this compartment.
+     *    installed for this compartment, and
+     * 6. JSCompartment::isDebuggee()'s bit must be set.
      *
-     * All five indications must be kept consistent.
+     * All six indications must be kept consistent.
      */
 
-    bool inDebuggees = false;
-    bool inGlobalDebuggers = false;
-    bool inZoneDebuggers = false;
-    bool inDebuggeeZones = false;
-
     AutoCompartment ac(cx, global);
     Zone* zone = global->zone();
 
+    // (1)
     auto* globalDebuggers = GlobalObject::getOrCreateDebuggers(cx, global);
     if (!globalDebuggers)
-        goto error;
-    if (!globalDebuggers->append(this))
-        goto oom;
-    inGlobalDebuggers = true;
-
-    if (!debuggees.put(global))
-        goto oom;
-    inDebuggees = true;
-
-    Zone::DebuggerVector* zoneDebuggers;
-    if (!debuggeeZones.has(zone)) {
-        zoneDebuggers = zone->getOrCreateDebuggers(cx);
-        if (!zoneDebuggers)
-            goto error;
-        if (!zoneDebuggers->append(this))
-            goto oom;
-        inZoneDebuggers = true;
-
-        if (!debuggeeZones.put(zone))
-            goto oom;
-        inDebuggeeZones = true;
+        return false;
+    if (!globalDebuggers->append(this)) {
+        ReportOutOfMemory(cx);
+        return false;
     }
+    auto globalDebuggersGuard = MakeScopeExit([&] {
+        globalDebuggers->popBack();
+    });
 
+    // (2)
+    if (!debuggees.put(global)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+    auto debuggeesGuard = MakeScopeExit([&] {
+        debuggees.remove(global);
+    });
+
+    bool addingZoneRelation = !debuggeeZones.has(zone);
+
+    // (3)
+    auto* zoneDebuggers = zone->getOrCreateDebuggers(cx);
+    if (!zoneDebuggers)
+        return false;
+    if (addingZoneRelation && !zoneDebuggers->append(this)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+    auto zoneDebuggersGuard = MakeScopeExit([&] {
+        if (addingZoneRelation)
+            zoneDebuggers->popBack();
+    });
+
+    // (4)
+    if (addingZoneRelation && !debuggeeZones.put(zone)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+    auto debuggeeZonesGuard = MakeScopeExit([&] {
+        if (addingZoneRelation)
+            debuggeeZones.remove(zone);
+    });
+
+    // (5)
     if (trackingAllocationSites && !Debugger::addAllocationsTracking(cx, *global))
-        goto error;
+            return false;
+    auto allocationsTrackingGuard = MakeScopeExit([&] {
+        if (trackingAllocationSites)
+            Debugger::removeAllocationsTracking(*global);
+    });
 
+    // (6)
     debuggeeCompartment->setIsDebuggee();
     debuggeeCompartment->updateDebuggerObservesAsmJS();
-
     if (observesAllExecution() && !ensureExecutionObservabilityOfCompartment(cx, debuggeeCompartment))
-        goto error;
+        return false;
 
+    globalDebuggersGuard.release();
+    debuggeesGuard.release();
+    zoneDebuggersGuard.release();
+    debuggeeZonesGuard.release();
+    allocationsTrackingGuard.release();
     return true;
-
-  oom:
-    ReportOutOfMemory(cx);
-    // Fall through...
-
-  error:
-    // Maintain consistency on error.
-    if (inGlobalDebuggers)
-        globalDebuggers->popBack();
-    if (inDebuggees)
-        debuggees.remove(global);
-    if (inZoneDebuggers)
-        zoneDebuggers->popBack();
-    if (inDebuggeeZones)
-        debuggeeZones.remove(zone);
-    return false;
 }
 
 bool