Bug 1005306 - Improve GC-handling of canonical JSScript for LazyScripts. r=jonco,billm

js/src/gc/Marking.cpp

@@ -489,20 +489,11 @@ IsAboutToBeFinalized(T **thingp)
 #endif  // JSGC_GENERATIONAL
 
     Zone *zone = thing->tenuredZone();
-    if (zone->isGCSweeping()) {
-        /*
-         * We should return false for things that have been allocated during
-         * incremental sweeping, but this possibility doesn't occur at the moment
-         * because this function is only called at the very start of the sweeping a
-         * compartment group and during minor gc. Rather than do the extra check,
-         * we just assert that it's not necessary.
-         */
-        JS_ASSERT_IF(!rt->isHeapMinorCollecting(), !thing->arenaHeader()->allocatedDuringIncremental);
+    if (zone->isGCSweeping())
+        return !(thing->isMarked() || thing->arenaHeader()->allocatedDuringIncremental);
 
-        return !thing->isMarked();
-    }
 #ifdef JSGC_COMPACTING
-    else if (zone->isGCCompacting() && IsForwarded(thing)) {
+    if (zone->isGCCompacting() && IsForwarded(thing)) {
         *thingp = Forwarded(thing);
         return false;
     }

js/src/jsfun.cpp

@@ -1531,11 +1531,6 @@ JSFunction::relazify(JSTracer *trc)
     u.i.s.lazy_ = lazy;
     if (lazy) {
         JS_ASSERT(!isSelfHostedBuiltin());
-        // If this is the script stored in the lazy script to be cloned
-        // for un-lazifying other functions, reset it so the script can
-        // be freed.
-        if (lazy->maybeScript() == script)
-            lazy->resetScript();
         MarkLazyScriptUnbarriered(trc, &u.i.s.lazy_, "lazyScript");
     } else {
         JS_ASSERT(isSelfHostedBuiltin());

js/src/jsscript.cpp

@@ -2634,6 +2634,14 @@ JSScript::finalize(FreeOp *fop)
     }
 
     fop->runtime()->lazyScriptCache.remove(this);
+    // Calling lazyScript->maybeScript here will reset lazyScript::script_ iff
+    // it points to this JSScript or another JSScript that is about to be
+    // finalized, so just the call guarantees that the lazyScript doesn't keep
+    // a pointer to the finalized script around.
+    if (lazyScript) {
+        lazyScript->maybeScript();
+        MOZ_ASSERT(lazyScript->maybeScript() != this);
+    }
 }
 
 static const uint32_t GSN_CACHE_THRESHOLD = 100;
@@ -3350,8 +3358,15 @@ JSScript::markChildren(JSTracer *trc)
     if (enclosingScopeOrOriginalFunction_)
         MarkObject(trc, &enclosingScopeOrOriginalFunction_, "enclosing");
 
-    if (maybeLazyScript())
+    if (maybeLazyScript()) {
         MarkLazyScriptUnbarriered(trc, &lazyScript, "lazyScript");
+        // We might be operating on a clone of a script, in which case
+        // lazyScript->maybeScript will return the canonical script. Marking
+        // the lazyScript's script here guarantees that it stays around as
+        // long as any clones do.
+        if (lazyScript->maybeScript())
+            lazyScript->markScript(trc);
+    }
 
     if (IS_GC_MARKING_TRACER(trc)) {
         compartment()->mark();
@@ -3377,9 +3392,6 @@ LazyScript::markChildren(JSTracer *trc)
     if (enclosingScope_)
         MarkObject(trc, &enclosingScope_, "enclosingScope");
 
-    if (script_)
-        MarkScript(trc, &script_, "realScript");
-
     HeapPtrAtom *freeVariables = this->freeVariables();
     for (size_t i = 0; i < numFreeVariables(); i++)
         MarkString(trc, &freeVariables[i], "lazyScriptFreeVariable");
@@ -3619,6 +3631,13 @@ LazyScript::resetScript()
     script_ = nullptr;
 }
 
+void
+LazyScript::markScript(JSTracer *trc)
+{
+    JS_ASSERT(script_);
+    MarkScript(trc, &script_, "script");
+}
+
 void
 LazyScript::setParent(JSObject *enclosingScope, ScriptSourceObject *sourceObject)
 {

js/src/jsscript.h

@@ -1777,7 +1777,17 @@ class LazyScript : public gc::BarrieredCell<LazyScript>
 
     void initScript(JSScript *script);
     void resetScript();
+    void markScript(JSTracer *trc);
     JSScript *maybeScript() {
+        // script_ is a weak pointer that only gets marked if the JSScript
+        // it points to is. If the script isn't marked and about to be
+        // collected, we manually reset the pointer here.
+        if (script_) {
+            if (gc::IsAboutToBeFinalized(&script_))
+                script_ = nullptr;
+            else
+                JSScript::readBarrier(script_);
+        }
         return script_;
     }