Bug 758278 - Sweep crossCompartmentWrappers of all compartments, not only GCed ones. r=billm
authorTill Schneidereit <tschneidereit@gmail.com>
Thu, 24 May 2012 19:04:44 +0200
changeset 95065 4c3f2ddd82e8c35cbababbbb2004571aea077be4
parent 95064 602c1435026d796555e6307116030a8f8c18ff3b
child 95066 4dd2d5f25910f940557cd728ef645a05afb0f116
child 95086 af889781505c6201ec46d0f8a5df26ce8aba76ea
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbillm
bugs758278
milestone15.0a1
Bug 758278 - Sweep crossCompartmentWrappers of all compartments, not only GCed ones. r=billm
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsgc.cpp
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -367,28 +367,16 @@ JSCompartment::markCrossCompartmentWrapp
             MarkValueRoot(trc, &referent, "cross-compartment wrapper");
             JS_ASSERT(referent == GetProxyPrivate(wrapper));
 
             if (IsFunctionProxy(wrapper)) {
                 Value call = GetProxyCall(wrapper);
                 MarkValueRoot(trc, &call, "cross-compartment wrapper");
                 JS_ASSERT(call == GetProxyCall(wrapper));
             }
-        } else {
-            /*
-             * Strings don't have a private pointer to mark, so we use the
-             * wrapper map key. (This does not work for wrappers because, in the
-             * case of Location objects, the wrapper map key is not the same as
-             * the proxy private slot. If we only marked the wrapper map key, we
-             * would miss same-compartment wrappers for Location objects.)
-             */
-            JS_ASSERT(v.isString());
-            Value v = e.front().key;
-            MarkValueRoot(trc, &v, "cross-compartment wrapper");
-            JS_ASSERT(v == e.front().key);
         }
     }
 }
 
 void
 JSCompartment::markTypes(JSTracer *trc)
 {
     /*
@@ -458,27 +446,17 @@ JSCompartment::discardJitCode(FreeOp *fo
     }
 
 #endif /* JS_METHODJIT */
 }
 
 void
 JSCompartment::sweep(FreeOp *fop, bool releaseTypes)
 {
-    /* Remove dead wrappers from the table. */
-    for (WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
-        Value key = e.front().key;
-        bool keyMarked = IsValueMarked(&key);
-        bool valMarked = IsValueMarked(e.front().value.unsafeGet());
-        JS_ASSERT_IF(!keyMarked && valMarked, key.isString());
-        if (!keyMarked || !valMarked)
-            e.removeFront();
-        else if (key != e.front().key)
-            e.rekeyFront(key);
-    }
+    sweepCrossCompartmentWrappers();
 
     /* Remove dead references held weakly by the compartment. */
 
     sweepBaseShapeTable();
     sweepInitialShapeTable();
     sweepNewTypeObjectTable(newTypeObjects);
     sweepNewTypeObjectTable(lazyTypeObjects);
 
@@ -546,16 +524,37 @@ JSCompartment::sweep(FreeOp *fop, bool r
                 script->clearAnalysis();
             }
         }
     }
 
     active = false;
 }
 
+/*
+ * Remove dead wrappers from the table. We must sweep all compartments, since
+ * string entries in the crossCompartmentWrappers table are not marked during
+ * markCrossCompartmentWrappers.
+ */
+void
+JSCompartment::sweepCrossCompartmentWrappers()
+{
+    /* Remove dead wrappers from the table. */
+    for (WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
+        Value key = e.front().key;
+        bool keyMarked = IsValueMarked(&key);
+        bool valMarked = IsValueMarked(e.front().value.unsafeGet());
+        JS_ASSERT_IF(!keyMarked && valMarked, key.isString());
+        if (!keyMarked || !valMarked)
+            e.removeFront();
+        else if (key != e.front().key)
+            e.rekeyFront(key);
+    }
+}
+
 void
 JSCompartment::purge()
 {
     dtoaCache.purge();
 }
 
 void
 JSCompartment::resetGCMallocBytes()
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -257,16 +257,17 @@ struct JSCompartment
     bool wrap(JSContext *cx, js::PropertyOp *op);
     bool wrap(JSContext *cx, js::StrictPropertyOp *op);
     bool wrap(JSContext *cx, js::PropertyDescriptor *desc);
     bool wrap(JSContext *cx, js::AutoIdVector &props);
 
     void markTypes(JSTracer *trc);
     void discardJitCode(js::FreeOp *fop);
     void sweep(js::FreeOp *fop, bool releaseTypes);
+    void sweepCrossCompartmentWrappers();
     void purge();
 
     void setGCLastBytes(size_t lastBytes, size_t lastMallocBytes, js::JSGCInvocationKind gckind);
     void reduceGCTriggerBytes(size_t amount);
 
     void resetGCMallocBytes();
     void setGCMaxMallocBytes(size_t value);
     void updateMallocCounter(size_t nbytes) {
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3250,18 +3250,22 @@ SweepPhase(JSRuntime *rt, JSGCInvocation
 
     /* Detach unreachable debuggers and global objects from each other. */
     Debugger::sweepAll(&fop);
 
     {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP_COMPARTMENTS);
 
         bool releaseTypes = ReleaseObservedTypes(rt);
-        for (GCCompartmentsIter c(rt); !c.done(); c.next())
-            c->sweep(&fop, releaseTypes);
+        for (CompartmentsIter c(rt); !c.done(); c.next()) {
+            if (c->isCollecting())
+                c->sweep(&fop, releaseTypes);
+            else
+                c->sweepCrossCompartmentWrappers();
+        }
     }
 
     {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP_OBJECT);
 
         /*
          * We finalize objects before other GC things to ensure that the object's
          * finalizer can access the other things even if they will be freed.