[Service] Support isolate/root in package:dart_runtime_service

Adds isSystemIsolate in the isolate startup message sent to the
VM service, allowing for the service to identify the first non-system
isolate as the root isolate.

TEST=Existing tests

CoreLibraryReviewExempt: dart:vmservice is internal.
Change-Id: I0a982a1fc06bd0be0426ad9d1401e89375cbed40
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500000
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>

pkg/dart_runtime_service/lib/src/isolate_manager.dart

@@ -87,6 +87,12 @@ abstract base class IsolateManager {
   /// Used to support the `isolates/root` isolate ID.
   int? _rootIsolateId;
 
+  @protected
+  int? get rootIsolateId => _rootIsolateId;
+
+  @protected
+  set rootIsolateId(int? value) => _rootIsolateId = value;
+
   @mustCallSuper
   Future<void> shutdown() async {
     _logger.info('Shutting down.');
@@ -103,10 +109,12 @@ abstract base class IsolateManager {
   });
 
   /// Initializes state for a newly started isolate.
-  void isolateStarted({required RunningIsolate isolate}) {
+  void isolateStarted({
+    required RunningIsolate isolate,
+    bool isSystemIsolate = false,
+  }) {
     _logger.info('Starting isolate: $isolate');
-    if (_rootIsolateId == null) {
-      // TODO(bkonyi): ensure this is a non-system isolate
+    if (_rootIsolateId == null && !isSystemIsolate) {
       _logger.info('$isolate is the root isolate.');
       _rootIsolateId = isolate.id;
     }

pkg/dart_runtime_service/test/isolate_manager_test.dart

@@ -0,0 +1,52 @@
+import 'dart:async';
+import 'package:dart_runtime_service/dart_runtime_service.dart';
+import 'package:test/test.dart';
+
+base class TestRunningIsolate extends RunningIsolate {
+  TestRunningIsolate({required super.id, required super.name});
+}
+
+base class TestIsolateManager extends IsolateManager {
+  @override
+  Future<RpcResponse> sendToIsolate({
+    required String method,
+    required Map<String, Object?> params,
+  }) {
+    throw UnimplementedError();
+  }
+}
+
+void main() {
+  group('IsolateManager', () {
+    test('ignores system isolates when assigning the root isolate', () async {
+      final manager = TestIsolateManager();
+
+      // A system isolate starts first.
+      final systemIsolate = TestRunningIsolate(id: 1, name: 'vm-service');
+      manager.isolateStarted(isolate: systemIsolate, isSystemIsolate: true);
+
+      // The root isolate id should not be set yet.
+      expect(
+        () => manager.lookupIsolateFromParams(
+          method: 'foo',
+          params: {'isolateId': 'isolates/root'},
+        ),
+        throwsA(isA<Exception>()),
+      );
+
+      // A non-system isolate starts next.
+      final normalIsolate = TestRunningIsolate(id: 2, name: 'main');
+      manager.isolateStarted(isolate: normalIsolate);
+
+      await Future<void>.delayed(const Duration(milliseconds: 10));
+
+      // Now the root isolate ID should point to the normal isolate.
+      final rootIsolate = manager.lookupIsolateFromParams(
+        method: 'foo',
+        params: {'isolateId': 'isolates/root'},
+      );
+      expect(rootIsolate, isNotNull);
+      expect(rootIsolate!.id, equals(2));
+    });
+  });
+}

pkg/dart_runtime_service_vm/bin/vm_service_entrypoint.dart

@@ -71,10 +71,19 @@ final _isolateRegistrationStreamController = StreamController<VmRunningIsolate>(
 
 @entrypoint
 // ignore: unused_element
-void _registerIsolate(int portId, SendPort sendPort, String name) =>
-    _isolateRegistrationStreamController.sink.add(
-      VmRunningIsolate(id: portId, name: name, sendPort: sendPort),
-    );
+void _registerIsolate(
+  int portId,
+  SendPort sendPort,
+  String name,
+  bool isSystemIsolate,
+) => _isolateRegistrationStreamController.sink.add(
+  VmRunningIsolate(
+    id: portId,
+    name: name,
+    sendPort: sendPort,
+    isSystemIsolate: isSystemIsolate,
+  ),
+);
 
 // ignore: unused_element
 StreamSubscription<ProcessSignal>? _signalSubscription;

pkg/dart_runtime_service_vm/lib/dart_runtime_service_vm.dart

@@ -271,11 +271,23 @@ class DartRuntimeServiceVMBackend
             final SendPort sendPort,
             final String name,
           ]
-          when opcode == _kIsolateStartupMessageId ||
-              opcode == _kIsolateShutdownMessageId:
-        // This is a message informing us of the birth or death of an
-        // isolate.
+          when opcode == _kIsolateShutdownMessageId:
         _isolateControlMessageHandler(opcode, portId, sendPort, name);
+      case [
+            final int opcode,
+            final int portId,
+            final SendPort sendPort,
+            final String name,
+            final bool isSystemIsolate,
+          ]
+          when opcode == _kIsolateStartupMessageId:
+        _isolateControlMessageHandler(
+          opcode,
+          portId,
+          sendPort,
+          name,
+          isSystemIsolate: isSystemIsolate,
+        );
       default:
         _logger.warning(
           'Internal vm-service error: ignoring illegal message: $message',
@@ -311,14 +323,16 @@ class DartRuntimeServiceVMBackend
     int code,
     int portId,
     SendPort sp,
-    String name,
-  ) {
+    String name, {
+    bool isSystemIsolate = false,
+  }) {
     switch (code) {
       case _kIsolateStartupMessageId:
         isolateManager.onIsolateStartupMessage(
           id: portId,
           sendPort: sp,
           name: name,
+          isSystemIsolate: isSystemIsolate,
         );
       case _kIsolateShutdownMessageId:
         isolateManager.onIsolateShutdownMessage(id: portId);

pkg/dart_runtime_service_vm/lib/src/vm_isolate_manager.dart

@@ -17,11 +17,15 @@ final class VmRunningIsolate extends RunningIsolate {
     required super.id,
     required super.name,
     required this.sendPort,
+    required this.isSystemIsolate,
   });
 
   /// The port used to send service requests to the isolate within the VM.
   final SendPort sendPort;
 
+  /// Whether this is a system isolate.
+  final bool isSystemIsolate;
+
   /// The set of ports for outstanding requests that are used by the VM to send
   /// responses.
   final outstandingRequestPorts = <RawReceivePort>{};
@@ -64,10 +68,16 @@ final class VmIsolateManager extends IsolateManager {
     required int id,
     required SendPort sendPort,
     required String name,
+    required bool isSystemIsolate,
   }) {
-    final isolate = VmRunningIsolate(id: id, name: name, sendPort: sendPort);
+    final isolate = VmRunningIsolate(
+      id: id,
+      name: name,
+      sendPort: sendPort,
+      isSystemIsolate: isSystemIsolate,
+    );
     _logger.info('Isolate startup message received for $isolate');
-    isolateStarted(isolate: isolate);
+    isolateStarted(isolate: isolate, isSystemIsolate: isSystemIsolate);
   }
 
   /// Reports that an isolate is shutting down based on a message over the

runtime/lib/vmservice.cc

@@ -35,17 +35,20 @@ class RegisterRunningIsolatesVisitor : public IsolateVisitor {
   virtual void VisitIsolate(Isolate* isolate) {
     isolate_ports_.Add(isolate->main_port());
     isolate_names_.Add(&String::Handle(zone_, String::New(isolate->name())));
+    isolate_is_system_.Add(Isolate::IsSystemIsolate(isolate));
     isolate->set_is_service_registered(true);
   }
 
   void RegisterIsolates() {
-    ServiceIsolate::RegisterRunningIsolates(isolate_ports_, isolate_names_);
+    ServiceIsolate::RegisterRunningIsolates(isolate_ports_, isolate_names_,
+                                            isolate_is_system_);
   }
 
  private:
   Zone* zone_;
   GrowableArray<Dart_Port> isolate_ports_;
   GrowableArray<const String*> isolate_names_;
+  GrowableArray<bool> isolate_is_system_;
   Function& register_function_;
   Isolate* service_isolate_;
 };

runtime/vm/service_isolate.cc

@@ -70,15 +70,25 @@ bool ServiceIsolate::SendServiceControlMessage(Thread* thread,
   cname.type = Dart_CObject_kString;
   cname.value.as_string = const_cast<char*>(name);
 
-  Dart_CObject* values[4];
+  Dart_CObject cis_system;
+  bool is_startup = (code == VM_SERVICE_ISOLATE_STARTUP_MESSAGE_ID);
+  if (is_startup) {
+    cis_system.type = Dart_CObject_kBool;
+    cis_system.value.as_bool = Isolate::IsSystemIsolate(thread->isolate());
+  }
+
+  Dart_CObject* values[5];
   values[0] = &ccode;
   values[1] = &port_int;
   values[2] = &send_port;
   values[3] = &cname;
+  if (is_startup) {
+    values[4] = &cis_system;
+  }
 
   Dart_CObject message;
   message.type = Dart_CObject_kArray;
-  message.value.as_array.length = 4;
+  message.value.as_array.length = is_startup ? 5 : 4;
   message.value.as_array.values = values;
 
   return PortMap::PostMessage(WriteApiMessage(thread->zone(), &message, port_,
@@ -630,7 +640,8 @@ void ServiceIsolate::BootVmServiceLibrary() {
 
 void ServiceIsolate::RegisterRunningIsolates(
     const GrowableArray<Dart_Port>& isolate_ports,
-    const GrowableArray<const String*>& isolate_names) {
+    const GrowableArray<const String*>& isolate_names,
+    const GrowableArray<bool>& isolate_is_system) {
   auto thread = Thread::Current();
   auto zone = thread->zone();
 
@@ -653,10 +664,11 @@ void ServiceIsolate::RegisterRunningIsolates(
 
   Integer& port_int = Integer::Handle(zone);
   SendPort& send_port = SendPort::Handle(zone);
-  Array& args = Array::Handle(zone, Array::New(3));
+  Array& args = Array::Handle(zone, Array::New(4));
   Object& result = Object::Handle(zone);
 
   ASSERT(isolate_ports.length() == isolate_names.length());
+  ASSERT(isolate_ports.length() == isolate_is_system.length());
   for (intptr_t i = 0; i < isolate_ports.length(); ++i) {
     const Dart_Port port_id = isolate_ports[i];
     const String& name = *isolate_names[i];
@@ -666,6 +678,7 @@ void ServiceIsolate::RegisterRunningIsolates(
     args.SetAt(0, port_int);
     args.SetAt(1, send_port);
     args.SetAt(2, name);
+    args.SetAt(3, Bool::Get(isolate_is_system[i]));
     result = DartEntry::InvokeFunction(register_function_, args);
     if (FLAG_trace_service) {
       OS::PrintErr("vm-service: Isolate %s %" Pd64 " registered.\n",

runtime/vm/service_isolate.h

@@ -55,7 +55,8 @@ class ServiceIsolate : public AllStatic {
 
   static void RegisterRunningIsolates(
       const GrowableArray<Dart_Port>& isolate_ports,
-      const GrowableArray<const String*>& isolate_names);
+      const GrowableArray<const String*>& isolate_names,
+      const GrowableArray<bool>& isolate_is_system);
 
   static void RequestServerInfo(const SendPort& sp);
   static void ControlWebServer(const SendPort& sp,

sdk/lib/vmservice/vmservice.dart

@@ -476,7 +476,8 @@ class VMService extends MessageRouter {
         _serverMessageHandler(opcode, sendPort, enable, silenceOutput);
         return;
       }
-      if (message case [int opcode, int portId, SendPort sendPort, String name]
+      if (message
+          case [int opcode, int portId, SendPort sendPort, String name, ...]
           when opcode == Constants.ISOLATE_STARTUP_MESSAGE_ID ||
               opcode == Constants.ISOLATE_SHUTDOWN_MESSAGE_ID) {
         // This is a message informing us of the birth or death of an
@@ -802,8 +803,12 @@ RawReceivePort boot() {
 
 @pragma('vm:entry-point', !bool.fromEnvironment('dart.vm.product'))
 // ignore: unused_element
-void _registerIsolate(int port_id, SendPort sp, String name) =>
-    VMService().runningIsolates.isolateStartup(port_id, sp, name);
+void _registerIsolate(
+  int port_id,
+  SendPort sp,
+  String name,
+  bool isSystemIsolate,
+) => VMService().runningIsolates.isolateStartup(port_id, sp, name);
 
 /// Notify the VM that the service is running.
 @pragma("vm:external-name", "VMService_OnStart")