[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>
This commit is contained in:
committed by
Commit Queue
parent
343ffd1ea8
commit
e4e8360eba
+13
-17
@@ -454,11 +454,6 @@ bool IsolateGroup::ContainsOnlyOneIsolate() {
|
||||
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 @@ Isolate* Isolate::InitIsolate(const char* name_prefix,
|
||||
#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 @@ Isolate* Isolate::InitIsolate(const char* name_prefix,
|
||||
// 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 @@ void Isolate::LowLevelCleanup(Isolate* isolate) {
|
||||
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;
|
||||
|
||||
@@ -327,8 +327,6 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
|
||||
|
||||
bool ContainsOnlyOneIsolate();
|
||||
|
||||
void RunWithLockedGroup(std::function<void()> fun);
|
||||
|
||||
void ScheduleInterrupts(uword interrupt_bits);
|
||||
|
||||
ThreadRegistry* thread_registry() const { return thread_registry_.get(); }
|
||||
|
||||
@@ -59,10 +59,6 @@ Dart_Port PortMap::CreatePort(PortHandler* handler) {
|
||||
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});
|
||||
|
||||
Reference in New Issue
Block a user