diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc index 62d67a46f42..3c57d4978fe 100644 --- a/runtime/lib/isolate.cc +++ b/runtime/lib/isolate.cc @@ -577,6 +577,8 @@ class IsolateAcquireScope : public ValueObject { return "Isolate is pinned to a different thread already"; case IsolateAcquireResult::BUSY: return "Isolate is busy, running on a different thread"; + case IsolateAcquireResult::HAS_MESSAGE_LOOP_OR_UNAVAILABLE: + return "Isolate has a message loop running or otherwise unavailable."; default: UNREACHABLE(); } @@ -638,13 +640,6 @@ DEFINE_NATIVE_ENTRY(Isolate_runEventLoopSync_, 0, 1) { Dart_Port control_port_id = isolate_control_port.Id(); - if (PortMap::HasEventLoopRunning(control_port_id)) { - const auto& error = - String::Handle(String::New("Isolate has a message loop running.")); - Exceptions::ThrowStateError(error); - UNREACHABLE(); - } - Error& result_error = Error::Handle(); Thread::ExitIsolateGroupAsMutator(/*bypass_safepoint=*/false); { @@ -714,13 +709,6 @@ DEFINE_NATIVE_ENTRY(Isolate_runSync_, 1, 2) { UNREACHABLE(); } } else { - if (PortMap::HasEventLoopRunning(control_port_id)) { - const auto& error = - String::Handle(String::New("Isolate has a message loop running.")); - Exceptions::ThrowStateError(error); - UNREACHABLE(); - } - if (isolate != nullptr) { ASSERT(Thread::Current()->isolate() == isolate); Thread::ExitIsolate(/*isolate_shutdown=*/false); diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index a3ff0aa8947..52b9a1580f2 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -1167,6 +1167,11 @@ class IsolateMessageHandler : public MessageHandler { ErrorPtr HandleLibMessage(const Array& message); MessageStatus ProcessUnhandledException(const Error& result); + + void set_is_scheduled() override { + ASSERT(isolate_ != nullptr); + isolate_->set_is_not_acquirable(); + } Isolate* isolate_; }; diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 9a8d77567a6..e5e6f00ee41 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -1112,6 +1112,7 @@ class Isolate : public IntrusiveDListEntry { void set_message_notify_callback(Dart_MessageNotifyCallback value) { message_notify_callback_.store(value, std::memory_order_release); + is_acquirable_ = false; } void set_on_shutdown_callback(Dart_IsolateShutdownCallback value) { @@ -1301,6 +1302,8 @@ class Isolate : public IntrusiveDListEntry { void clear_is_permanently_pinned_for_testing_only() { is_permanently_pinned_ = false; } + bool is_acquirable() { return is_acquirable_; } + void set_is_not_acquirable() { is_acquirable_ = false; } static void SetCreateGroupCallback(Dart_IsolateGroupCreateCallback cb) { create_group_callback_ = cb; @@ -1709,6 +1712,7 @@ class Isolate : public IntrusiveDListEntry { intptr_t ffi_callback_keep_alive_counter_ = 0; RelaxedAtomic owner_thread_ = OSThread::kInvalidThreadId; bool is_permanently_pinned_ = false; + bool is_acquirable_ = true; ErrorPtr sticky_error_; diff --git a/runtime/vm/message_handler.cc b/runtime/vm/message_handler.cc index 41b114d3885..83b75166bb3 100644 --- a/runtime/vm/message_handler.cc +++ b/runtime/vm/message_handler.cc @@ -100,6 +100,7 @@ bool MessageHandler::Run(ThreadPool* pool, } ASSERT(pool_ == nullptr); pool_ = pool; + set_is_scheduled(); end_callback_ = end_callback; callback_data_ = data; task_running_ = true; diff --git a/runtime/vm/message_handler.h b/runtime/vm/message_handler.h index b45691f8744..294c18d461b 100644 --- a/runtime/vm/message_handler.h +++ b/runtime/vm/message_handler.h @@ -146,7 +146,7 @@ class MessageHandler : public PortHandler { void PostMessage(std::unique_ptr message, bool before_events = false) override; - bool is_scheduled() { return pool_ != nullptr; } + virtual void set_is_scheduled() {} private: template diff --git a/runtime/vm/port.cc b/runtime/vm/port.cc index c2f403e6c38..7e7a2d2b869 100644 --- a/runtime/vm/port.cc +++ b/runtime/vm/port.cc @@ -244,6 +244,10 @@ IsolateAcquireResult PortMap::AcquireIsolateByControlPort(Dart_Port target_port, ASSERT(target_handler != nullptr); auto target_isolate = target_handler->isolate(); + if (!target_handler->isolate()->is_acquirable()) { + return IsolateAcquireResult::HAS_MESSAGE_LOOP_OR_UNAVAILABLE; + } + if (!target_isolate->TryAcquireOwnership()) { return target_isolate->is_permanently_pinned() ? IsolateAcquireResult::PINNED_TO_ANOTHER_THREAD @@ -254,25 +258,6 @@ IsolateAcquireResult PortMap::AcquireIsolateByControlPort(Dart_Port target_port, return IsolateAcquireResult::SUCCESS; } -bool PortMap::HasEventLoopRunning(Dart_Port id) { - Locker ml; // isolates are not exiting while we hold this lock - if (ports_ == nullptr) { - return false; - } - auto it = ports_->TryLookup(id); - if (it == ports_->end()) { - return false; - } - auto target_handler = (*it).handler; - ASSERT(target_handler != nullptr); - auto isolate = target_handler->isolate(); - if (isolate->message_notify_callback() != nullptr) { - return true; - } - auto message_handler = isolate->message_handler(); - return message_handler != nullptr && message_handler->is_scheduled(); -} - #if defined(TESTING) bool PortMap::HasPorts(MessageHandler* handler) { Locker ml; diff --git a/runtime/vm/port.h b/runtime/vm/port.h index d7a7ba980e5..98f76ebc3c6 100644 --- a/runtime/vm/port.h +++ b/runtime/vm/port.h @@ -28,6 +28,7 @@ enum class IsolateAcquireResult { ISOLATE_NOT_AVAILABLE, BUSY, PINNED_TO_ANOTHER_THREAD, + HAS_MESSAGE_LOOP_OR_UNAVAILABLE, }; class PortMap : public AllStatic { @@ -61,8 +62,6 @@ class PortMap : public AllStatic { // Returns true if the port is owned by somebody. static bool IsOwned(Dart_Port id); - static bool HasEventLoopRunning(Dart_Port id); - static IsolateAcquireResult AcquireIsolateByControlPort(Dart_Port target_port, Isolate** p_isolate); diff --git a/tests/ffi/threading_test.dart b/tests/ffi/threading_test.dart index 8c36321fe18..8eeb4805ff9 100644 --- a/tests/ffi/threading_test.dart +++ b/tests/ffi/threading_test.dart @@ -118,7 +118,10 @@ Future testFailToRunOnExitedIsolate() async { () => child.runSync(() { print('child runSync is running'); }), - (e) => e is StateError && e.message.contains("Unable to enter the isolate"), + (e) => + e is StateError && + (e.message.contains("Unable to enter the isolate") || + e.message.contains("Isolate has a message loop running")), ); rp.close(); }