[flutter_tools] catch SocketException writing to ios-deploy stdin (#139784)

Fixes flutter/flutter#139709

This adds a static helper method `ProcessUtils.writelnToStdinGuarded()`, which will asynchronously write to a sub-process's STDIN `IOSink` and catch errors.

In talking with Brian, it sounds like this is the best and most reliable way to catch `SocketException`s during these writes *to sub-process file descriptors* specifically (with a "real" hard drive file, the future returned by `.flush()` should complete with the write error).

Also, as I note in the dartdoc to `writelnToStdinGuarded()`, the behavior seems to be different between macOS and linux.

Moving forward, in any place where we want to catch exceptions writing to STDIN, we will want to use this new helper.

Author: christopherfujino

packages/flutter_tools/lib/src/base/process.dart

@@ -233,6 +233,52 @@ abstract class ProcessUtils {
     List<String> cli, {
     Map<String, String>? environment,
   });
+
+  /// Write [line] to [stdin] and catch any errors with [onError].
+  ///
+  /// Specifically with [Process] file descriptors, an exception that is
+  /// thrown as part of a write can be most reliably caught with a
+  /// [ZoneSpecification] error handler.
+  ///
+  /// On some platforms, the following code appears to work:
+  ///
+  /// ```dart
+  /// stdin.writeln(line);
+  /// try {
+  ///   await stdin.flush(line);
+  /// } catch (err) {
+  ///   // handle error
+  /// }
+  /// ```
+  ///
+  /// However it did not catch a [SocketException] on Linux.
+  static Future<void> writelnToStdinGuarded({
+    required IOSink stdin,
+    required String line,
+    required void Function(Object, StackTrace) onError,
+  }) async {
+    final Completer<void> completer = Completer<void>();
+
+    void writeFlushAndComplete() {
+      stdin.writeln(line);
+      stdin.flush().whenComplete(() {
+        if (!completer.isCompleted) {
+          completer.complete();
+        }
+      });
+    }
+
+    runZonedGuarded(
+      writeFlushAndComplete,
+      (Object error, StackTrace stackTrace) {
+        onError(error, stackTrace);
+        if (!completer.isCompleted) {
+          completer.complete();
+        }
+      },
+    );
+    return completer.future;
+  }
 }
 
 class _DefaultProcessUtils implements ProcessUtils {

packages/flutter_tools/lib/src/ios/ios_deploy.dart

@@ -598,29 +598,66 @@ class IOSDeployDebugger {
     if (!debuggerAttached) {
       return;
     }
-    try {
-      // Stop the app, which will prompt the backtrace to be printed for all threads in the stdoutSubscription handler.
-      _iosDeployProcess?.stdin.writeln(_signalStop);
-    } on SocketException catch (error) {
-      // Best effort, try to detach, but maybe the app already exited or already detached.
-      _logger.printTrace('Could not stop app from debugger: $error');
-    }
+    // Stop the app, which will prompt the backtrace to be printed for all
+    // threads in the stdoutSubscription handler.
+    await stdinWriteln(
+      _signalStop,
+      onError: (Object error, _) {
+        _logger.printTrace('Could not stop the app: $error');
+      },
+    );
+
     // Wait for logging to finish on process exit.
     return logLines.drain();
   }
 
-  void detach() {
-    if (!debuggerAttached) {
+  Future<void>? _stdinWriteFuture;
+
+  /// Queue write of [line] to STDIN of [_iosDeployProcess].
+  ///
+  /// No-op if [_iosDeployProcess] is null.
+  ///
+  /// This write will not happen until the flush of any previous writes have
+  /// completed, because calling [IOSink.flush()] before a previous flush has
+  /// completed will throw a [StateError].
+  ///
+  /// This method needs to keep track of the [_stdinWriteFuture] from previous
+  /// calls because the future returned by [detach] is not always await-ed.
+  Future<void> stdinWriteln(String line, {required void Function(Object, StackTrace) onError}) async {
+    final Process? process = _iosDeployProcess;
+    if (process == null) {
       return;
     }
 
-    try {
-      // Detach lldb from the app process.
-      _iosDeployProcess?.stdin.writeln('process detach');
-    } on SocketException catch (error) {
-      // Best effort, try to detach, but maybe the app already exited or already detached.
-      _logger.printTrace('Could not detach from debugger: $error');
+    Future<void> writeln() {
+      return ProcessUtils.writelnToStdinGuarded(
+        stdin: process.stdin,
+        line: line,
+        onError: onError,
+      );
+    }
+
+    if (_stdinWriteFuture != null) {
+      _stdinWriteFuture = _stdinWriteFuture!.then<void>((_) => writeln());
+    } else {
+      _stdinWriteFuture = writeln();
     }
+
+    return _stdinWriteFuture;
+  }
+
+  Future<void> detach() async {
+    if (!debuggerAttached) {
+      return;
+    }
+
+    return stdinWriteln(
+      'process detach',
+      onError: (Object error, _) {
+        // Best effort, try to detach, but maybe the app already exited or already detached.
+        _logger.printTrace('Could not detach from debugger: $error');
+      }
+    );
   }
 }
 

packages/flutter_tools/test/general.shard/ios/ios_deploy_test.dart

@@ -9,12 +9,14 @@ import 'package:file/memory.dart';
 import 'package:file_testing/file_testing.dart';
 import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/base/process.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/ios/ios_deploy.dart';
+import 'package:test/fake.dart';
 
 import '../../src/common.dart';
 import '../../src/fake_process_manager.dart';
@@ -386,7 +388,26 @@ void main () {
       );
       expect(stdin.stream.transform<String>(const Utf8Decoder()), emits('process detach'));
       await iosDeployDebugger.launchAndAttach();
-      iosDeployDebugger.detach();
+      await iosDeployDebugger.detach();
+    });
+
+    testWithoutContext('detach handles broken pipe', () async {
+      final StreamSink<List<int>> stdinSink = _ClosedStdinController();
+      final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+        FakeCommand(
+          command: const <String>['ios-deploy'],
+          stdout: '(lldb)     run\nsuccess',
+          stdin: IOSink(stdinSink),
+        ),
+      ]);
+      final BufferLogger logger = BufferLogger.test();
+      final IOSDeployDebugger iosDeployDebugger = IOSDeployDebugger.test(
+        processManager: processManager,
+        logger: logger,
+      );
+      await iosDeployDebugger.launchAndAttach();
+      await iosDeployDebugger.detach();
+      expect(logger.traceText, contains('Could not detach from debugger'));
     });
 
     testWithoutContext('stop with backtrace', () async {
@@ -399,18 +420,28 @@ void main () {
           ],
           stdout:
           '(lldb)     run\nsuccess\nLog on attach\n(lldb) Process 6156 stopped\n* thread #1, stop reason = Assertion failed:\n(lldb) Process 6156 detached',
-          stdin: IOSink(stdin.sink),
+          stdin: IOSink(stdin),
         ),
       ]);
       final IOSDeployDebugger iosDeployDebugger = IOSDeployDebugger.test(
         processManager: processManager,
       );
       await iosDeployDebugger.launchAndAttach();
