Reland "[ Service / DDS ] Remove VM service polling logic, add --enable-service-fallback-port support to DDS"

This reverts commit d4f71515c7.

TEST=CQ

Change-Id: I180f9d045870a1ff5f7f3f82b6f4782e18c18cfc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237440
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/dartdev/lib/src/commands/run.dart

@@ -222,6 +222,8 @@ class RunCommand extends DartdevCommand {
       final bool debugDds = args['debug-dds'];
 
       bool disableServiceAuthCodes = args['disable-service-auth-codes'];
+      final bool enableServicePortFallback =
+          args['enable-service-port-fallback'];
 
       // If the user wants to start a debugging session we need to do some extra
       // work and spawn a Dart Development Service (DDS) instance. DDS is a VM
@@ -237,6 +239,7 @@ class RunCommand extends DartdevCommand {
           disableServiceAuthCodes,
           launchDevTools,
           debugDds,
+          enableServicePortFallback,
         )) {
           return errorExitCode;
         }
@@ -274,6 +277,7 @@ class _DebuggingSession {
     bool disableServiceAuthCodes,
     bool enableDevTools,
     bool debugDds,
+    bool enableServicePortFallback,
   ) async {
     final sdkDir = dirname(sdk.dart);
     final fullSdk = sdkDir.endsWith('bin');
@@ -303,6 +307,7 @@ class _DebuggingSession {
         enableDevTools.toString(),
         devToolsBinaries,
         debugDds.toString(),
+        enableServicePortFallback.toString(),
       ],
       mode: ProcessStartMode.detachedWithStdio,
     );

pkg/dartdev/test/commands/run_test.dart

@@ -333,6 +333,25 @@ void main(List<String> args) => print("$b $args");
     expect(result.exitCode, 0);
   });
 
