Bug 790338 - Sweep debugger objects in the same group as their debugees r=billm

--HG--
extra : rebase_source : a97bb6aefa4291496a40b643887b4f712bc18119

js/src/jsgc.cpp

@@ -3677,6 +3677,8 @@ JSCompartment::findOutgoingEdges(ComponentFinder& finder)
         JS_ASSERT_IF(IsFunctionProxy(wrapper), &GetProxyCall(wrapper).toObject() == other);
 #endif
     }
+
+    Debugger::findCompartmentEdges(this, finder);
 }
 
 static void

js/src/jsweakmap.cpp

@@ -247,6 +247,8 @@ WeakMap_set_impl(JSContext *cx, CallArgs args)
         }
     }
 
+    JS_ASSERT(key->compartment() == thisObj->compartment());
+    JS_ASSERT_IF(value.isObject(), value.toObject().compartment() == thisObj->compartment());
     if (!map->put(key, value)) {
         JS_ReportOutOfMemory(cx);
         return false;

js/src/jsweakmap.h

@@ -184,21 +184,11 @@ class WeakMap : public HashMap<Key, Value, HashPolicy, RuntimeAllocPolicy>, publ
             if (gc::IsAboutToBeFinalized(&k))
                 e.removeFront();
         }
-
-#if DEBUG
         /*
          * Once we've swept, all remaining edges should stay within the
          * known-live part of the graph.
          */
-        for (Range r = Base::all(); !r.empty(); r.popFront()) {
-            Key k(r.front().key);
-            Value v(r.front().value);
-            JS_ASSERT(!gc::IsAboutToBeFinalized(&k));
-            JS_ASSERT(!gc::IsAboutToBeFinalized(&v));
-            JS_ASSERT(k == r.front().key);
-            JS_ASSERT(v == r.front().value);
-        }
-#endif
+        assertEntriesNotAboutToBeFinalized();
     }
 
     /* memberOf can be NULL, which means that the map is not part of a JSObject. */
@@ -213,6 +203,20 @@ class WeakMap : public HashMap<Key, Value, HashPolicy, RuntimeAllocPolicy>, publ
             }
         }
     }
+
+protected:
+    void assertEntriesNotAboutToBeFinalized() {
+#if DEBUG
+        for (Range r = Base::all(); !r.empty(); r.popFront()) {
+            Key k(r.front().key);
+            Value v(r.front().value);
+            JS_ASSERT(!gc::IsAboutToBeFinalized(&k));
+            JS_ASSERT(!gc::IsAboutToBeFinalized(&v));
+            JS_ASSERT(k == r.front().key);
+            JS_ASSERT(v == r.front().value);
+        }
+#endif
+    }
 };
 
 } /* namespace js */

js/src/vm/Debugger.cpp

@@ -1355,29 +1355,9 @@ Debugger::markKeysInCompartment(JSTracer *tracer)
      * enumerating WeakMap keys. However in this case we need access, so we
      * make a base-class reference. Range is public in HashMap.
      */
