diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index bc0e4bfb45..de0960783e 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -3554,7 +3554,20 @@ void Isolate::UpdateLogObjectRelocation() { void Isolate::Deinit() { TRACE_ISOLATE(deinit); - DisallowGarbageCollection disallow_gc; + + // All client isolates should already be detached when the shared heap isolate + // tears down. + if (is_shared_heap_isolate()) { + global_safepoint()->AssertNoClientsOnTearDown(); + } + + if (has_shared_heap()) { + IgnoreLocalGCRequests ignore_gc_requests(heap()); + ParkedScope parked_scope(main_thread_local_heap()); + shared_heap_isolate()->global_safepoint()->clients_mutex_.Lock(); + } + + DisallowGarbageCollection no_gc; tracing_cpu_profiler_.reset(); if (v8_flags.stress_sampling_allocation_profiler > 0) { @@ -3590,11 +3603,6 @@ void Isolate::Deinit() { optimizing_compile_dispatcher_ = nullptr; } - // All client isolates should already be detached. - if (is_shared()) { - global_safepoint()->AssertNoClientsOnTearDown(); - } - if (v8_flags.print_deopt_stress) { PrintF(stdout, "=== Stress deopt counter: %u\n", stress_deopt_count_); } @@ -3631,20 +3639,15 @@ void Isolate::Deinit() { // At this point there are no more background threads left in this isolate. heap_.safepoint()->AssertMainThreadIsOnlyThread(); - // Tear down data using the shared heap before detaching. + // Tear down data that requires the shared heap before detaching. heap_.TearDownWithSharedHeap(); - { - // This isolate might have to park for a shared GC initiated by another - // client isolate before it can actually detach from the shared isolate. - AllowGarbageCollection allow_shared_gc; + // Detach from the shared heap isolate and then unlock the mutex. + if (has_shared_heap()) { + Isolate* shared_heap_isolate = this->shared_heap_isolate(); DetachFromSharedIsolate(); DetachFromSharedSpaceIsolate(); - } - - // All client isolates should already be detached. - if (is_shared_space_isolate()) { - global_safepoint()->AssertNoClientsOnTearDown(); + shared_heap_isolate->global_safepoint()->clients_mutex_.Unlock(); } // Since there are no other threads left, we can lock this mutex without any diff --git a/src/heap/safepoint.cc b/src/heap/safepoint.cc index 87a40837d0..12b0882b43 100644 --- a/src/heap/safepoint.cc +++ b/src/heap/safepoint.cc @@ -312,12 +312,7 @@ void GlobalSafepoint::AppendClient(Isolate* client) { void GlobalSafepoint::RemoveClient(Isolate* client) { DCHECK_EQ(client->heap()->gc_state(), Heap::TEAR_DOWN); - - // 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()); - ParkedRecursiveMutexGuard guard(client->main_thread_local_heap(), - &clients_mutex_); + AssertActive(); if (client->global_safepoint_next_client_isolate_) { client->global_safepoint_next_client_isolate_ @@ -338,12 +333,18 @@ void GlobalSafepoint::RemoveClient(Isolate* client) { } void GlobalSafepoint::AssertNoClientsOnTearDown() { - DCHECK_WITH_MSG( - clients_head_ == nullptr, - "Shared heap must not have clients at teardown. The first isolate that " - "is created (in a process that has no isolates) owns the lifetime of the " - "shared heap and is considered the main isolate. The main isolate must " - "outlive all other isolates."); + if (v8_flags.shared_space) { + DCHECK_EQ(clients_head_, shared_heap_isolate_); + DCHECK_NULL(shared_heap_isolate_->global_safepoint_prev_client_isolate_); + DCHECK_NULL(shared_heap_isolate_->global_safepoint_next_client_isolate_); + } else { + DCHECK_WITH_MSG( + clients_head_ == nullptr, + "Shared heap must not have clients at teardown. The first isolate that " + "is created (in a process that has no isolates) owns the lifetime of " + "the shared heap and is considered the main isolate. The main isolate " + "must outlive all other isolates."); + } } void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) {