Skip to content

Commit 1af75d3

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Add non-null isolate checks in few native runtime calls.
There are native runtime calls that won't work without isolate, but still can be invoked from dart code - from isolategroup callback for example. So rather than crashing, let them throw an exception if they are invoked outside of an isolate. TEST=run_isolate_group_run_test Change-Id: I04a423b2a10f1dc01fc395ede9813f452fd6a37d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443624 Reviewed-by: Liam Appelbe <[email protected]> Commit-Queue: Alexander Aprelev <[email protected]>
1 parent db2fad7 commit 1af75d3

File tree

2 files changed

+67
-12
lines changed

2 files changed

+67
-12
lines changed

runtime/lib/isolate.cc

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,21 @@ DEFINE_NATIVE_ENTRY(Capability_get_hashcode, 0, 1) {
5757
return Smi::New(hash);
5858
}
5959

60+
static void ThrowCantRunWithoutIsolateError() {
61+
const auto& error =
62+
String::Handle(String::New("Only available when running in context of "
63+
"an isolate, rather than isolate group."));
64+
Exceptions::ThrowArgumentError(error);
65+
}
66+
6067
DEFINE_NATIVE_ENTRY(RawReceivePort_factory, 0, 2) {
6168
ASSERT(
6269
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
6370
GET_NON_NULL_NATIVE_ARGUMENT(String, debug_name, arguments->NativeArgAt(1));
71+
if (isolate == nullptr) {
72+
ThrowCantRunWithoutIsolateError();
73+
UNREACHABLE();
74+
}
6475
return isolate->CreateReceivePort(debug_name);
6576
}
6677

@@ -498,6 +509,11 @@ class MessageValidator : private WorkSet {
498509

499510
// TODO(http://dartbug.com/47777): Add support for Finalizers.
500511
DEFINE_NATIVE_ENTRY(Isolate_exit_, 0, 2) {
512+
if (isolate == nullptr) {
513+
ThrowCantRunWithoutIsolateError();
514+
UNREACHABLE();
515+
}
516+
501517
GET_NATIVE_ARGUMENT(SendPort, port, arguments->NativeArgAt(0));
502518
if (!port.IsNull()) {
503519
GET_NATIVE_ARGUMENT(Instance, obj, arguments->NativeArgAt(1));
@@ -534,8 +550,7 @@ DEFINE_NATIVE_ENTRY(Isolate_exit_, 0, 2) {
534550
Exceptions::PropagateError(Error::Cast(validated_result));
535551
UNREACHABLE();
536552
}
537-
PersistentHandle* handle =
538-
isolate->group()->api_state()->AllocatePersistentHandle();
553+
PersistentHandle* handle = group->api_state()->AllocatePersistentHandle();
539554
handle->set_ptr(msg_array);
540555
isolate->bequeath(std::unique_ptr<Bequest>(new Bequest(handle, port.Id())));
541556
}
@@ -1061,6 +1076,11 @@ static const char* String2UTF8(const String& str) {
10611076
}
10621077

10631078
DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 0, 10) {
1079+
if (isolate == nullptr) {
1080+
ThrowCantRunWithoutIsolateError();
1081+
UNREACHABLE();
1082+
}
1083+
10641084
GET_NON_NULL_NATIVE_ARGUMENT(SendPort, port, arguments->NativeArgAt(0));
10651085
GET_NON_NULL_NATIVE_ARGUMENT(String, script_uri, arguments->NativeArgAt(1));
10661086
GET_NON_NULL_NATIVE_ARGUMENT(Closure, closure, arguments->NativeArgAt(2));
@@ -1080,8 +1100,8 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 0, 10) {
10801100
ASSERT(closure_copy_tuple.IsArray());
10811101
ASSERT(
10821102
Object::Handle(zone, Array::Cast(closure_copy_tuple).At(0)).IsClosure());
1083-
closure_tuple_handle =
1084-
isolate->group()->api_state()->AllocatePersistentHandle();
1103+
auto isolate_group = thread->isolate_group();
1104+
closure_tuple_handle = isolate_group->api_state()->AllocatePersistentHandle();
10851105
closure_tuple_handle->set_ptr(closure_copy_tuple.ptr());
10861106

10871107
bool fatal_errors = fatalErrors.IsNull() ? true : fatalErrors.value();
@@ -1107,10 +1127,10 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 0, 10) {
11071127
std::unique_ptr<IsolateSpawnState> state(new IsolateSpawnState(
11081128
port.Id(), String2UTF8(script_uri), closure_tuple_handle, &message_buffer,
11091129
utf8_package_config, paused.value(), fatal_errors, on_exit_port,
1110-
on_error_port, utf8_debug_name, isolate->group()));
1130+
on_error_port, utf8_debug_name, isolate_group));
11111131

1112-
isolate->group()->thread_pool()->Run<SpawnIsolateTask>(isolate,
1113-
std::move(state));
1132+
isolate_group->thread_pool()->Run<SpawnIsolateTask>(isolate,
1133+
std::move(state));
11141134
return Object::null();
11151135
}
11161136