+  test('--enable-service-port-fallback', () async {
+    final p = project(mainSrc: '''void main() {}''');
+    final server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
+    final result = await p.run(
+      [
+        'run',
+        '--enable-vm-service=${server.port}',
+        '--enable-service-port-fallback',
+        p.relativeFilePath,
+      ],
+    );
+    final regexp = RegExp(
+        r'Observatory listening on http:\/\/127.0.0.1:(\d*)\/[a-zA-Z0-9_-]+=\/\n.*');
+    final vmServicePort =
+        int.parse(regexp.firstMatch(result.stdout)!.group(1)!);
+    expect(server.port != vmServicePort, isTrue);
+    await server.close();
+  });
+
   test('without verbose CFE info', () async {
     final p = project(mainSrc: '''void main() {}''');
 

pkg/dds/bin/dds.dart

@@ -18,6 +18,7 @@ import 'package:dds/dds.dart';
 ///   - Start DevTools
 ///   - DevTools build directory
 ///   - Enable logging
+///   - Enable service port fallback
 Future<void> main(List<String> args) async {
   if (args.isEmpty) return;
 
@@ -46,6 +47,7 @@ Future<void> main(List<String> args) async {
     devToolsBuildDirectory = Uri.file(args[5]);
   }
   final logRequests = args[6] == 'true';
+  final enableServicePortFallback = args[7] == 'true';
   try {
     // TODO(bkonyi): add retry logic similar to that in vmservice_server.dart
     // See https://github.com/dart-lang/sdk/issues/43192.
@@ -61,6 +63,7 @@ Future<void> main(List<String> args) async {
             )
           : null,
       logRequests: logRequests,
+      enableServicePortFallback: enableServicePortFallback,
     );
     stderr.write(json.encode({
       'state': 'started',

pkg/dds/lib/dds.dart

@@ -37,11 +37,15 @@ abstract class DartDevelopmentService {
   ///
   /// [ipv6] controls whether or not DDS is served via IPv6. IPv4 is enabled by
   /// default.
+  ///
+  /// If [enablesServicePortFallback] is enabled, DDS will attempt to bind to any
+  /// available port if the specified port is unavailable.
   static Future<DartDevelopmentService> startDartDevelopmentService(
     Uri remoteVmServiceUri, {
     Uri? serviceUri,
     bool enableAuthCodes = true,
     bool ipv6 = false,
+    bool enableServicePortFallback = false,
     List<String> cachedUserTags = const [],
     DevToolsConfiguration? devToolsConfiguration,
     bool logRequests = false,
@@ -84,6 +88,7 @@ abstract class DartDevelopmentService {
       ipv6,
       devToolsConfiguration,
       logRequests,
+      enableServicePortFallback,
     );
     await service.startService();
     return service;

pkg/dds/lib/src/dds_impl.dart

@@ -59,6 +59,7 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
     this._ipv6,
     this._devToolsConfiguration,
     this.shouldLogRequests,
+    this._enableServicePortFallback,
   ) {
     _clientManager = ClientManager(this);
     _expressionEvaluator = ExpressionEvaluator(this);
@@ -136,7 +137,7 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
     final host = uri?.host ??
         (_ipv6 ? InternetAddress.loopbackIPv6 : InternetAddress.loopbackIPv4)
             .host;
-    final port = uri?.port ?? 0;
+    var port = uri?.port ?? 0;
     var pipeline = const Pipeline();
     if (shouldLogRequests) {
       pipeline = pipeline.addMiddleware(
@@ -155,16 +156,25 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
     late String errorMessage;
     final tmpServer = await runZonedGuarded(
       () async {
-        try {
-          return await io.serve(handler, host, port);
-        } on SocketException catch (e) {
-          errorMessage = e.message;
-          if (e.osError != null) {
-            errorMessage += ' (${e.osError!.message})';
+        Future<HttpServer?> startServer() async {
+          try {
+            return await io.serve(handler, host, port);
+          } on SocketException catch (e) {
+            if (_enableServicePortFallback && port != 0) {
+              // Try again, this time with a random port.
+              port = 0;
+              return await startServer();
+            }
+            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;
         }
+
+        return await startServer();
       },
       (error, stack) {
         if (shouldLogRequests) {
@@ -387,6 +397,7 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService {
   String? get authCode => _authCode;
   String? _authCode;
 
+  final bool _enableServicePortFallback;
   final bool shouldLogRequests;
 
   Uri get remoteVmServiceUri => _remoteVmServiceUri;

runtime/bin/main_options.cc

@@ -666,6 +666,10 @@ bool Options::ParseArguments(int argc,
         if (implicitly_use_dart_dev && Options::vm_service_auth_disabled()) {
           dart_options->AddArgument("--disable-service-auth-codes");
         }
+        if (implicitly_use_dart_dev &&
+            Options::enable_service_port_fallback()) {
+          dart_options->AddArgument("--enable-service-port-fallback");
+        }
       }
       first_option = false;
     }

sdk/lib/_internal/vm/bin/vmservice_io.dart

@@ -103,6 +103,7 @@ class _DebuggingSession {
         enableDevTools.toString(),
         devToolsBinaries,
         enableLogging.toString(),
+        _enableServicePortFallback.toString(),
       ],
       mode: ProcessStartMode.detachedWithStdio,
     );

sdk/lib/_internal/vm/bin/vmservice_server.dart

@@ -409,11 +409,8 @@ class Server {
       // Already running.
       return this;
     }
-
     // Startup HTTP server.
-    var pollError;
-    var pollStack;
-    Future<bool> poll() async {
+    Future<bool> startServer() async {
       try {
         var address;
         var addresses = await InternetAddress.lookup(_ip);
@@ -423,32 +420,25 @@ class Server {
           if (address.type == InternetAddressType.IPv4) break;
         }
         _server = await HttpServer.bind(address, _port);
-        return true;
       } catch (e, st) {
-        pollError = e;
-        pollStack = st;
-        return false;
+        if (_port != 0 && _enableServicePortFallback) {
+          serverPrint('Failed to bind Observatory HTTP server to port $_port. '
+              'Falling back to automatic port selection');
+          _port = 0;
+          return await startServer();
+        } else {
+          serverPrint('Could not start Observatory HTTP server:\n'
+              '$e\n$st');
+          _notifyServerState('');
+          onServerAddressChange(null);
+          return false;
+        }
       }
+      return true;
     }
 
-    // poll for the network for ~10 seconds.
-    int attempts = 0;
-    final maxAttempts = 10;
-    while (!await poll()) {
-      attempts++;
-      serverPrint('Observatory server failed to start after $attempts tries');
-      if (attempts > maxAttempts) {
-        serverPrint('Could not start Observatory HTTP server:\n'
-            '$pollError\n$pollStack\n');
-        _notifyServerState('');
-        onServerAddressChange(null);
-        return this;
-      }
-      if (_port != 0 && _enableServicePortFallback && attempts >= 3) {
-        _port = 0;
-        serverPrint('Falling back to automatic port selection');
-      }
-      await Future<void>.delayed(const Duration(seconds: 1));
+    if (!(await startServer())) {
+      return this;
     }
     if (_service.isExiting) {
       serverPrint('Observatory HTTP server exiting before listening as '