Bug 1395366 - Extend zone group's state to cover those intended for future use by helper threads and disallow GC of such groups r=sfink

js/src/gc/Verifier.cpp

@@ -525,7 +525,7 @@ HeapCheckTracerBase::onChild(const JS::GCCellPtr& thing)
 
     // Don't trace into GC in zones being used by helper threads.
     Zone* zone = thing.is<JSObject>() ? thing.as<JSObject>().zone() : cell->asTenured().zone();
-    if (zone->group() && zone->group()->usedByHelperThread)
+    if (zone->group() && zone->group()->usedByHelperThread())
         return;
 
     WorkItem item(thing, contextName(), parentIndex);

js/src/gc/Zone.cpp

@@ -298,9 +298,11 @@ Zone::hasMarkedCompartments()
 bool
 Zone::canCollect()
 {
-    // Zones cannot be collected while in use by other threads.
-    if (usedByHelperThread())
+    // Zones that will be or are currently used by other threads cannot be
+    // collected.
+    if (!isAtomsZone() && group()->createdForHelperThread())
         return false;
+
     JSRuntime* rt = runtimeFromAnyThread();
     if (isAtomsZone() && rt->hasHelperThreadZones())
         return false;

js/src/gc/Zone.h

@@ -533,7 +533,7 @@ struct Zone : public JS::shadow::Zone,
     js::ZoneGroupData<bool> isSystem;
 
     bool usedByHelperThread() {
-        return !isAtomsZone() && group()->usedByHelperThread;
+        return !isAtomsZone() && group()->usedByHelperThread();
     }
 
 #ifdef DEBUG
@@ -693,7 +693,7 @@ class ZoneGroupsIter
         it = rt->gc.groups().begin();
         end = rt->gc.groups().end();
 
-        if (!done() && (*it)->usedByHelperThread)
+        if (!done() && (*it)->createdForHelperThread())
             next();
     }
 