@@ -1146,6 +1166,11 @@ static const char* CanonicalizeUri(Thread* thread,
11461166
}
11471167

11481168
DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 0, 12) {
1169+
if (isolate == nullptr) {
1170+
ThrowCantRunWithoutIsolateError();
1171+
UNREACHABLE();
1172+
}
1173+
11491174
GET_NON_NULL_NATIVE_ARGUMENT(SendPort, port, arguments->NativeArgAt(0));
11501175
GET_NON_NULL_NATIVE_ARGUMENT(String, uri, arguments->NativeArgAt(1));
11511176
GET_NON_NULL_NATIVE_ARGUMENT(Instance, args, arguments->NativeArgAt(2));
@@ -1177,8 +1202,9 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 0, 12) {
11771202
}
11781203

11791204
// Canonicalize the uri with respect to the current isolate.
1205+
auto isolate_group = thread->isolate_group();
11801206
const Library& root_lib =
1181-
Library::Handle(isolate->group()->object_store()->root_library());
1207+
Library::Handle(isolate_group->object_store()->root_library());
11821208
char* error = nullptr;
11831209
const char* canonical_uri = CanonicalizeUri(thread, root_lib, uri, &error);
11841210
if (canonical_uri == nullptr) {
@@ -1204,8 +1230,8 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 0, 12) {
12041230
flags->enable_asserts = enableAsserts.value();
12051231
}
12061232

1207-
isolate->group()->thread_pool()->Run<SpawnIsolateTask>(isolate,
1208-
std::move(state));
1233+
isolate_group->thread_pool()->Run<SpawnIsolateTask>(isolate,
1234+
std::move(state));
12091235
return Object::null();
12101236
}
12111237

@@ -1229,8 +1255,8 @@ DEFINE_NATIVE_ENTRY(Isolate_getPortAndCapabilitiesOfCurrentIsolate, 0, 0) {
12291255
}
12301256

12311257
DEFINE_NATIVE_ENTRY(Isolate_getCurrentRootUriStr, 0, 0) {
1232-
const Library& root_lib =
1233-
Library::Handle(zone, isolate->group()->object_store()->root_library());
1258+
const Library& root_lib = Library::Handle(
1259+
zone, thread->isolate_group()->object_store()->root_library());
12341260
return root_lib.url();
12351261
}
12361262

tests/ffi/run_isolate_group_run_test.dart

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,5 +152,34 @@ main() {
152152
});
153153
Expect.equals("foo bar", string_foo);
154154

155+
Expect.throws(
156+
() {
157+
IsolateGroup.runSync(() {
158+
ReceivePort();
159+
});
160+
},
161+
(e) =>
162+
e is ArgumentError &&
163+
e.toString().contains("Only available when running in context"),
164+
);
165+
166+
Expect.throws(() {
167+
IsolateGroup.runSync(() {
168+
Isolate.exit();
169+
});
170+
}, (e) => e.toString().contains("Attempt to access isolate static field"));
171+
172+
Expect.throws(() {
173+
IsolateGroup.runSync(() {
174+
Isolate.spawn((_) {}, null);
175+
});
176+
}, (e) => e.toString().contains("Attempt to access isolate static field"));
177+
178+
Expect.throws(() {
179+
IsolateGroup.runSync(() {
180+
Isolate.spawnUri(Uri.parse("http://127.0.0.1"), [], (_) {});
181+
});
182+
}, (e) => e.toString().contains("Attempt to access isolate static field"));
183+
155184
print("All tests completed :)");
156185
}

0 commit comments

Comments
 (0)