[Service] Add support for specifying bind address to dart_runtime_service_vm

Also adds a Pool to protect against race conditions around managing the
HttpServer.

TEST=Manual

Change-Id: If748b6e5beb59c0b853e16477f0dac96ffafcaf3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499760
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>

pkg/dart_runtime_service/lib/src/dart_runtime_service.dart

@@ -8,6 +8,7 @@ import 'dart:io';
 
 import 'package:logging/logging.dart';
 import 'package:meta/meta.dart';
+import 'package:pool/pool.dart';
 import 'package:shelf/shelf.dart' as shelf;
 import 'package:shelf/shelf_io.dart' as io;
 import 'package:stream_channel/stream_channel.dart';
@@ -141,7 +142,7 @@ class DartRuntimeService {
     clientsGetter: () => clients,
   );
 
-  // TODO(bkonyi): this should be protected by a mutex.
+  final _serverLock = Pool(1);
   HttpServer? _server;
 
   /// Returns true if the HTTP server is active.
@@ -176,7 +177,9 @@ class DartRuntimeService {
   Future<void> shutdown() async {
     await backend.clearState();
     await backend.shutdown();
-    await _shutdownServer();
+    try {
+      await _shutdownServer();
+    } on DartRuntimeServiceServerNotRunning catch (_) {}
     await clientManager.shutdown();
     Logger.root.clearListeners();
   }
@@ -198,7 +201,6 @@ class DartRuntimeService {
       );
       silenceServiceOutput = silenceOutput;
     }
