[ Tool ] Fix crash from possible DDS startup race (flutter#173362)

It's possible (but unlikely) that two DDS instances can try and start at
the same time (e.g., a "flutter run" is initiated while an existing
"flutter attach" is waiting for a target to attach to). This can lead to
DDS failing to initialize for one of the processes when the VM service
disconnects it after the other process successfully invokes the
"_yieldControlToDDS" RPC.

This change adds some retry logic to avoid crashing in this scenario.

Fixes flutter#169265

Author: bkonyi

packages/flutter_tools/lib/src/commands/attach.dart

@@ -370,7 +370,6 @@ known, it can be explicitly provided to attach via the command-line, e.g.
               return runner.attach(
                 connectionInfoCompleter: connectionInfoCompleter,
                 appStartedCompleter: appStartedCompleter,
-                allowExistingDdsInstance: true,
               );
             },
             device,
@@ -411,10 +410,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
                   ..setupTerminal();
           }),
         );
-        result = await runner.attach(
-          appStartedCompleter: onAppStart,
-          allowExistingDdsInstance: true,
-        );
+        result = await runner.attach(appStartedCompleter: onAppStart);
         if (result != 0) {
           throwToolExit(null, exitCode: result);
         }

packages/flutter_tools/lib/src/isolated/resident_web_runner.dart

