From 8a21090cbb92d41786a32691215b168b9bd604fa Mon Sep 17 00:00:00 2001
From: Jon Coppeard <jcoppeard@mozilla.com>
Date: Wed, 21 Jun 2023 07:19:07 +0000
Subject: [PATCH] Bug 1828024 - Require the helper thread lock in the GC helper
 thread count getter r=sfink

This makes us take a lock to read this state (we already lock when writing it).

Also it adds a release assert in case something goes wrong with the thread
count calculations, as a crash is preferable to the potential deadlock.

Differential Revision: https://phabricator.services.mozilla.com/D181257
---
 js/src/gc/GC.cpp              | 9 +++++++--
 js/src/gc/ParallelMarking.cpp | 4 ++++
 js/src/vm/HelperThreadState.h | 6 ++++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
index fb82e8b76278..86d4c35b26c4 100644
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -1331,6 +1331,11 @@ void GCRuntime::assertNoMarkingWork() const {
 }
 #endif
 
+static size_t GetGCParallelThreadCount() {
+  AutoLockHelperThreadState lock;
+  return HelperThreadState().getGCParallelThreadCount(lock);
+}
+
 bool GCRuntime::updateMarkersVector() {
   MOZ_ASSERT(helperThreadCount >= 1,
              "There must always be at least one mark task");
@@ -1339,8 +1344,8 @@ bool GCRuntime::updateMarkersVector() {
 
   // Limit worker count to number of GC parallel tasks that can run
   // concurrently, otherwise one thread can deadlock waiting on another.
-  size_t targetCount = std::min(markingWorkerCount(),
-                                HelperThreadState().getGCParallelThreadCount());
+  size_t targetCount =
+      std::min(markingWorkerCount(), GetGCParallelThreadCount());
 
   if (markers.length() > targetCount) {
     return markers.resize(targetCount);
diff --git a/js/src/gc/ParallelMarking.cpp b/js/src/gc/ParallelMarking.cpp
index 4c92c2de0a8b..67c29c02d720 100644
--- a/js/src/gc/ParallelMarking.cpp
+++ b/js/src/gc/ParallelMarking.cpp
@@ -103,6 +103,10 @@ bool ParallelMarker::markOneColor(MarkColor color, SliceBudget& sliceBudget) {
   {
     AutoLockHelperThreadState lock;
 
+    // There should always be enough parallel tasks to run our marking work.
+    MOZ_RELEASE_ASSERT(HelperThreadState().getGCParallelThreadCount(lock) >=
+                       workerCount());
+
     for (size_t i = 0; i < workerCount(); i++) {
       gc->startTask(*tasks[i], lock);
     }
diff --git a/js/src/vm/HelperThreadState.h b/js/src/vm/HelperThreadState.h
index c8f6bc056a3e..e803559ab430 100644
--- a/js/src/vm/HelperThreadState.h
+++ b/js/src/vm/HelperThreadState.h
@@ -333,9 +333,11 @@ class GlobalHelperThreadState {
 
   GCParallelTaskList& gcParallelWorklist() { return gcParallelWorklist_; }
 
-  size_t getGCParallelThreadCount() const { return gcParallelThreadCount; }
+  size_t getGCParallelThreadCount(const AutoLockHelperThreadState& lock) const {
+    return gcParallelThreadCount;
+  }
   void setGCParallelThreadCount(size_t count,
-                                const AutoLockHelperThreadState&) {
+                                const AutoLockHelperThreadState& lock) {
     MOZ_ASSERT(count >= 1);
     MOZ_ASSERT(count <= threadCount);
     gcParallelThreadCount = count;
-- 
2.11.4.GIT