[vm, isolates] Avoid three-way deadlock during isolate exit.
Running mutator
holds safepoint operation scope starting old space GC
waiting for old space tasks to reach 0
Concurrent marker
holds old space tasks > 0
waiting for isolates_list_ lock to interrupt for finalization
Exiting mutator
holds isolates_list_ lock_ to unregister isolate
waiting for safepoint to end TransitionVMToBlocked
Reorder isolate [un]registeration to not need safepoint transition to acquire the isolates_list_ lock.
TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/59574
Change-Id: Ia98fabd654c880b253893a0598d2e26ed77f52da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397660
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 0236145..887d560 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -454,11 +454,6 @@
return isolate_count_ == 0 || isolate_count_ == 1;
}
-void IsolateGroup::RunWithLockedGroup(std::function<void()> fun) {
- SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
- fun();
-}
-
void IsolateGroup::UnregisterIsolate(Isolate* isolate) {
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
isolates_.Remove(isolate);
@@ -1850,14 +1845,6 @@
#undef ISOLATE_METRIC_INIT
#endif // !defined(PRODUCT)
- // First we ensure we enter the isolate. This will ensure we're participating
- // in any safepointing requests from this point on. Other threads requesting a
- // safepoint operation will therefore wait until we've stopped.
- //
- // Though the [result] isolate is still in a state where no memory has been
- // allocated, which means it's safe to GC the isolate group until here.
- Thread::EnterIsolate(result);
-
// Setup the isolate message handler.
result->message_handler_ = new IsolateMessageHandler(result);
@@ -1885,6 +1872,14 @@
// to vm-isolate objects, e.g. null)
isolate_group->RegisterIsolate(result);
+ // Now we enter the isolate. This will ensure we're participating in any
+ // safepointing requests from this point on. Other threads requesting a
+ // safepoint operation will therefore wait until we've stopped.
+ //
+ // Though the [result] isolate is still in a state where no memory has been
+ // allocated, which means it's safe to GC the isolate group until here.
+ Thread::EnterIsolate(result);
+
if (api_flags.is_service_isolate) {
ASSERT(!ServiceIsolate::Exists());
ServiceIsolate::SetServiceIsolate(result);
@@ -2619,14 +2614,15 @@
Dart_IsolateCleanupCallback cleanup = isolate->on_cleanup_callback();
auto callback_data = isolate->init_callback_data_;
- // From this point on the isolate is no longer visited by GC (which is ok,
- // since we're just going to delete it anyway).
- isolate_group->UnregisterIsolate(isolate);
-
// From this point on the isolate doesn't participate in safepointing
// requests anymore.
ASSERT(!Thread::Current()->HasActiveState());
Thread::ExitIsolate(/*isolate_shutdown=*/true);
+ ASSERT(Thread::Current() == nullptr);
+
+ // From this point on the isolate is no longer visited by GC (which is ok,
+ // since we're just going to delete it anyway).
+ isolate_group->UnregisterIsolate(isolate);
// Now it's safe to delete the isolate.
delete isolate;
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index a1d43d6..8b7e148 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -327,8 +327,6 @@
bool ContainsOnlyOneIsolate();
- void RunWithLockedGroup(std::function<void()> fun);
-
void ScheduleInterrupts(uword interrupt_bits);
ThreadRegistry* thread_registry() const { return thread_registry_.get(); }
diff --git a/runtime/vm/port.cc b/runtime/vm/port.cc
index a6d0375..547959d 100644
--- a/runtime/vm/port.cc
+++ b/runtime/vm/port.cc
@@ -59,10 +59,6 @@
return ILLEGAL_PORT;
}
-#if defined(DEBUG)
- handler->CheckAccess();
-#endif
-
const Dart_Port port = AllocatePort();
if (auto ports = handler->ports(ml)) {
ports->Insert(PortHandler::PortSetEntry{port});
