Bug 1324773 - Sweep JSCompartment::varNames_ r=sfink a=abillings a=jcristau
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 06 Jan 2017 11:23:22 +0000
changeset 368565 32d04f8230aa83e83ca6609d732c03f02c9926de
parent 368564 6bbe26dd67152c1768d9c8e17e5cddae4f02fa7c
child 368566 fc150c017ac2a53b02a05d007d9c61bcd841c3ed
push id1369
push userjlorenzo@mozilla.com
push dateMon, 27 Feb 2017 14:59:41 +0000
treeherdermozilla-release@d75a1dba431f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, abillings, jcristau
bugs1324773
milestone52.0a2
Bug 1324773 - Sweep JSCompartment::varNames_ r=sfink a=abillings a=jcristau
js/public/GCPolicyAPI.h
js/public/TracingAPI.h
js/src/jit-test/tests/parser/bug-1324773-2.js
js/src/jit-test/tests/parser/bug-1324773.js
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsgc.cpp
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -114,16 +114,21 @@ template <> struct GCPolicy<uint64_t> : 
 template <typename T>
 struct GCPointerPolicy
 {
     static T initial() { return nullptr; }
     static void trace(JSTracer* trc, T* vp, const char* name) {
         if (*vp)
             js::UnsafeTraceManuallyBarrieredEdge(trc, vp, name);
     }
+    static bool needsSweep(T* vp) {
+        if (*vp)
+            return js::gc::IsAboutToBeFinalizedUnbarriered(vp);
+        return false;
+    }
 };
 template <> struct GCPolicy<JS::Symbol*> : public GCPointerPolicy<JS::Symbol*> {};
 template <> struct GCPolicy<JSAtom*> : public GCPointerPolicy<JSAtom*> {};
 template <> struct GCPolicy<JSFunction*> : public GCPointerPolicy<JSFunction*> {};
 template <> struct GCPolicy<JSObject*> : public GCPointerPolicy<JSObject*> {};
 template <> struct GCPolicy<JSScript*> : public GCPointerPolicy<JSScript*> {};
 template <> struct GCPolicy<JSString*> : public GCPointerPolicy<JSString*> {};
 
--- a/js/public/TracingAPI.h
+++ b/js/public/TracingAPI.h
@@ -380,16 +380,24 @@ namespace js {
 //
 // This method does not check if |*edgep| is non-null before tracing through
 // it, so callers must check any nullable pointer before calling this method.
 template <typename T>
 extern JS_PUBLIC_API(void)
 UnsafeTraceManuallyBarrieredEdge(JSTracer* trc, T* edgep, const char* name);
 
 namespace gc {
+
 // Return true if the given edge is not live and is about to be swept.
 template <typename T>
 extern JS_PUBLIC_API(bool)
 EdgeNeedsSweep(JS::Heap<T>* edgep);
+
+// Not part of the public API, but declared here so we can use it in GCPolicy
+// which is.
+template <typename T>
+bool
+IsAboutToBeFinalizedUnbarriered(T* thingp);
+
 } // namespace gc
 } // namespace js
 
 #endif /* js_TracingAPI_h */
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/parser/bug-1324773-2.js
@@ -0,0 +1,15 @@
+if (!('gczeal' in this))
+    quit();
+var lfGlobal = newGlobal();
+lfGlobal.evaluate(`
+    for (var i = 0; i < 600; i++)
+        eval('function f' + i + '() {}');
+`);
+var lfGlobal = newGlobal();
+lfGlobal.evaluate(`
+    if (!('gczeal' in this))
+        quit();
+    var dbg = new Debugger();
+    gczeal(9, 10);
+    dbg.findScripts({});
+`);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/parser/bug-1324773.js
@@ -0,0 +1,15 @@
+if (!('gczeal' in this))
+    quit();
+var lfGlobal = newGlobal();
+lfGlobal.evaluate(`
+    for (var i = 0; i < 600; i++)
+        eval('function f' + i + '() {}');
+`);
+var lfGlobal = newGlobal();
+lfGlobal.evaluate(`
+    if (!('gczeal' in this))
+        quit();
+    var dbg = new Debugger();
+    gczeal(9, 1);
+    dbg.findScripts({});
+`);
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -564,17 +564,20 @@ JSCompartment::getNonSyntacticLexicalEnv
     if (!lexicalEnv)
         return nullptr;
     return &lexicalEnv->as<LexicalEnvironmentObject>();
 }
 
 bool
 JSCompartment::addToVarNames(JSContext* cx, JS::Handle<JSAtom*> name)
 {
-    if (varNames_.put(name.get()))
+    MOZ_ASSERT(name);
+    MOZ_ASSERT(!isAtomsCompartment());
+
+    if (varNames_.put(name))
         return true;
 
     ReportOutOfMemory(cx);
     return false;
 }
 
 void
 JSCompartment::traceOutgoingCrossCompartmentWrappers(JSTracer* trc)
@@ -796,16 +799,22 @@ JSCompartment::sweepNativeIterators()
  * markCrossCompartmentWrappers.
  */
 void
 JSCompartment::sweepCrossCompartmentWrappers()
 {
     crossCompartmentWrappers.sweep();
 }
 
+void
+JSCompartment::sweepVarNames()
+{
+    varNames_.sweep();
+}
+
 namespace {
 struct TraceRootFunctor {
     JSTracer* trc;
     const char* name;
     TraceRootFunctor(JSTracer* trc, const char* name) : trc(trc), name(name) {}
     template <class T> void operator()(T* t) { return TraceRoot(trc, t, name); }
 };
 struct NeedsSweepUnbarrieredFunctor {
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -648,16 +648,17 @@ struct JSCompartment
     void sweepSavedStacks();
     void sweepGlobalObject(js::FreeOp* fop);
     void sweepSelfHostingScriptSource();
     void sweepJitCompartment(js::FreeOp* fop);
     void sweepRegExps();
     void sweepDebugEnvironments();
     void sweepNativeIterators();
     void sweepTemplateObjects();
+    void sweepVarNames();
 
     void purge();
     void clearTables();
 
     static void fixupCrossCompartmentWrappersAfterMovingGC(JSTracer* trc);
     void fixupAfterMovingGC();
     void fixupGlobal();
     void fixupScriptMapsAfterMovingGC();
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4865,16 +4865,18 @@ MAKE_GC_SWEEP_TASK(SweepObjectGroupsTask
 MAKE_GC_SWEEP_TASK(SweepRegExpsTask);
 MAKE_GC_SWEEP_TASK(SweepMiscTask);
 #undef MAKE_GC_SWEEP_TASK
 
 /* virtual */ void
 SweepAtomsTask::run()
 {
     runtime->sweepAtoms();
+    for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
+        comp->sweepVarNames();
 }
 
 /* virtual */ void
 SweepCCWrappersTask::run()
 {
     for (GCCompartmentGroupIter c(runtime); !c.done(); c.next())
         c->sweepCrossCompartmentWrappers();
 }