Skip to content

Commit 35a07b2

Browse files
rmacnak-googleCommit Queue
authored andcommitted
Revert "[vm, isolates] Avoid three-way deadlock during isolate exit."
This reverts commit e4e8360. Reason for revert: #59632 Original change's description: > [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: #59574 > Change-Id: Ia98fabd654c880b253893a0598d2e26ed77f52da > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397660 > Reviewed-by: Alexander Aprelev <[email protected]> > Commit-Queue: Ryan Macnak <[email protected]> Bug: #59574 Change-Id: I529fc140160018c2b6768b108073b2e14ab7bdaa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398422 Reviewed-by: Alexander Aprelev <[email protected]> Commit-Queue: Ryan Macnak <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent 3feb138 commit 35a07b2

File tree

3 files changed

+23
-13
lines changed

3 files changed

+23
-13
lines changed

runtime/vm/isolate.cc

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,11 @@ bool IsolateGroup::ContainsOnlyOneIsolate() {
454454
return isolate_count_ == 0 || isolate_count_ == 1;
455455
}
456456

457+
void IsolateGroup::RunWithLockedGroup(std::function<void()> fun) {
458+
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
459+
fun();
460+
}
461+
457462
void IsolateGroup::UnregisterIsolate(Isolate* isolate) {
458463
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
459464
isolates_.Remove(isolate);
@@ -1845,6 +1850,14 @@ Isolate* Isolate::InitIsolate(const char* name_prefix,
18451850
#undef ISOLATE_METRIC_INIT
18461851
#endif // !defined(PRODUCT)
18471852

1853+
// First we ensure we enter the isolate. This will ensure we're participating
1854+
// in any safepointing requests from this point on. Other threads requesting a
1855+
// safepoint operation will therefore wait until we've stopped.
1856+
//
1857+
// Though the [result] isolate is still in a state where no memory has been
1858+
// allocated, which means it's safe to GC the isolate group until here.
1859+
Thread::EnterIsolate(result);
1860+
18481861
// Setup the isolate message handler.
18491862
result->message_handler_ = new IsolateMessageHandler(result);
18501863

@@ -1872,14 +1885,6 @@ Isolate* Isolate::InitIsolate(const char* name_prefix,
18721885
// to vm-isolate objects, e.g. null)
18731886
isolate_group->RegisterIsolate(result);
18741887

1875-
// Now we enter the isolate. This will ensure we're participating in any
1876-
// safepointing requests from this point on. Other threads requesting a
1877-
// safepoint operation will therefore wait until we've stopped.
1878-
//
1879-
// Though the [result] isolate is still in a state where no memory has been
1880-
// allocated, which means it's safe to GC the isolate group until here.
1881-
Thread::EnterIsolate(result);
1882-
18831888
if (api_flags.is_service_isolate) {
18841889
ASSERT(!ServiceIsolate::Exists());
18851890
ServiceIsolate::SetServiceIsolate(result);
@@ -2614,15 +2619,14 @@ void Isolate::LowLevelCleanup(Isolate* isolate) {
26142619
Dart_IsolateCleanupCallback cleanup = isolate->on_cleanup_callback();
26152620
auto callback_data = isolate->init_callback_data_;
26162621

2622+
// From this point on the isolate is no longer visited by GC (which is ok,
2623+
// since we're just going to delete it anyway).
2624+
isolate_group->UnregisterIsolate(isolate);
2625+
26172626
// From this point on the isolate doesn't participate in safepointing
26182627
// requests anymore.
26192628
ASSERT(!Thread::Current()->HasActiveState());
26202629
Thread::ExitIsolate(/*isolate_shutdown=*/true);
2621-
ASSERT(Thread::Current() == nullptr);
2622-
2623-
// From this point on the isolate is no longer visited by GC (which is ok,
2624-
// since we're just going to delete it anyway).
2625-
isolate_group->UnregisterIsolate(isolate);
26262630

26272631
// Now it's safe to delete the isolate.
26282632
delete isolate;

runtime/vm/isolate.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
327327

328328
bool ContainsOnlyOneIsolate();
329329

330+
void RunWithLockedGroup(std::function<void()> fun);
331+
330332
void ScheduleInterrupts(uword interrupt_bits);
331333

332334
ThreadRegistry* thread_registry() const { return thread_registry_.get(); }

runtime/vm/port.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ Dart_Port PortMap::CreatePort(PortHandler* handler) {
5959
return ILLEGAL_PORT;
6060
}
6161

62+
#if defined(DEBUG)
63+
handler->CheckAccess();
64+
#endif
65+
6266
const Dart_Port port = AllocatePort();
6367
if (auto ports = handler->ports(ml)) {
6468
ports->Insert(PortHandler::PortSetEntry{port});

0 commit comments

Comments
 (0)