-    ObjectWeakMap::Base &objStorage = objects;
-    for (ObjectWeakMap::Base::Range r = objStorage.all(); !r.empty(); r.popFront()) {
-        const EncapsulatedPtrObject key = r.front().key;
-        HeapPtrObject tmp(key);
-        gc::MarkObject(tracer, &tmp, "cross-compartment WeakMap key");
-        JS_ASSERT(tmp == key);
-    }
-
-    ObjectWeakMap::Base &envStorage = environments;
-    for (ObjectWeakMap::Base::Range r = envStorage.all(); !r.empty(); r.popFront()) {
-        const EncapsulatedPtrObject &key = r.front().key;
-        HeapPtrObject tmp(key);
-        js::gc::MarkObject(tracer, &tmp, "cross-compartment WeakMap key");
-        JS_ASSERT(tmp == key);
-    }
-
-    const ScriptWeakMap::Base &scriptStorage = scripts;
-    for (ScriptWeakMap::Base::Range r = scriptStorage.all(); !r.empty(); r.popFront()) {
-        const EncapsulatedPtrScript &key = r.front().key;
-        HeapPtrScript tmp(key);
-        gc::MarkScript(tracer, &tmp, "cross-compartment WeakMap key");
-        JS_ASSERT(tmp == key);
-    }
+    objects.markKeys(tracer);
+    environments.markKeys(tracer);
+    scripts.markKeys(tracer);
 }
 
 /*
@@ -1576,6 +1556,24 @@ Debugger::detachAllDebuggersFromGlobal(FreeOp *fop, GlobalObject *global,
         debuggers->back()->removeDebuggeeGlobal(fop, global, compartmentEnum, NULL);
 }
 
+/* static */ void
+Debugger::findCompartmentEdges(JSCompartment *comp, js::gc::ComponentFinder &finder)
+{
+    /*
+     * For debugger cross compartment wrappers, add edges in the opposite
+     * direction to those already added by JSCompartment::findOutgoingEdges.
+     * This ensure that debuggers and their debuggees are finalized in the same
+     * group.
+     */
+    JSRuntime *rt = comp->rt;
+    for (JSCList *p = &rt->debuggerList; (p = JS_NEXT_LINK(p)) != &rt->debuggerList;) {
+        Debugger *dbg = Debugger::fromLinks(p);
+        dbg->scripts.findCompartmentEdges(comp, finder);
+        dbg->objects.findCompartmentEdges(comp, finder);
+        dbg->environments.findCompartmentEdges(comp, finder);
+    }
+}
+
 void
 Debugger::finalize(FreeOp *fop, RawObject obj)
 {

js/src/vm/Debugger.h

@@ -19,11 +19,145 @@
 #include "jswrapper.h"
 
 #include "gc/Barrier.h"
+#include "gc/FindSCCs.h"
 #include "js/HashTable.h"
 #include "vm/GlobalObject.h"
 
 namespace js {
 
+/*
+ * A weakmap that supports the keys being in different compartments to the
+ * values, although all values must be in the same compartment.
+ *
+ * The Key and Value classes must support the compartment() method.
+ *
+ * The purpose of this is to allow the garbage collector to easily find edges
+ * from debugee object compartments to debugger compartments when calculating
+ * the compartment groups.  Note that these edges are the inverse of the edges
+ * stored in the cross compartment map.
+ *
+ * The current implementation results in all debuggee object compartments being
+ * swept in the same group as the debugger.  This is a conservative approach,
+ * and compartments may be unnecessarily grouped, however it results in a
+ * simpler and faster implementation.
+ */
+template <class Key, class Value>
+class DebuggerWeakMap : private WeakMap<Key, Value, DefaultHasher<Key> >
+{
+  private:
+    typedef HashMap<JSCompartment *,
+                    uintptr_t,
+                    DefaultHasher<JSCompartment *>,
+                    RuntimeAllocPolicy> CountMap;
+
+    JSCompartment *valueCompartment;
+    CountMap compartmentCounts;
+
+  public:
+    typedef WeakMap<Key, Value, DefaultHasher<Key> > Base;
+    explicit DebuggerWeakMap(JSRuntime *rt)
+        : Base(rt), valueCompartment(NULL), compartmentCounts(rt) { }
+    explicit DebuggerWeakMap(JSContext *cx)
+        : Base(cx), valueCompartment(NULL), compartmentCounts(cx) { }
+
+  public:
+    /* Expose those parts of HashMap public interface that are used by Debugger methods. */
+
+    typedef typename Base::Ptr Ptr;
+    typedef typename Base::AddPtr AddPtr;
+    typedef typename Base::Range Range;
+    typedef typename Base::Enum Enum;
+    typedef typename Base::Lookup Lookup;
+
+    bool init(uint32_t len = 16) {
+        return Base::init(len) && compartmentCounts.init();
+    }
+
+    AddPtr lookupForAdd(const Lookup &l) const {
+        return Base::lookupForAdd(l);
+    }
+
+    template<typename KeyInput, typename ValueInput>
+    bool relookupOrAdd(AddPtr &p, const KeyInput &k, const ValueInput &v) {
+        if (!valueCompartment)
+            valueCompartment = v->compartment();
+        JS_ASSERT(v->compartment() == valueCompartment);
+        if (!incCompartmentCount(k->compartment()))
+            return false;
+        bool ok = Base::relookupOrAdd(p, k, v);
+        if (!ok)
+            decCompartmentCount(k->compartment());
+        return ok;
+    }
+
+    Range all() const {
+        return Base::all();
+    }
+
+    void remove(const Lookup &l) {
+        Base::remove(l);
+        decCompartmentCount(l->compartment());
+        if (Base::count() == 0)
+            valueCompartment = NULL;
+    }
+
+  public:
+    /* Expose WeakMap public interface*/
+    void trace(JSTracer *tracer) {
+        Base::trace(tracer);
+    }
+
+  public:
+    void markKeys(JSTracer *tracer) {
+        for (Range r = all(); !r.empty(); r.popFront()) {
+            Key key = r.front().key;
+            gc::Mark(tracer, &key, "cross-compartment WeakMap key");
+            JS_ASSERT(key == r.front().key);
+        }
+    }
+
+    void findCompartmentEdges(JSCompartment *v, js::gc::ComponentFinder &finder) {
+        if (!valueCompartment || valueCompartment == v || !valueCompartment->isGCMarking())
+            return;
+        CountMap::Ptr p = compartmentCounts.lookup(v);
+        if (!p)
+            return;
+        JS_ASSERT(p->value > 0);
+        finder.addEdgeTo(valueCompartment);
+    }
+
+  private:
+    /* Override sweep method to also update our edge cache. */
+    void sweep(JSTracer *trc) {
+        for (Enum e(*static_cast<Base *>(this)); !e.empty(); e.popFront()) {
+            Key k(e.front().key);
+            Value v(e.front().value);
+            if (gc::IsAboutToBeFinalized(&k)) {
+                e.removeFront();
+                decCompartmentCount(k->compartment());
+            }
+        }
+        Base::assertEntriesNotAboutToBeFinalized();
+    }
+
+    bool incCompartmentCount(JSCompartment *c) {
+        CountMap::Ptr p = compartmentCounts.lookupWithDefault(c, 0);
+        if (!p)
+            return false;
+        ++p->value;
+        return true;
+    }
+
+    void decCompartmentCount(JSCompartment *c) {
+        CountMap::Ptr p = compartmentCounts.lookup(c);
+        JS_ASSERT(p);
+        JS_ASSERT(p->value > 0);
+        --p->value;
+        if (p->value == 0)
+            compartmentCounts.remove(c);
+    }
+};
+
 class Debugger {
     friend class Breakpoint;
     friend JSBool (::JS_DefineDebuggerObject)(JSContext *cx, JSObject *obj);
@@ -86,11 +220,11 @@ class Debugger {
     FrameMap frames;
 
     /* An ephemeral map from JSScript* to Debugger.Script instances. */
-    typedef WeakMap<EncapsulatedPtrScript, RelocatablePtrObject> ScriptWeakMap;
+    typedef DebuggerWeakMap<EncapsulatedPtrScript, RelocatablePtrObject> ScriptWeakMap;
     ScriptWeakMap scripts;
 
     /* The map from debuggee objects to their Debugger.Object instances. */
-    typedef WeakMap<EncapsulatedPtrObject, RelocatablePtrObject> ObjectWeakMap;
+    typedef DebuggerWeakMap<EncapsulatedPtrObject, RelocatablePtrObject> ObjectWeakMap;
     ObjectWeakMap objects;
 
     /* The map from debuggee Envs to Debugger.Environment instances. */
@@ -253,6 +387,7 @@ class Debugger {
     static void detachAllDebuggersFromGlobal(FreeOp *fop, GlobalObject *global,
                                              GlobalObjectSet::Enum *compartmentEnum);
     static unsigned gcGrayLinkSlot();
+    static void findCompartmentEdges(JSCompartment *v, js::gc::ComponentFinder &finder);
 
     static inline JSTrapStatus onEnterFrame(JSContext *cx, Value *vp);
     static inline bool onLeaveFrame(JSContext *cx, bool ok);