diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index d1372093bdf..d0acf2b5d8b 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -233,6 +233,52 @@ abstract class ProcessUtils { List cli, { Map? 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 writelnToStdinGuarded({ + required IOSink stdin, + required String line, + required void Function(Object, StackTrace) onError, + }) async { + final Completer completer = Completer(); + + 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 { diff --git a/packages/flutter_tools/lib/src/ios/ios_deploy.dart b/packages/flutter_tools/lib/src/ios/ios_deploy.dart index 71cb6dbee63..a4cf23ffa54 100644 --- a/packages/flutter_tools/lib/src/ios/ios_deploy.dart +++ b/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() { + Future? _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 stdinWriteln(String line, {required void Function(Object, StackTrace) onError}) async { + final Process? process = _iosDeployProcess; + if (process == null) { + return; + } + + Future writeln() { + return ProcessUtils.writelnToStdinGuarded( + stdin: process.stdin, + line: line, + onError: onError, + ); + } + + if (_stdinWriteFuture != null) { + _stdinWriteFuture = _stdinWriteFuture!.then((_) => writeln()); + } else { + _stdinWriteFuture = writeln(); + } + + return _stdinWriteFuture; + } + + Future detach() async { if (!debuggerAttached) { 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'); - } + 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'); + } + ); } } diff --git a/packages/flutter_tools/test/general.shard/ios/ios_deploy_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_deploy_test.dart index 7862b4d9191..7ef5f83c163 100644 --- a/packages/flutter_tools/test/general.shard/ios/ios_deploy_test.dart +++ b/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(const Utf8Decoder()), emits('process detach')); await iosDeployDebugger.launchAndAttach(); - iosDeployDebugger.detach(); + await iosDeployDebugger.detach(); + }); + + testWithoutContext('detach handles broken pipe', () async { + final StreamSink> stdinSink = _ClosedStdinController(); + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: const ['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(), [ + List? stdinLines; + + // These two futures will deadlock if await-ed sequentially + await Future.wait(>[ + iosDeployDebugger.stopAndDumpBacktrace(), + stdinStream.take(5).toList().then( + (List lines) => stdinLines = lines, + ), + ]); + expect(stdinLines, const [ 'thread backtrace all', '\n', 'process detach', + '\n', + 'process signal SIGSTOP', ]); }); @@ -616,6 +647,11 @@ process continue }); } +class _ClosedStdinController extends Fake implements StreamSink> { + @override + Future addStream(Stream> stream) async => throw const SocketException('Bad pipe'); +} + IOSDeploy setUpIOSDeploy(ProcessManager processManager, { Artifacts? artifacts, }) { diff --git a/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart index 01506cd99df..c113334ba03 100644 --- a/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart @@ -1001,7 +1001,7 @@ class FakeIOSDeployDebugger extends Fake implements IOSDeployDebugger { Stream logLines = const Stream.empty(); @override - void detach() { + Future detach() async { detached = true; } }