[heap] Allow nesting of GlobalSafepointScopes

GlobalSafepointScope may be nested similar to isolate-local
SafepointScopes.

Bug: v8:13267, v8:13539
Change-Id: Ic2709a6466a42160d2a2c392fdf9521432725604
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4062090
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84556}

src/execution/isolate.cc

@@ -4218,7 +4218,7 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
 
   // Lock clients_mutex_ in order to prevent shared GCs from other clients
   // during deserialization.
-  base::Optional<base::MutexGuard> clients_guard;
+  base::Optional<base::RecursiveMutexGuard> clients_guard;
 
   if (Isolate* isolate =
           shared_isolate_ ? shared_isolate_ : attach_to_shared_space_isolate) {

src/heap/parked-scope.h

@@ -70,6 +70,32 @@ class V8_NODISCARD ParkedMutexGuard {
   base::Mutex* mutex_;
 };
 
+// Scope that automatically parks the thread while blocking on the given
+// base::RecursiveMutex.
+class V8_NODISCARD ParkedRecursiveMutexGuard {
+ public:
+  explicit ParkedRecursiveMutexGuard(LocalIsolate* local_isolate,
+                                     base::RecursiveMutex* mutex)
+      : ParkedRecursiveMutexGuard(local_isolate->heap(), mutex) {}
+  explicit ParkedRecursiveMutexGuard(LocalHeap* local_heap,
+                                     base::RecursiveMutex* mutex)
+      : mutex_(mutex) {
+    DCHECK(AllowGarbageCollection::IsAllowed());
+    if (!mutex_->TryLock()) {
+      ParkedScope scope(local_heap);
+      mutex_->Lock();
+    }
+  }
+
+  ParkedRecursiveMutexGuard(const ParkedMutexGuard&) = delete;
+  ParkedRecursiveMutexGuard& operator=(const ParkedMutexGuard&) = delete;
+
+  ~ParkedRecursiveMutexGuard() { mutex_->Unlock(); }
+
+ private:
+  base::RecursiveMutex* mutex_;
+};
+
 template <base::MutexSharedType kIsShared,
           base::NullBehavior Behavior = base::NullBehavior::kRequireNotNull>
 class V8_NODISCARD ParkedSharedMutexGuardIf final {

src/heap/safepoint.cc

@@ -26,8 +26,7 @@
 namespace v8 {
 namespace internal {
 
-IsolateSafepoint::IsolateSafepoint(Heap* heap)
-    : heap_(heap), local_heaps_head_(nullptr), active_safepoint_scopes_(0) {}
+IsolateSafepoint::IsolateSafepoint(Heap* heap) : heap_(heap) {}
 
 void IsolateSafepoint::EnterLocalSafepointScope() {
   // Safepoints need to be initiated on some main thread.
@@ -317,7 +316,8 @@ void GlobalSafepoint::RemoveClient(Isolate* client) {
   // A shared heap may have already acquired the client mutex to perform a
   // shared GC. We need to park the Isolate here to allow for a shared GC.
   IgnoreLocalGCRequests ignore_gc_requests(client->heap());
-  ParkedMutexGuard guard(client->main_thread_local_heap(), &clients_mutex_);
+  ParkedRecursiveMutexGuard guard(client->main_thread_local_heap(),
+                                  &clients_mutex_);
 
   if (client->global_safepoint_next_client_isolate_) {
     client->global_safepoint_next_client_isolate_
@@ -356,6 +356,8 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {
     clients_mutex_.Lock();
   }
 
+  if (++active_safepoint_scopes_ > 1) return;
+
   TimedHistogramScope timer(
       initiator->counters()->gc_time_to_global_safepoint());
   TRACE_GC(initiator->heap()->tracer(),
@@ -404,14 +406,19 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {
 }
 
 void GlobalSafepoint::LeaveGlobalSafepointScope(Isolate* initiator) {
-  if (shared_heap_isolate_->is_shared()) {
-    shared_heap_isolate_->heap()->safepoint()->local_heaps_mutex_.Unlock();
-  }
+  clients_mutex_.AssertHeld();
+  DCHECK_GT(active_safepoint_scopes_, 0);
 
-  IterateClientIsolates([initiator](Isolate* client) {
-    Heap* client_heap = client->heap();
-    client_heap->safepoint()->LeaveGlobalSafepointScope(initiator);
-  });
+  if (--active_safepoint_scopes_ == 0) {
+    if (shared_heap_isolate_->is_shared()) {
+      shared_heap_isolate_->heap()->safepoint()->local_heaps_mutex_.Unlock();
+    }
+
+    IterateClientIsolates([initiator](Isolate* client) {
+      Heap* client_heap = client->heap();
+      client_heap->safepoint()->LeaveGlobalSafepointScope(initiator);
+    });
+  }
 
   clients_mutex_.Unlock();
 }

src/heap/safepoint.h

@@ -141,9 +141,9 @@ class IsolateSafepoint final {
   // Mutex is used both for safepointing and adding/removing threads. A
   // RecursiveMutex is needed since we need to support nested SafepointScopes.
   base::RecursiveMutex local_heaps_mutex_;
-  LocalHeap* local_heaps_head_;
+  LocalHeap* local_heaps_head_ = nullptr;
 
-  int active_safepoint_scopes_;
+  int active_safepoint_scopes_ = 0;
 
   friend class GlobalSafepoint;
   friend class GlobalSafepointScope;
@@ -187,8 +187,11 @@ class GlobalSafepoint final {
   void LeaveGlobalSafepointScope(Isolate* initiator);
 
   Isolate* const shared_heap_isolate_;
-  base::Mutex clients_mutex_;
+  // RecursiveMutex is needed since we need to support nested
+  // GlobalSafepointScopes.
+  base::RecursiveMutex clients_mutex_;
   Isolate* clients_head_ = nullptr;
+  int active_safepoint_scopes_ = 0;
 
   friend class GlobalSafepointScope;
   friend class Isolate;