Revert "[vmservice] allow fallback on port bind failure"

This reverts commit 6ff7e9ebad.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [vmservice] allow fallback on port bind failure
> 
> Work towards https://github.com/flutter/flutter/issues/46724
> 
> Background:
> 
> on the latest versions of iOS today, we have to use mdns to discover the observatory port and authentication code. For a variety of reasons this can fail, leaving us with no way to connect.
> 
> An alternative approach is to specify the observatory port and disable the authentication code. If binding to this port fails, however, we would still like to attempt connecting with mdns.
> 
> Overview:
> 
> This adds a new flag to the dart SDK, --enable-service-port-fallback. This amends the behavior of the vmservice with a specified port. After failing to bind once to a non-zero port, update the port selection to 0.
> 
> Results:
> 
> Tested locally two dart VMs with the same specified port:
> 
> ```
> jonahwilliams@jonahwilliams0:~/Documents/engine/src/out/host_debug_unopt$ ./dart --enable-service-port-fallback --observe=8080 example.dart
> Observatory server failed to start after 1 tries
> Falling back to automatic port selection
> vm-service: isolate(2785434094928835)  'main' has no debugger attached and is paused at exit.  Connect to Observatory to debug.
> Observatory listening on http://127.0.0.1:45663/62A5IHjmv9E=/
> ```
> 
> Change-Id: I371583edcb603325428f1cb760992e39b9f3b6dc
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/130441
> Commit-Queue: Jonah Williams <jonahwilliams@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>

TBR=bkonyi@google.com,jonahwilliams@google.com

Change-Id: I337b2d549e33ef9c616c276a48ce894a14b1c317
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133327
Reviewed-by: Jonah Williams <jonahwilliams@google.com>
Commit-Queue: Jonah Williams <jonahwilliams@google.com>

runtime/bin/dart_embedder_api_impl.cc

@@ -90,8 +90,7 @@ Dart_Isolate CreateVmServiceIsolate(const IsolateCreationData& data,
   if (!bin::VmService::Setup(config.ip, config.port, config.dev_mode,
                              config.disable_auth_codes,
                              config.write_service_info_filename,
-                             /*trace_loading=*/false, config.deterministic,
-                             /*enable_service_port_fallback=*/false)) {
+                             /*trace_loading=*/false, config.deterministic)) {
     *error = strdup(bin::VmService::GetErrorMessage());
     return nullptr;
   }
@@ -125,8 +124,7 @@ Dart_Isolate CreateVmServiceIsolateFromKernel(
   if (!bin::VmService::Setup(config.ip, config.port, config.dev_mode,
                              config.disable_auth_codes,
                              config.write_service_info_filename,
-                             /*trace_loading=*/false, config.deterministic,
-                             /*enable_service_port_fallback=*/false)) {
+                             /*trace_loading=*/false, config.deterministic)) {
     *error = strdup(bin::VmService::GetErrorMessage());
     return nullptr;
   }

runtime/bin/main.cc

@@ -565,7 +565,7 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
           Options::vm_service_server_ip(), Options::vm_service_server_port(),
           Options::vm_service_dev_mode(), Options::vm_service_auth_disabled(),
           Options::vm_write_service_info_filename(), Options::trace_loading(),
-          Options::deterministic(), Options::enable_service_port_fallback())) {
+          Options::deterministic())) {
     *error = strdup(VmService::GetErrorMessage());
     return NULL;
   }

runtime/bin/main_options.cc