-    // TODO(bkonyi): verify there's no race conditions
     if (!enable && isServerRunning) {
       await _shutdownServer();
     } else if (enable && !isServerRunning) {
@@ -209,7 +211,6 @@ class DartRuntimeService {
   /// Toggles the state of the HTTP server, enabling it if it's not running and
   /// disabling it if it is running.
   Future<void> toggleServer() async {
-    // TODO(bkonyi): verify there's no race conditions
     if (isServerRunning) {
       await _shutdownServer();
     } else {
@@ -230,72 +231,80 @@ class DartRuntimeService {
     );
   }
 
-  Future<void> _startServer() async {
-    if (_server != null) {
-      _logger.warning(
-        "Attempted to start the HTTP server, but it's already running.",
-      );
-      throw const DartRuntimeServiceServerAlreadyRunning();
-    }
-    // TODO(bkonyi): support IPv6
-    final host = InternetAddress.loopbackIPv4.host;
+  Future<void> _startServer() {
+    return _serverLock.withResource(() async {
+      if (_server != null) {
+        _logger.warning(
+          "Attempted to start the HTTP server, but it's already running.",
+        );
+        throw const DartRuntimeServiceServerAlreadyRunning();
+      }
+      final hostStr = config.host ?? InternetAddress.loopbackIPv4.host;
+      final host =
+          InternetAddress.tryParse(hostStr) ??
+          (await InternetAddress.lookup(hostStr)).first;
 
-    _logger.info('Starting the Dart Runtime Service.');
-    late String errorMessage;
-    final server = await runZonedGuarded(
-      () async {
-        try {
-          final handlers = _handlers();
-          _logger.info('Attempting to bind to $host:${config.port}');
-          return await io.serve(handlers, host, config.port);
-        } on SocketException catch (e) {
-          errorMessage = e.message;
-          if (e.osError != null) {
-            errorMessage += ' (${e.osError!.message})';
+      _logger.info('Starting the Dart Runtime Service.');
+      late String errorMessage;
+      final server = await runZonedGuarded(
+        () async {
+          try {
+            final handlers = _handlers();
+            _logger.info(
+              'Attempting to bind to ${host.address}:${config.port}',
+            );
+            return await io.serve(handlers, host, config.port);
+          } on SocketException catch (e) {
+            errorMessage = e.message;
+            if (e.osError != null) {
+              errorMessage += ' (${e.osError!.message})';
+            }
+            errorMessage += ': ${e.address?.host}:${e.port}';
+            return null;
           }
-          errorMessage += ': ${e.address?.host}:${e.port}';
-          return null;
-        }
-      },
-      (e, st) {
-        _logger.warning('Asynchronous error: $e\n$st');
-      },
-    );
+        },
+        (e, st) {
+          _logger.warning('Asynchronous error: $e\n$st');
+        },
+      );
 
-    if (server == null) {
-      final message = 'Failed to start server: $errorMessage';
-      _logger.warning(message);
-      throw DartRuntimeServiceFailedToStartException(message: errorMessage);
-    }
+      if (server == null) {
+        final message = 'Failed to start server: $errorMessage';
+        _logger.warning(message);
+        throw DartRuntimeServiceFailedToStartException(message: errorMessage);
+      }
 
-    _server = server;
-    _uri = Uri(
-      scheme: 'ws',
-      host: host,
-      port: server.port,
-      path: authCode != null ? '/$authCode' : '',
-    );
-    await backend.onServerStarted(httpUri: httpUri, wsUri: uri);
-    _logger.info(
-      'Dart Runtime Service HTTP server started successfully and is listening '
-      'at $uri.',
-    );
+      _server = server;
+      _uri = Uri(
+        scheme: 'ws',
+        host: host.host,
+        port: server.port,
+        path: authCode != null ? '/$authCode' : '',
+      );
+      await backend.onServerStarted(httpUri: httpUri, wsUri: uri);
+      _logger.info(
+        'Dart Runtime Service HTTP server started successfully and is '
+        'listening at $uri.',
+      );
+    });
   }
 
-  Future<void> _shutdownServer() async {
-    final server = _server;
-    if (server == null) {
-      _logger.warning(
-        "Attempting to shut down the HTTP server, but it's not "
-        'running.',
-      );
-      throw const DartRuntimeServiceServerNotRunning();
-    }
-    _logger.info('Dart Runtime Service HTTP server is shutting down.');
-    _server = null;
-    _uri = null;
-    await server.close();
-    await backend.onServerShutdown();
+  Future<void> _shutdownServer() {
+    return _serverLock.withResource(() async {
+      final server = _server;
+      if (server == null) {
+        _logger.warning(
+          "Attempting to shut down the HTTP server, but it's not "
+          'running.',
+        );
+        throw const DartRuntimeServiceServerNotRunning();
+      }
+      _logger.info('Dart Runtime Service HTTP server is shutting down.');
+      _server = null;
+      _uri = null;
+      await server.close();
+      await backend.onServerShutdown();
+    });
   }
 
   /// Send a [StreamEvent] to subscribed clients.

pkg/dart_runtime_service/lib/src/dart_runtime_service_options.dart

@@ -15,6 +15,7 @@ class DartRuntimeServiceOptions {
     this.autoStart = true,
     this.serveDevTools = false,
     this.enableServicePortFallback = false,
+    this.host,
   });
 
   /// If true, enables log output for the service.
@@ -52,6 +53,11 @@ class DartRuntimeServiceOptions {
   /// is unavailable.
   final bool enableServicePortFallback;
 
+  /// The host the service should attempt to bind to.
+  ///
+  /// If null, defaults to loopback IPv4 address.
+  final String? host;
+
   DartRuntimeServiceOptions copyWith({
     bool? enableLogging,
     int? port,
@@ -61,6 +67,7 @@ class DartRuntimeServiceOptions {
     bool? autoStart,
     bool? serveDevTools,
     bool? enableServicePortFallback,
+    String? host,
   }) {
     return DartRuntimeServiceOptions(
       enableLogging: enableLogging ?? this.enableLogging,
@@ -72,6 +79,7 @@ class DartRuntimeServiceOptions {
       serveDevTools: serveDevTools ?? this.serveDevTools,
       enableServicePortFallback:
           enableServicePortFallback ?? this.enableServicePortFallback,
+      host: host ?? this.host,
     );
   }
 }

pkg/dart_runtime_service/pubspec.yaml

@@ -14,6 +14,7 @@ dependencies:
   json_rpc_2: any
   logging: any
   meta: any
+  pool: any
   shelf: any
   shelf_web_socket: any
   sse: any

pkg/dart_runtime_service/test/server_configuration_test.dart

@@ -34,5 +34,16 @@ void main() {
         throwsFailedToStartException,
       );
     });
+
+    test('successfully binds to loopback IPv6 when configured', () async {
+      final service = await createDartRuntimeServiceForTest(
+        config: const DartRuntimeServiceOptions(
+          enableLogging: true,
+          host: '::1',
+        ),
+      );
+      expect(service.uri.host, equals('::1'));
+      expect(service.isServerRunning, true);
+    });
   });
 }

pkg/dart_runtime_service_vm/bin/vm_service_entrypoint.dart

@@ -93,7 +93,6 @@ bool _printDtd = false;
 
 File? _residentCompilerInfoFile;
 
-@entrypoint
 /// Sets the resident compiler info file, which is used to configure the
 /// service to utilize a resident compiler.
 ///
@@ -101,6 +100,7 @@ File? _residentCompilerInfoFile;
 /// was supplied on the command line, the CLI argument should be forwarded as
 /// the argument to [residentCompilerInfoFilePathArgumentFromCli]. If neither
 /// option was supplied, the argument to this parameter should be null.
+@entrypoint
 // ignore: unused_element
 void _populateResidentCompilerInfoFile(
   String? residentCompilerInfoFilePathArgumentFromCli,
@@ -124,6 +124,7 @@ Future<void> main([List<String> args = const []]) async {
       autoStart: _autoStart,
       serveDevTools: _serveDevtools,
       enableServicePortFallback: _enableServicePortFallback,
+      host: _ip,
     ),
     backendBuilder: (frontend) => DartRuntimeServiceVMBackend(
       frontend: frontend,