@@ -703,7 +703,7 @@ class ZoneGroupsIter
         MOZ_ASSERT(!done());
         do {
             it++;
-        } while (!done() && (*it)->usedByHelperThread);
+        } while (!done() && (*it)->createdForHelperThread());
     }
 
     ZoneGroup* get() const {

js/src/gc/ZoneGroup.cpp

@@ -20,7 +20,7 @@ ZoneGroup::ZoneGroup(JSRuntime* runtime)
     ownerContext_(TlsContext.get()),
     enterCount(1),
     zones_(this),
-    usedByHelperThread(false),
+    helperThreadUse(HelperThreadUse::None),
 #ifdef DEBUG
     ionBailAfter_(this, 0),
 #endif
@@ -45,6 +45,7 @@ ZoneGroup::init()
 ZoneGroup::~ZoneGroup()
 {
 #ifdef DEBUG
+    MOZ_ASSERT(helperThreadUse == HelperThreadUse::None);
     {
         AutoLockHelperThreadState lock;
         MOZ_ASSERT(ionLazyLinkListSize_ == 0);
@@ -65,7 +66,7 @@ ZoneGroup::enter(JSContext* cx)
         MOZ_ASSERT(enterCount);
     } else {
         if (useExclusiveLocking) {
-            MOZ_ASSERT(!usedByHelperThread);
+            MOZ_ASSERT(!usedByHelperThread());
             while (ownerContext().context() != nullptr) {
                 cx->yieldToEmbedding();
             }

js/src/gc/ZoneGroup.h

@@ -59,8 +59,37 @@ class ZoneGroup
   public:
     ZoneVector& zones() { return zones_.ref(); }
 
-    // Whether a zone in this group is in use by a helper thread.
-    mozilla::Atomic<bool> usedByHelperThread;
+  private:
+    enum class HelperThreadUse : uint32_t
+    {
+        None,
+        Pending,
+        Active
+    };
+
+    mozilla::Atomic<HelperThreadUse> helperThreadUse;
+
+  public:
+    // Whether a zone in this group was created for use by a helper thread.
+    bool createdForHelperThread() const {
+        return helperThreadUse != HelperThreadUse::None;
+    }
+    // Whether a zone in this group is currently in use by a helper thread.
+    bool usedByHelperThread() const {
+        return helperThreadUse == HelperThreadUse::Active;
+    }
+    void setCreatedForHelperThread() {
+        MOZ_ASSERT(helperThreadUse == HelperThreadUse::None);
+        helperThreadUse = HelperThreadUse::Pending;
+    }
+    void setUsedByHelperThread() {
+        MOZ_ASSERT(helperThreadUse == HelperThreadUse::Pending);
+        helperThreadUse = HelperThreadUse::Active;
+    }
+    void clearUsedByHelperThread() {
+        MOZ_ASSERT(helperThreadUse != HelperThreadUse::None);
+        helperThreadUse = HelperThreadUse::None;
+    }
 
     explicit ZoneGroup(JSRuntime* runtime);
     ~ZoneGroup();

js/src/threading/ProtectedData.cpp

@@ -79,7 +79,7 @@ CheckZoneGroup<Helper>::check() const
 
     JSContext* cx = TlsContext.get();
     if (group) {
-        if (group->usedByHelperThread) {
+        if (group->usedByHelperThread()) {
             MOZ_ASSERT(group->ownedByCurrentThread());
         } else {
             // This check is disabled on windows for the same reason as in

js/src/vm/HelperThreads.cpp

@@ -36,6 +36,7 @@ using namespace js;
 
 using mozilla::ArrayLength;
 using mozilla::DebugOnly;
+using mozilla::Maybe;
 using mozilla::Unused;
 using mozilla::TimeDuration;
 using mozilla::TimeStamp;
@@ -646,6 +647,14 @@ js::CancelOffThreadParses(JSRuntime* rt)
         if (!found)
             break;
     }
+
+#ifdef DEBUG
+    GlobalHelperThreadState::ParseTaskVector& worklist = HelperThreadState().parseWorklist(lock);
+    for (size_t i = 0; i < worklist.length(); i++) {
+        ParseTask* task = worklist[i];
+        MOZ_ASSERT(!task->runtimeMatches(rt));
+    }
+#endif
 }
 
 bool
@@ -696,8 +705,29 @@ EnsureParserCreatedClasses(JSContext* cx, ParseTaskKind kind)
     return true;
 }
 
+class AutoClearUsedByHelperThread
+{
+    ZoneGroup* group;
+
+  public:
+    AutoClearUsedByHelperThread(JSObject* global)
+      : group(global->zone()->group())
+    {}
+
+    void forget() {
+        group = nullptr;
+    }
+
+    ~AutoClearUsedByHelperThread() {
+        if (group)
+            group->clearUsedByHelperThread();
+    }
+};
+
 static JSObject*
-CreateGlobalForOffThreadParse(JSContext* cx, ParseTaskKind kind, const gc::AutoSuppressGC& nogc)
+CreateGlobalForOffThreadParse(JSContext* cx, ParseTaskKind kind,
+                              Maybe<AutoClearUsedByHelperThread>& clearUseGuard,
+                              const gc::AutoSuppressGC& nogc)
 {
     JSCompartment* currentCompartment = cx->compartment();
 
@@ -720,11 +750,18 @@ CreateGlobalForOffThreadParse(JSContext* cx, ParseTaskKind kind, const gc::AutoS
 
     JS_SetCompartmentPrincipals(global->compartment(), currentCompartment->principals());
 
+    // Mark this zone group as created for a helper thread. This prevents it
+    // from being collected until clearUsedByHelperThread() is called.
+    ZoneGroup* group = global->zone()->group();
+    group->setCreatedForHelperThread();
+    clearUseGuard.emplace(global);
+
     // Initialize all classes required for parsing while still on the active
     // thread, for both the target and the new global so that prototype
     // pointers can be changed infallibly after parsing finishes.
     if (!EnsureParserCreatedClasses(cx, kind))
         return nullptr;
+
     {
         AutoCompartment ac(cx, global);
         if (!EnsureParserCreatedClasses(cx, kind))
@@ -737,19 +774,18 @@ CreateGlobalForOffThreadParse(JSContext* cx, ParseTaskKind kind, const gc::AutoS
 static bool
 QueueOffThreadParseTask(JSContext* cx, ParseTask* task)
 {
-    if (OffThreadParsingMustWaitForGC(cx->runtime())) {
-        AutoLockHelperThreadState lock;
-        if (!HelperThreadState().parseWaitingOnGC(lock).append(task)) {
-            ReportOutOfMemory(cx);
-            return false;
-        }
-    } else {
-        AutoLockHelperThreadState lock;
-        if (!HelperThreadState().parseWorklist(lock).append(task)) {
-            ReportOutOfMemory(cx);
-            return false;
-        }
+    AutoLockHelperThreadState lock;
 
+    bool mustWait = OffThreadParsingMustWaitForGC(cx->runtime());
+
+    auto& queue = mustWait ? HelperThreadState().parseWaitingOnGC(lock)
+                           : HelperThreadState().parseWorklist(lock);
+    if (!queue.append(task)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+
+    if (!mustWait) {
         task->activate(cx->runtime());
         HelperThreadState().notifyOne(GlobalHelperThreadState::PRODUCER, lock);
     }
@@ -768,7 +804,8 @@ StartOffThreadParseTask(JSContext* cx, const ReadOnlyCompileOptions& options,
     gc::AutoAssertNoNurseryAlloc noNurseryAlloc;
     AutoSuppressAllocationMetadataBuilder suppressMetadata(cx);
 
-    JSObject* global = CreateGlobalForOffThreadParse(cx, kind, nogc);
+    Maybe<AutoClearUsedByHelperThread> clearUseGuard;
+    JSObject* global = CreateGlobalForOffThreadParse(cx, kind, clearUseGuard, nogc);
     if (!global)
         return false;
 
@@ -780,6 +817,7 @@ StartOffThreadParseTask(JSContext* cx, const ReadOnlyCompileOptions& options,
         return false;
 
     task.forget();
+    clearUseGuard->forget();
 
     return true;
 }
@@ -844,7 +882,7 @@ js::EnqueuePendingParseTasksAfterGC(JSRuntime* rt)
 
         for (size_t i = 0; i < waiting.length(); i++) {
             ParseTask* task = waiting[i];
-            if (task->runtimeMatches(rt) && !task->parseGlobal->zone()->wasGCStarted()) {
+            if (task->runtimeMatches(rt)) {
                 AutoEnterOOMUnsafeRegion oomUnsafe;
                 if (!newTasks.append(task))
                     oomUnsafe.crash("EnqueuePendingParseTasksAfterGC");
@@ -856,8 +894,8 @@ js::EnqueuePendingParseTasksAfterGC(JSRuntime* rt)
     if (newTasks.empty())
         return;
 
-    // This logic should mirror the contents of the !activeGCInAtomsZone()
-    // branch in StartOffThreadParseScript:
+    // This logic should mirror the contents of the
+    // !OffThreadParsingMustWaitForGC() branch in QueueOffThreadParseTask:
 
     for (size_t i = 0; i < newTasks.length(); i++)
         newTasks[i]->activate(rt);
@@ -1429,7 +1467,7 @@ js::GCParallelTask::~GCParallelTask()
     // base class can't ensure that the task is done using the members. All we
     // can do now is check that someone has previously stopped the task.
 #ifdef DEBUG
-    mozilla::Maybe<AutoLockHelperThreadState> helperLock;
+    Maybe<AutoLockHelperThreadState> helperLock;
     if (!HelperThreadState().isLockedByCurrentThread())
         helperLock.emplace();
     MOZ_ASSERT(state == NotStarted);

js/src/vm/Runtime.cpp

@@ -136,7 +136,7 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
 #ifdef DEBUG
     activeThreadHasExclusiveAccess(false),
 #endif
-    numHelperThreadZones(0),
+    numActiveHelperThreadZones(0),
     numCompartments(0),
     localeCallbacks(nullptr),
     defaultLocale(nullptr),
@@ -892,18 +892,18 @@ JSRuntime::destroyAtomsAddedWhileSweepingTable()
 void
 JSRuntime::setUsedByHelperThread(Zone* zone)
 {
-    MOZ_ASSERT(!zone->group()->usedByHelperThread);
+    MOZ_ASSERT(!zone->group()->usedByHelperThread());
     MOZ_ASSERT(!zone->wasGCStarted());
-    zone->group()->usedByHelperThread = true;
-    numHelperThreadZones++;
+    zone->group()->setUsedByHelperThread();
+    numActiveHelperThreadZones++;
 }
 
 void
 JSRuntime::clearUsedByHelperThread(Zone* zone)
 {
-    MOZ_ASSERT(zone->group()->usedByHelperThread);
-    zone->group()->usedByHelperThread = false;
-    numHelperThreadZones--;
+    MOZ_ASSERT(zone->group()->usedByHelperThread());
+    zone->group()->clearUsedByHelperThread();
+    numActiveHelperThreadZones--;
     JSContext* cx = TlsContext.get();
     if (gc.fullGCForAtomsRequested() && cx->canCollectAtoms())
         gc.triggerFullGCForAtoms(cx);

js/src/vm/Runtime.h

@@ -624,7 +624,7 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
 #endif
 
     /* Number of zones which may be operated on by non-cooperating helper threads. */
-    js::UnprotectedData<size_t> numHelperThreadZones;
+    js::UnprotectedData<size_t> numActiveHelperThreadZones;
 
     friend class js::AutoLockForExclusiveAccess;
 
@@ -633,7 +633,7 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
     void clearUsedByHelperThread(JS::Zone* zone);
 
     bool hasHelperThreadZones() const {
-        return numHelperThreadZones > 0;
+        return numActiveHelperThreadZones > 0;
     }
 
 #ifdef DEBUG