@@ -212,10 +212,6 @@ void Options::PrintUsage() {
 "  so it is not recommended to disable them unless behind a firewall on a\n"
 "  secure device.\n"
 "\n"
-"--enable-service-port-fallback\n"
-"  When the VM service is told to bind to a particular port, fallback to 0 if\n"
-"  it fails to bind instead of failing to start.\n"
-"\n"
 "--root-certs-file=<path>\n"
 "  The path to a file containing the trusted root certificates to use for\n"
 "  secure socket connections.\n"

runtime/bin/main_options.h

@@ -45,8 +45,7 @@ namespace bin {
   V(short_socket_write, short_socket_write)                                    \
   V(disable_exit, exit_disabled)                                               \
   V(preview_dart_2, nop_option)                                                \
-  V(suppress_core_dump, suppress_core_dump)                                    \
-  V(enable_service_port_fallback, enable_service_port_fallback)
+  V(suppress_core_dump, suppress_core_dump)
 
 // Boolean flags that have a short form.
 #define SHORT_BOOL_OPTIONS_LIST(V)                                             \

runtime/bin/run_vm_tests.cc

@@ -148,8 +148,7 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
   if (!bin::VmService::Setup("127.0.0.1", 0,
                              /*dev_mode=*/false, /*auth_disabled=*/true,
                              /*write_service_info_filename*/ "",
-                             /*trace_loading=*/false, /*deterministic=*/true,
-                             /*enable_service_port_fallback=*/false)) {
+                             /*trace_loading=*/false, /*deterministic=*/true)) {
     *error = strdup(bin::VmService::GetErrorMessage());
     return nullptr;
   }

runtime/bin/vmservice_impl.cc

@@ -116,8 +116,7 @@ bool VmService::Setup(const char* server_ip,
                       bool auth_codes_disabled,
                       const char* write_service_info_filename,
                       bool trace_loading,
-                      bool deterministic,
-                      bool enable_service_port_fallback) {
+                      bool deterministic) {
   Dart_Isolate isolate = Dart_CurrentIsolate();
   ASSERT(isolate != NULL);
   SetServerAddress("");
@@ -178,11 +177,6 @@ bool VmService::Setup(const char* server_ip,
                          Dart_NewBoolean(auth_codes_disabled));
   SHUTDOWN_ON_ERROR(result);
 
-  result =
-      Dart_SetField(library, DartUtils::NewString("_enableServicePortFallback"),
-                    Dart_NewBoolean(enable_service_port_fallback));
-  SHUTDOWN_ON_ERROR(result);
-
   if (write_service_info_filename != nullptr) {
     result = DartUtils::SetStringField(library, "_serviceInfoFilename",
                                        write_service_info_filename);

runtime/bin/vmservice_impl.h

@@ -20,8 +20,7 @@ class VmService {
                     bool auth_codes_disabled,
                     const char* write_service_info_filename,
                     bool trace_loading,
-                    bool deterministic,
-                    bool enable_service_port_fallback);
+                    bool deterministic);
 
   static void SetNativeResolver();
 

runtime/observatory/tests/service/enable_service_port_fallback_positive_test.dart

@@ -1,36 +0,0 @@
-// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import 'dart:io';
-
-import 'package:observatory/service_io.dart';
-import 'package:unittest/unittest.dart';
-import 'test_helper.dart';
-
-// Tests that the --enable-service-port-fallback flag works correctly by trying to bind to
-// a port that is available.
-var tests = <VMTest>[
-  (VM vm) async {
-    // Correctly bound to provided port.
-    expect(Uri.parse(serviceHttpAddress).port == selectedPort, true);
-  }
-];
-
-int selectedPort = 0;
-
-main(args) async {
-  selectedPort = await _getUnusedPort();
-  await runVMTests(args, tests,
-      enable_service_port_fallback: true,
-      // Choose a port number that should always be open.
-      port: selectedPort,
-      extraArgs: []);
-}
-
-Future<int> _getUnusedPort() async {
-  var socket = await ServerSocket.bind(InternetAddress.anyIPv4, 0);
-  var port = socket.port;
-  await socket.close();
-  return port;
-}

runtime/observatory/tests/service/enable_service_port_fallback_test.dart

@@ -1,23 +0,0 @@
-// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import 'package:observatory/service_io.dart';
-import 'package:unittest/unittest.dart';
-import 'test_helper.dart';
-
-// Tests that the --enable-service-port-fallback flag works correctly by trying to bind to
-// a port that is unsupported.
-var tests = <VMTest>[
-  (VM vm) async {
-    // Did not bind to provided port.
-    expect(Uri.parse(serviceHttpAddress).port != 1, true);
-  }
-];
-
-main(args) => runVMTests(args, tests,
-    enable_service_port_fallback: true,
-    // Choose a port number that most machines do not have permission
-    // to bind to.
-    port: 1,
-    extraArgs: []);

runtime/observatory/tests/service/test_helper.dart

@@ -105,10 +105,8 @@ class _ServiceTesteeLauncher {
       bool pause_on_start,
       bool pause_on_exit,
       bool pause_on_unhandled_exceptions,
-      bool enable_service_port_fallback,
       bool testeeControlsServer,
       Uri serviceInfoUri,
-      int port,
       List<String> extraArgs) {
     assert(pause_on_start != null);
     assert(pause_on_exit != null);
@@ -123,10 +121,8 @@ class _ServiceTesteeLauncher {
           pause_on_start,
           pause_on_exit,
           pause_on_unhandled_exceptions,
-          enable_service_port_fallback,
           testeeControlsServer,
           serviceInfoUri,
-          port,
           extraArgs);
     }
   }
@@ -135,10 +131,8 @@ class _ServiceTesteeLauncher {
       bool pause_on_start,
       bool pause_on_exit,
       bool pause_on_unhandled_exceptions,
-      bool enable_service_port_fallback,
       bool testeeControlsServer,
       Uri serviceInfoUri,
-      int port,
       List<String> extraArgs) {
     assert(!_shouldLaunchSkyShell());
 
@@ -151,9 +145,6 @@ class _ServiceTesteeLauncher {
     if (pause_on_exit) {
       fullArgs.add('--pause-isolates-on-exit');
     }
-    if (enable_service_port_fallback) {
-      fullArgs.add('--enable_service_port_fallback');
-    }
     fullArgs.add('--write-service-info=$serviceInfoUri');
 
     if (pause_on_unhandled_exceptions) {
@@ -166,7 +157,7 @@ class _ServiceTesteeLauncher {
 
     fullArgs.addAll(Platform.executableArguments);
     if (!testeeControlsServer) {
-      fullArgs.add('--enable-vm-service:$port');
+      fullArgs.add('--enable-vm-service:0');
     }
     fullArgs.addAll(args);
 
@@ -230,24 +221,15 @@ class _ServiceTesteeLauncher {
       bool pause_on_start,
       bool pause_on_exit,
       bool pause_on_unhandled_exceptions,
-      bool enable_service_port_fallback,
       bool testeeControlsServer,
-      int port,
       List<String> extraArgs) async {
     final completer = new Completer<Uri>();
     final serviceInfoDir =
         await Directory.systemTemp.createTemp('dart_service');
     final serviceInfoUri = serviceInfoDir.uri.resolve('service_info.json');
     final serviceInfoFile = await File.fromUri(serviceInfoUri).create();
-    _spawnProcess(
-            pause_on_start,
-            pause_on_exit,
-            pause_on_unhandled_exceptions,
-            enable_service_port_fallback,
-            testeeControlsServer,
-            serviceInfoUri,
-            port,
-            extraArgs)
+    _spawnProcess(pause_on_start, pause_on_exit, pause_on_unhandled_exceptions,
+            testeeControlsServer, serviceInfoUri, extraArgs)
         .then((p) async {
       process = p;
       Uri uri;
@@ -309,31 +291,22 @@ void setupAddresses(Uri serverAddress) {
 }
 
 class _ServiceTesterRunner {
-  void run({
-    List<String> mainArgs,
-    List<String> extraArgs,
-    List<VMTest> vmTests,
-    List<IsolateTest> isolateTests,
-    bool pause_on_start: false,
-    bool pause_on_exit: false,
-    bool verbose_vm: false,
-    bool pause_on_unhandled_exceptions: false,
-    bool enable_service_port_fallback: false,
-    bool testeeControlsServer: false,
-    int port = 0,
-  }) {
+  void run(
+      {List<String> mainArgs,
+      List<String> extraArgs,
+      List<VMTest> vmTests,
+      List<IsolateTest> isolateTests,
+      bool pause_on_start: false,
+      bool pause_on_exit: false,
+      bool verbose_vm: false,
+      bool pause_on_unhandled_exceptions: false,
+      bool testeeControlsServer: false}) {
     var process = new _ServiceTesteeLauncher();
     bool testsDone = false;
     runZoned(() {
       process
-          .launch(
-              pause_on_start,
-              pause_on_exit,
-              pause_on_unhandled_exceptions,
-              enable_service_port_fallback,
-              testeeControlsServer,
-              port,
-              extraArgs)
+          .launch(pause_on_start, pause_on_exit, pause_on_unhandled_exceptions,
+              testeeControlsServer, extraArgs)
           .then((Uri serverAddress) async {
         if (mainArgs.contains("--gdb")) {
           var pid = process.process.pid;
@@ -513,8 +486,6 @@ Future runVMTests(List<String> mainArgs, List<VMTest> tests,
     bool pause_on_exit: false,
     bool verbose_vm: false,
     bool pause_on_unhandled_exceptions: false,
-    bool enable_service_port_fallback: false,
-    int port = 0,
     List<String> extraArgs}) async {
   if (_isTestee()) {
     new _ServiceTesteeRunner().run(
@@ -524,15 +495,12 @@ Future runVMTests(List<String> mainArgs, List<VMTest> tests,
         pause_on_exit: pause_on_exit);
   } else {
     new _ServiceTesterRunner().run(
-      mainArgs: mainArgs,
-      extraArgs: extraArgs,
-      vmTests: tests,
-      pause_on_start: pause_on_start,
-      pause_on_exit: pause_on_exit,
-      verbose_vm: verbose_vm,
-      pause_on_unhandled_exceptions: pause_on_unhandled_exceptions,
-      enable_service_port_fallback: enable_service_port_fallback,
-      port: port,
-    );
+        mainArgs: mainArgs,
+        extraArgs: extraArgs,
+        vmTests: tests,
+        pause_on_start: pause_on_start,
+        pause_on_exit: pause_on_exit,
+        verbose_vm: verbose_vm,
+        pause_on_unhandled_exceptions: pause_on_unhandled_exceptions);
   }
 }

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

@@ -40,8 +40,6 @@ bool _isFuchsia = false;
 @pragma("vm:entry-point")
 var _signalWatch;
 var _signalSubscription;
-@pragma("vm:entry-point")
-bool _enableServicePortFallback = false;
 
 // HTTP server.
 Server server;
@@ -55,7 +53,7 @@ _lazyServerBoot() {
   var service = new VMService();
   // Lazily create server.
   server = new Server(service, _ip, _port, _originCheckDisabled,
-      _authCodesDisabled, _serviceInfoFilename, _enableServicePortFallback);
+      _authCodesDisabled, _serviceInfoFilename);
 }
 
 Future cleanupCallback() async {

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

@@ -147,13 +147,12 @@ class Server {
 
   final VMService _service;
   final String _ip;
+  final int _port;
   final bool _originCheckDisabled;
   final bool _authCodesDisabled;
-  final bool _enableServicePortFallback;
   final String _serviceInfoFilename;
   HttpServer _server;
   bool get running => _server != null;
-  int _port = -1;
 
   /// Returns the server address including the auth token.
   Uri get serverAddress {
@@ -168,14 +167,8 @@ class Server {
 
   // On Fuchsia, authentication codes are disabled by default. To enable, the authentication token
   // would have to be written into the hub alongside the port number.
-  Server(
-      this._service,
-      this._ip,
-      this._port,
-      this._originCheckDisabled,
-      bool authCodesDisabled,
-      this._serviceInfoFilename,
-      this._enableServicePortFallback)
+  Server(this._service, this._ip, this._port, this._originCheckDisabled,
+      bool authCodesDisabled, this._serviceInfoFilename)
       : _authCodesDisabled = (authCodesDisabled || Platform.isFuchsia);
 
   bool _isAllowedOrigin(String origin) {
@@ -429,10 +422,6 @@ class Server {
         onServerAddressChange(null);
         return this;
       }
-      if (_port != 0 && _enableServicePortFallback && attempts >= 3) {
-        _port = 0;
-        serverPrint('Falling back to automatic port selection');
-      }
       await new Future<Null>.delayed(const Duration(seconds: 1));
     }
     if (_service.isExiting) {

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

@@ -35,8 +35,6 @@ bool _isFuchsia = false;
 @pragma('vm:entry-point')
 var _signalWatch = null;
 var _signalSubscription;
-@pragma("vm:entry-point")
-bool _enableServicePortFallback = false;
 
 // HTTP server.
 Server? server;
@@ -51,7 +49,7 @@ Server _lazyServerBoot() {
   final service = VMService();
   // Lazily create server.
   localServer = Server(service, _ip, _port, _originCheckDisabled,
-      _authCodesDisabled, _serviceInfoFilename, _enableServicePortFallback);
+      _authCodesDisabled, _serviceInfoFilename);
   server = localServer;
   return localServer;
 }

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

@@ -147,13 +147,12 @@ class Server {
 
   final VMService _service;
   final String _ip;
+  final int _port;
   final bool _originCheckDisabled;
   final bool _authCodesDisabled;
-  final bool _enableServicePortFallback;
   final String? _serviceInfoFilename;
   HttpServer? _server;
   bool get running => _server != null;
-  int _port = -1;
 
   /// Returns the server address including the auth token.
   Uri? get serverAddress {
@@ -169,14 +168,8 @@ class Server {
 
   // On Fuchsia, authentication codes are disabled by default. To enable, the authentication token
   // would have to be written into the hub alongside the port number.
-  Server(
-      this._service,
-      this._ip,
-      this._port,
-      this._originCheckDisabled,
-      bool authCodesDisabled,
-      this._serviceInfoFilename,
-      this._enableServicePortFallback)
+  Server(this._service, this._ip, this._port, this._originCheckDisabled,
+      bool authCodesDisabled, this._serviceInfoFilename)
       : _authCodesDisabled = (authCodesDisabled || Platform.isFuchsia);
 
   bool _isAllowedOrigin(String origin) {
@@ -431,10 +424,6 @@ class Server {
         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 (_service.isExiting) {