Skip to content

Commit 0f3b092

Browse files
authored
Use the Dart isolate ownership API on the root isolate (#163703)
The isolate ownership API was [introduced recently](https://dart-review.googlesource.com/c/sdk/+/407700) to solve [some deadlock bugs](dart-lang/native#1908) in native callbacks. A native callback is a call from native code into a Dart function. Currently all such callbacks must run that Dart function in the isolate that created the callback (called the target isolate). The only native callback primitives at the moment are `NativeCallable.isolateLocal` (blocking, but must be invoked from the same thread as the target isolate, and the target isolate must be currently entered on that thread) and `NativeCallable.listener` (non-blocking, can be invoked from any thread). To build blocking callbacks that can be called from any thread, we can use a `NativeCallable.listener`, and use a synchronization object like a mutex or a condition variable to block until the callback is complete. However, if we try to do this on the thread that is currently entered in the target isolate, we will deadlock: we invoke the listener, a message is sent to the target isolate, and we block waiting for the message to be handled, so we never pass control flow back to the isolate to handle the message, and never stop waiting. To fix this deadlock, Ffigen and Jnigen both have a mechanism that checks if we're on the target isolate's thread first: - If the native caller is already on the same thread as the target isolate, and the target isolate is entered: - Call the Dart function directly using `NativeCallable.isolateLocal` or similar - Otherwise, if the native caller is coming from a different thread: - Call the Dart function asynchronously using `NativeCallable.listener` or similar - Block until the callback finishes However, this neglects the case where we're on the target isolate's thread, but not entered into the isolate. This case happens in Flutter when the callback is invoked from the UI thread (or the platform thread when thread merging is enabled), and the target isolate is the root isolate. When the native callback is invoked, the root isolate is not entered, so we hit the second case: we send a message to the root isolate, and block to wait for a response. Since the root isolate is exclusively run on the UI thread, and we're blocking the UI thread, the message will never be handled, and we deadlock. The isolate ownership API fixes this by allowing the embedder to inform the VM that it will run a particular isolate exclusively on a particular thread, using `Dart_SetCurrentThreadOwnsIsolate`. Other native code can then query that ownership using `Dart_GetCurrentThreadOwnsIsolate`. This lets us add a third case to our conditional: - If the native caller is on the thread that is currently entered in the target isolate: - Call the Dart function directly using `NativeCallable.isolateLocal` or similar - Otherwise, if the native caller is on the thread that owns the target isolate - Enter the target isolate - Call the Dart function directly using `NativeCallable.isolateLocal `or similar - Exit the target isolate - Otherwise, the native caller is coming from an unrelated thread: - Call the Dart function asynchronously using `NativeCallable.listener` or similar - Block until the callback finishes **Note:** We don't need to set the ownership of VM managed threads, because they run in a thread pool exclusively used by the VM, so there's no way for native code to be executed on the thread (except by FFI, in which case we're entered into the isolate anyway). We only need this for Flutter's root isolate because work can be sent to the UI thread/platform thread using OS specific APIs like Android's `Looper.getMainLooper()`.
1 parent 59b3923 commit 0f3b092

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

engine/src/flutter/runtime/dart_isolate.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,15 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRunningRootIsolate(
153153
return {};
154154
}
155155

156-
if (settings.root_isolate_create_callback) {
157-
// Isolate callbacks always occur in isolate scope and before user code has
158-
// had a chance to run.
156+
{
159157
tonic::DartState::Scope scope(isolate.get());
160-
settings.root_isolate_create_callback(*isolate.get());
158+
Dart_SetCurrentThreadOwnsIsolate();
159+
160+
if (settings.root_isolate_create_callback) {
161+
// Isolate callbacks always occur in isolate scope and before user code
162+
// has had a chance to run.
163+
settings.root_isolate_create_callback(*isolate.get());
164+
}
161165
}
162166

163167
if (root_isolate_create_callback) {

engine/src/flutter/runtime/dart_isolate_unittests.cc

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,61 @@ TEST_F(DartIsolateTest, PlatformIsolateMainThrowsError) {
11131113
// root isolate will be auto-shutdown
11141114
}
11151115

1116+
TEST_F(DartIsolateTest, RootIsolateIsOwnedByMainThread) {
1117+
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
1118+
auto settings = CreateSettingsForFixture();
1119+
auto vm_ref = DartVMRef::Create(settings);
1120+
ASSERT_TRUE(vm_ref);
1121+
auto vm_data = vm_ref.GetVMData();
1122+
ASSERT_TRUE(vm_data);
1123+
TaskRunners task_runners(GetCurrentTestName(), //
1124+
GetCurrentTaskRunner(), //
1125+
GetCurrentTaskRunner(), //
1126+
GetCurrentTaskRunner(), //
1127+
GetCurrentTaskRunner() //
1128+
);
1129+
1130+
auto isolate_configuration =
1131+
IsolateConfiguration::InferFromSettings(settings);
1132+
1133+
UIDartState::Context context(task_runners);
1134+
context.advisory_script_uri = "main.dart";
1135+
context.advisory_script_entrypoint = "main";
1136+
auto weak_isolate = DartIsolate::CreateRunningRootIsolate(
1137+
vm_data->GetSettings(), // settings
1138+
vm_data->GetIsolateSnapshot(), // isolate snapshot
1139+
nullptr, // platform configuration
1140+
DartIsolate::Flags{}, // flags
1141+
nullptr, // root_isolate_create_callback
1142+
settings.isolate_create_callback, // isolate create callback
1143+
settings.isolate_shutdown_callback, // isolate shutdown callback
1144+
"main", // dart entrypoint
1145+
std::nullopt, // dart entrypoint library
1146+
{}, // dart entrypoint arguments
1147+
std::move(isolate_configuration), // isolate configuration
1148+
context // engine context
1149+
);
1150+
auto root_isolate = weak_isolate.lock();
1151+
1152+
Dart_Port main_port;
1153+
{
1154+
tonic::DartState::Scope scope(root_isolate.get());
1155+
main_port = Dart_GetMainPortId();
1156+
1157+
ASSERT_TRUE(Dart_GetCurrentThreadOwnsIsolate(main_port));
1158+
}
1159+
1160+
ASSERT_TRUE(Dart_GetCurrentThreadOwnsIsolate(main_port));
1161+
1162+
std::thread([main_port]() {
1163+
ASSERT_FALSE(Dart_GetCurrentThreadOwnsIsolate(main_port));
1164+
}).join();
1165+
1166+
ASSERT_TRUE(root_isolate->Shutdown());
1167+
1168+
ASSERT_FALSE(Dart_GetCurrentThreadOwnsIsolate(main_port));
1169+
}
1170+
11161171
} // namespace testing
11171172
} // namespace flutter
11181173

0 commit comments

Comments
 (0)