[heap] Fix safepoint in shared space isolate GC

A shared space isolate needs to safepoint all clients as well in order
to collect garbage in the shared spaces.

Bug: v8:13267
Change-Id: I3f00a84bd46353c4351bbbe4240b90d8847afc8b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3912764Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83393}

src/execution/isolate.cc

@@ -3572,7 +3572,7 @@ void Isolate::Deinit() {
   }
 
   // All client isolates should already be detached.
-  if (is_shared() || is_shared_space_isolate()) {
+  if (is_shared()) {
     global_safepoint()->AssertNoClientsOnTearDown();
   }
 
@@ -3624,6 +3624,11 @@ void Isolate::Deinit() {
     DetachFromSharedSpaceIsolate();
   }
 
+  // All client isolates should already be detached.
+  if (is_shared_space_isolate()) {
+    global_safepoint()->AssertNoClientsOnTearDown();
+  }
+
   // Since there are no other threads left, we can lock this mutex without any
   // ceremony. This signals to the tear down code that we are in a safepoint.
   base::RecursiveMutexGuard safepoint(&heap_.safepoint()->local_heaps_mutex_);
@@ -5952,6 +5957,7 @@ void Isolate::AttachToSharedIsolate() {
 
   if (shared_isolate_) {
     DCHECK(shared_isolate_->is_shared());
+    DCHECK(!v8_flags.shared_space);
     shared_isolate_->global_safepoint()->AppendClient(this);
   }
 
@@ -5964,6 +5970,7 @@ void Isolate::DetachFromSharedIsolate() {
   DCHECK(attached_to_shared_isolate_);
 
   if (shared_isolate_) {
+    DCHECK(!v8_flags.shared_space);
     shared_isolate_->global_safepoint()->RemoveClient(this);
     shared_isolate_ = nullptr;
   }
@@ -5976,7 +5983,8 @@ void Isolate::DetachFromSharedIsolate() {
 void Isolate::AttachToSharedSpaceIsolate(Isolate* shared_space_isolate) {
   DCHECK(!shared_space_isolate_.has_value());
   shared_space_isolate_ = shared_space_isolate;
-  if (shared_space_isolate && shared_space_isolate != this) {
+  if (shared_space_isolate) {
+    DCHECK(v8_flags.shared_space);
     shared_space_isolate->global_safepoint()->AppendClient(this);
   }
 }
@@ -5984,7 +5992,8 @@ void Isolate::AttachToSharedSpaceIsolate(Isolate* shared_space_isolate) {
 void Isolate::DetachFromSharedSpaceIsolate() {
   DCHECK(shared_space_isolate_.has_value());
   Isolate* shared_space_isolate = shared_space_isolate_.value();
-  if (shared_space_isolate && shared_space_isolate != this) {
+  if (shared_space_isolate) {
+    DCHECK(v8_flags.shared_space);
     shared_space_isolate->global_safepoint()->RemoveClient(this);
   }
   shared_space_isolate_.reset();

src/heap/heap.cc

@@ -2338,12 +2338,18 @@ size_t Heap::PerformGarbageCollection(
   DCHECK(tracer()->IsConsistentWithCollector(collector));
   TRACE_GC_EPOCH(tracer(), CollectorScopeId(collector), ThreadKind::kMain);
 
-  base::Optional<SafepointScope> safepoint_scope;
+  base::Optional<GlobalSafepointScope> global_safepoint_scope;
+  base::Optional<SafepointScope> isolate_safepoint_scope;
 
   {
     AllowGarbageCollection allow_shared_gc;
     IgnoreLocalGCRequests ignore_gc_requests(this);
-    safepoint_scope.emplace(this);
+
+    if (isolate()->is_shared_heap_isolate()) {
+      global_safepoint_scope.emplace(isolate());
+    } else {
+      isolate_safepoint_scope.emplace(this);
+    }
   }
 
   collection_barrier_->StopTimeToCollectionTimer();
@@ -3669,6 +3675,12 @@ void Heap::FreeLinearAllocationAreas() {
   safepoint()->IterateLocalHeaps(
       [](LocalHeap* local_heap) { local_heap->FreeLinearAllocationArea(); });
 
+  if (isolate()->is_shared_space_isolate()) {
+    isolate()->global_safepoint()->IterateClientIsolates([](Isolate* client) {
+      client->heap()->FreeSharedLinearAllocationAreas();
+    });
+  }
+
   PagedSpaceIterator spaces(this);
   for (PagedSpace* space = spaces.Next(); space != nullptr;
        space = spaces.Next()) {

src/heap/safepoint.cc

@@ -72,7 +72,7 @@ class PerClientSafepointData final {
 
 void IsolateSafepoint::InitiateGlobalSafepointScope(
     Isolate* initiator, PerClientSafepointData* client_data) {
-  shared_isolate()->global_safepoint()->AssertActive();
+  shared_heap_isolate()->global_safepoint()->AssertActive();
   IgnoreLocalGCRequests ignore_gc_requests(initiator->heap());
   LockMutex(initiator->main_thread_local_heap());
   InitiateGlobalSafepointScopeRaw(initiator, client_data);
@@ -80,7 +80,7 @@ void IsolateSafepoint::InitiateGlobalSafepointScope(
 
 void IsolateSafepoint::TryInitiateGlobalSafepointScope(
     Isolate* initiator, PerClientSafepointData* client_data) {
-  shared_isolate()->global_safepoint()->AssertActive();
+  shared_heap_isolate()->global_safepoint()->AssertActive();
   if (!local_heaps_mutex_.TryLock()) return;
   InitiateGlobalSafepointScopeRaw(initiator, client_data);
 }
@@ -278,8 +278,8 @@ void IsolateSafepoint::AssertMainThreadIsOnlyThread() {
 
 Isolate* IsolateSafepoint::isolate() const { return heap_->isolate(); }
 
-Isolate* IsolateSafepoint::shared_isolate() const {
-  return isolate()->shared_isolate();
+Isolate* IsolateSafepoint::shared_heap_isolate() const {
+  return isolate()->shared_heap_isolate();
 }
 
 SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) {
@@ -289,7 +289,7 @@ SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) {
 SafepointScope::~SafepointScope() { safepoint_->LeaveLocalSafepointScope(); }
 
 GlobalSafepoint::GlobalSafepoint(Isolate* isolate)
-    : shared_isolate_(isolate), shared_heap_(isolate->heap()) {}
+    : shared_heap_isolate_(isolate) {}
 
 void GlobalSafepoint::AppendClient(Isolate* client) {
   clients_mutex_.AssertHeld();
@@ -368,11 +368,15 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {
         initiator, &clients.back());
   });
 
-  // Make it possible to use AssertActive() on shared isolates.
-  CHECK(shared_isolate_->heap()->safepoint()->local_heaps_mutex_.TryLock());
+  if (shared_heap_isolate_->is_shared()) {
+    // Make it possible to use AssertActive() on shared isolates.
+    CHECK(shared_heap_isolate_->heap()
+              ->safepoint()
+              ->local_heaps_mutex_.TryLock());
 
-  // Shared isolates should never have multiple threads.
-  shared_isolate_->heap()->safepoint()->AssertMainThreadIsOnlyThread();
+    // Shared isolates should never have multiple threads.
+    shared_heap_isolate_->heap()->safepoint()->AssertMainThreadIsOnlyThread();
+  }
 
   // Iterate all clients again to initiate the safepoint for all of them - even
   // if that means blocking.
@@ -383,7 +387,7 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {
 
 #if DEBUG
   for (const PerClientSafepointData& client : clients) {
-    DCHECK_EQ(client.isolate()->shared_isolate(), shared_isolate_);
+    DCHECK_EQ(client.isolate()->shared_heap_isolate(), shared_heap_isolate_);
     DCHECK(client.heap()->deserialization_complete());
   }
 #endif  // DEBUG
@@ -397,7 +401,9 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {
 }
 
 void GlobalSafepoint::LeaveGlobalSafepointScope(Isolate* initiator) {
-  shared_isolate_->heap()->safepoint()->local_heaps_mutex_.Unlock();
+  if (shared_heap_isolate_->is_shared()) {
+    shared_heap_isolate_->heap()->safepoint()->local_heaps_mutex_.Unlock();
+  }
 
   IterateClientIsolates([initiator](Isolate* client) {
     Heap* client_heap = client->heap();
@@ -408,17 +414,22 @@ void GlobalSafepoint::LeaveGlobalSafepointScope(Isolate* initiator) {
 }
 
 GlobalSafepointScope::GlobalSafepointScope(Isolate* initiator)
-    : initiator_(initiator), shared_isolate_(initiator->shared_isolate()) {
-  if (shared_isolate_) {
-    shared_isolate_->global_safepoint()->EnterGlobalSafepointScope(initiator_);
+    : initiator_(initiator),
+      shared_heap_isolate_(initiator->has_shared_heap()
+                               ? initiator->shared_heap_isolate()
+                               : nullptr) {
+  if (shared_heap_isolate_) {
+    shared_heap_isolate_->global_safepoint()->EnterGlobalSafepointScope(
+        initiator_);
   } else {
     initiator_->heap()->safepoint()->EnterLocalSafepointScope();
   }
 }
 
 GlobalSafepointScope::~GlobalSafepointScope() {
-  if (shared_isolate_) {
-    shared_isolate_->global_safepoint()->LeaveGlobalSafepointScope(initiator_);
+  if (shared_heap_isolate_) {
+    shared_heap_isolate_->global_safepoint()->LeaveGlobalSafepointScope(
+        initiator_);
   } else {
     initiator_->heap()->safepoint()->LeaveLocalSafepointScope();
   }

src/heap/safepoint.h

@@ -133,7 +133,7 @@ class IsolateSafepoint final {
   }
 
   Isolate* isolate() const;
-  Isolate* shared_isolate() const;
+  Isolate* shared_heap_isolate() const;
 
   Barrier barrier_;
   Heap* heap_;
@@ -186,8 +186,7 @@ class GlobalSafepoint final {
   void EnterGlobalSafepointScope(Isolate* initiator);
   void LeaveGlobalSafepointScope(Isolate* initiator);
 
-  Isolate* const shared_isolate_;
-  Heap* const shared_heap_;
+  Isolate* const shared_heap_isolate_;
   base::Mutex clients_mutex_;
   Isolate* clients_head_ = nullptr;
 
@@ -202,7 +201,7 @@ class V8_NODISCARD GlobalSafepointScope {
 
  private:
   Isolate* const initiator_;
-  Isolate* const shared_isolate_;
+  Isolate* const shared_heap_isolate_;
 };
 
 }  // namespace internal