@@ -765,7 +765,6 @@ class ResidentWebRunner extends ResidentRunner {
     Completer<DebugConnectionInfo>? connectionInfoCompleter,
     Completer<void>? appStartedCompleter,
     Future<ConnectionResult?>? connectDebug,
-    bool allowExistingDdsInstance = false,
     bool needsFullRestart = true,
   }) async {
     if (_chromiumLauncher != null) {

packages/flutter_tools/lib/src/resident_runner.dart

@@ -235,7 +235,6 @@ class FlutterDevice {
     PrintStructuredErrorLogMethod? printStructuredErrorLogMethod,
     required DebuggingOptions debuggingOptions,
     int? hostVmServicePort,
-    required bool allowExistingDdsInstance,
   }) {
     final completer = Completer<void>();
     late StreamSubscription<void> subscription;
@@ -256,41 +255,99 @@ class FlutterDevice {
             }
           }
 
-          // First check if the VM service is actually listening on vmServiceUri as
-          // this may not be the case when scraping logcat for URIs. If this URI is
-          // from an old application instance, we shouldn't try and start DDS.
-          try {
-            service = await connectToVmService(vmServiceUri!, logger: globals.logger);
-            await service.dispose();
-          } on Exception catch (exception) {
-            globals.printTrace('Fail to connect to service protocol: $vmServiceUri: $exception');
-            if (!completer.isCompleted && !_isListeningForVmServiceUri!) {
-              completer.completeError('failed to connect to $vmServiceUri $exception');
+          const kMaxAttempts = 3;
+          for (var attempts = 1; attempts <= kMaxAttempts; ++attempts) {
+            void handleVmServiceCheckException(Exception e) {
+              globals.printTrace('Fail to connect to service protocol: $vmServiceUri: $e');
+              if (!completer.isCompleted && !_isListeningForVmServiceUri!) {
+                completer.completeError('failed to connect to $vmServiceUri $e');
+              }
+            }
+
+            // First check if the VM service is actually listening on vmServiceUri as
+            // this may not be the case when scraping logcat for URIs. If this URI is
+            // from an old application instance, we shouldn't try and start DDS.
+            try {
+              service = await connectToVmService(vmServiceUri!, logger: globals.logger);
+              await service.dispose();
+            } on vm_service.RPCError catch (e, st) {
+              if (!e.isConnectionDisposedException) {
+                handleVmServiceCheckException(e);
+                return;
+              }
+              // It's possible (but unlikely) that two DDS instances can try and start at the same
+              // time (e.g., a "flutter run" is initiated while an existing "flutter attach" is
+              // waiting for a target to attach to). This can lead to the initial VM service connection
+              // failing for one of the processes when the VM service disconnects it after the other
+              // instance successfully invoked the "_yieldControlToDDS" RPC.
+              //
+              // To handle this, we retry connecting to the VM service, which should successfully
+              // be redirected to the DDS instance.
+              //
+              // See https://github.com/flutter/flutter/issues/169265 for details.
+              if (attempts == kMaxAttempts) {
+                globals.printTrace(
+                  'Failed to make initial connection to VM Service (attempt $attempts of $kMaxAttempts).',
+                );
+                handleError(e, st);
+                return;
+              }
+              // Exponential backoff.
+              final int backoffPeriod = (1 << (attempts - 1)) * 100;
+              globals.printTrace(
+                'Failed to make initial connection to VM Service (attempt $attempts of $kMaxAttempts). '
+                'Retrying in ${backoffPeriod}ms...',
+              );
+              await Future<void>.delayed(Duration(milliseconds: backoffPeriod));
+            } on Exception catch (e) {
+              handleVmServiceCheckException(e);
+              return;
             }
-            return;
           }
 
-          // This first try block is meant to catch errors that occur during DDS startup
-          // (e.g., failure to bind to a port, failure to connect to the VM service,
-          // attaching to a VM service with existing clients, etc.).
-          try {
-            await device!.dds.startDartDevelopmentServiceFromDebuggingOptions(
-              vmServiceUri,
-              debuggingOptions: debuggingOptions,
-            );
-          } on DartDevelopmentServiceException catch (e, st) {
-            if (!allowExistingDdsInstance ||
-                (e.errorCode != DartDevelopmentServiceException.existingDdsInstanceError)) {
+          for (var attempts = 1; attempts <= kMaxAttempts; ++attempts) {
+            // This try block is meant to catch errors that occur during DDS startup
+            // (e.g., failure to bind to a port, failure to connect to the VM service,
+            // attaching to a VM service with existing clients, etc.).
+            try {
+              await device!.dds.startDartDevelopmentServiceFromDebuggingOptions(
+                vmServiceUri!,
+                debuggingOptions: debuggingOptions,
+              );
+              break;
+            } on DartDevelopmentServiceException catch (e, st) {
+              if (e.errorCode == DartDevelopmentServiceException.existingDdsInstanceError) {
+                existingDds = true;
+                break;
+              }
+              // It's possible (but unlikely) that two DDS instances can try and start at the same
+              // time (e.g., a "flutter run" is initiated while an existing "flutter attach" is
+              // waiting for a target to attach to). This leads to DDS failing to initialize for
+              // one of the processes when the VM service disconnects it after the other instance
+              // successfully invoked the "_yieldControlToDDS" RPC.
+              //
+              // To handle this, we retry to start DDS after a short delay, which should result in
+              // an existingDdsInstanceError if the failure to start was due to a startup race.
+              //
+              // See https://github.com/flutter/flutter/issues/169265 for details.
+              if (attempts == kMaxAttempts) {
+                globals.printTrace('Failed to start DDS (attempt $attempts of $kMaxAttempts).');
+                handleError(e, st);
+                return;
+              }
+              // Exponential backoff.
+              final int backoffPeriod = (1 << (attempts - 1)) * 100;
+              globals.printTrace(
+                'Failed to start DDS (attempt $attempts of $kMaxAttempts). '
+                'Retrying in ${backoffPeriod}ms...',
+              );
+              await Future<void>.delayed(Duration(milliseconds: backoffPeriod));
+            } on ToolExit {
+              rethrow;
+            } on Exception catch (e, st) {
               handleError(e, st);
               return;
-            } else {
-              existingDds = true;
             }
-          } on ToolExit {
-            rethrow;
-          } on Exception catch (e, st) {
-            handleError(e, st);
-            return;
           }
         }
         // This second try block handles cases where the VM service connection goes down
@@ -1144,7 +1201,6 @@ abstract class ResidentRunner extends ResidentHandlers {
   Future<int?> attach({
     Completer<DebugConnectionInfo>? connectionInfoCompleter,
     Completer<void>? appStartedCompleter,
-    bool allowExistingDdsInstance = false,
     bool needsFullRestart = true,
   });
 
@@ -1227,7 +1283,6 @@ abstract class ResidentRunner extends ResidentHandlers {
     await residentDevtoolsHandler!.shutdown();
     await stopEchoingDeviceLog();
     await preExit();
-    shutdownDartDevelopmentService();
     appFinished();
   }
 
@@ -1298,7 +1353,6 @@ abstract class ResidentRunner extends ResidentHandlers {
     ReloadSources? reloadSources,
     Restart? restart,
     CompileExpression? compileExpression,
-    required bool allowExistingDdsInstance,
   }) async {
     if (!debuggingOptions.debuggingEnabled) {
       throw Exception('The service protocol is not enabled.');
@@ -1311,7 +1365,6 @@ abstract class ResidentRunner extends ResidentHandlers {
         reloadSources: reloadSources,
         restart: restart,
         compileExpression: compileExpression,
-        allowExistingDdsInstance: allowExistingDdsInstance,
         hostVmServicePort: debuggingOptions.hostVmServicePort,
         printStructuredErrorLogMethod: printStructuredErrorLog,
       );

packages/flutter_tools/lib/src/run_cold.dart

@@ -67,7 +67,7 @@ class ColdRunner extends ResidentRunner {
     // Connect to the VM Service.
     if (debuggingEnabled) {
       try {
-        await connectToServiceProtocol(allowExistingDdsInstance: false);
+        await connectToServiceProtocol();
       } on Exception catch (exception) {
         globals.printError(exception.toString());
         appFailedToStart();
@@ -138,12 +138,11 @@ class ColdRunner extends ResidentRunner {
   Future<int> attach({
     Completer<DebugConnectionInfo>? connectionInfoCompleter,
     Completer<void>? appStartedCompleter,
-    bool allowExistingDdsInstance = false,
     bool needsFullRestart = true,
   }) async {
     _didAttach = true;
     try {
-      await connectToServiceProtocol(allowExistingDdsInstance: allowExistingDdsInstance);
+      await connectToServiceProtocol();
     } on Exception catch (error) {
       globals.printError('Error connecting to the service protocol: $error');
       return 2;

packages/flutter_tools/lib/src/run_hot.dart

@@ -233,30 +233,26 @@ class HotRunner extends ResidentRunner {
   Future<int> attach({
     Completer<DebugConnectionInfo>? connectionInfoCompleter,
     Completer<void>? appStartedCompleter,
-    bool allowExistingDdsInstance = false,
     bool needsFullRestart = true,
   }) async {
     stopAppDuringCleanup = false;
     return _attach(
       connectionInfoCompleter: connectionInfoCompleter,
       appStartedCompleter: appStartedCompleter,
-      allowExistingDdsInstance: allowExistingDdsInstance,
       needsFullRestart: needsFullRestart,
     );
   }
 
   Future<int> _attach({
     Completer<DebugConnectionInfo>? connectionInfoCompleter,
     Completer<void>? appStartedCompleter,
-    bool allowExistingDdsInstance = false,
     bool needsFullRestart = true,
   }) async {
     try {
       await connectToServiceProtocol(
         reloadSources: _reloadSourcesService,
         restart: _restartService,
         compileExpression: _compileExpressionService,
-        allowExistingDdsInstance: allowExistingDdsInstance,
       );
       // Catches all exceptions, non-Exception objects are rethrown.
     } catch (error) {

packages/flutter_tools/lib/src/vmservice.dart

@@ -326,6 +326,9 @@ Future<vm_service.VmService> setUpVmService({
   try {
     await Future.wait(registrationRequests);
   } on vm_service.RPCError catch (e) {
+    if (e.isConnectionDisposedException) {
+      rethrow;
+    }
     throwToolExit('Failed to register service methods on attached VM Service: $e');
   }
   return vmService;
@@ -483,9 +486,7 @@ class FlutterVmService {
     try {
       return await service.getVM();
     } on vm_service.RPCError catch (err) {
-      if (err.code == vm_service.RPCErrorKind.kServiceDisappeared.code ||
-          err.code == vm_service.RPCErrorKind.kConnectionDisposed.code ||
-          err.message.contains('Service connection disposed')) {
+      if (err.isConnectionDisposedException) {
         globals.printTrace('VmService.getVm call failed: $err');
         return null;
       }
@@ -505,9 +506,7 @@ class FlutterVmService {
       // and should begin to shutdown due to the service connection closing.
       // Swallow the exception here and let the shutdown logic elsewhere deal
       // with cleaning up.
-      if (e.code == vm_service.RPCErrorKind.kServiceDisappeared.code ||
-          e.code == vm_service.RPCErrorKind.kConnectionDisposed.code ||
-          e.message.contains('Service connection disposed')) {
+      if (e.isConnectionDisposedException) {
         return null;
       }
       rethrow;
@@ -773,10 +772,8 @@ class FlutterVmService {
     } on vm_service.RPCError catch (err) {
       // If an application is not using the framework or the VM service
       // disappears while handling a request, return null.
-      if ((err.code == vm_service.RPCErrorKind.kMethodNotFound.code) ||
-          (err.code == vm_service.RPCErrorKind.kServiceDisappeared.code) ||
-          (err.code == vm_service.RPCErrorKind.kConnectionDisposed.code) ||
-          (err.message.contains('Service connection disposed'))) {
+      if (err.code == vm_service.RPCErrorKind.kMethodNotFound.code ||
+          err.isConnectionDisposedException) {
         return null;
       }
       rethrow;
@@ -1010,3 +1007,10 @@ String processVmServiceMessage(vm_service.Event event) {
   }
   return message;
 }
+
+extension RPCErrorExtension on vm_service.RPCError {
+  bool get isConnectionDisposedException =>
+      code == vm_service.RPCErrorKind.kServiceDisappeared.code ||
+      code == vm_service.RPCErrorKind.kConnectionDisposed.code ||
+      message.contains('Service connection disposed');
+}