-      await iosDeployDebugger.stopAndDumpBacktrace();
-      expect(await stdinStream.take(3).toList(), <String>[
+      List<String>? stdinLines;
+
+      // These two futures will deadlock if await-ed sequentially
+      await Future.wait(<Future<void>>[
+        iosDeployDebugger.stopAndDumpBacktrace(),
+        stdinStream.take(5).toList().then<void>(
+          (List<String> lines) => stdinLines = lines,
+        ),
+      ]);
+      expect(stdinLines, const <String>[
         'thread backtrace all',
         '\n',
         'process detach',
+        '\n',
+        'process signal SIGSTOP',
       ]);
     });
 
@@ -616,6 +647,11 @@ process continue
   });
 }
 
+class _ClosedStdinController extends Fake implements StreamSink<List<int>> {
+  @override
+  Future<Object?> addStream(Stream<List<int>> stream) async => throw const SocketException('Bad pipe');
+}
+
 IOSDeploy setUpIOSDeploy(ProcessManager processManager, {
     Artifacts? artifacts,
   }) {

packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart

@@ -1001,7 +1001,7 @@ class FakeIOSDeployDebugger extends Fake implements IOSDeployDebugger {
   Stream<String> logLines = const Stream<String>.empty();
 
   @override
-  void detach() {
+  Future<void> detach() async {
     detached = true;
   }
 }