From 4c978efead184f629ed38e8bbb9e220610d3090b Mon Sep 17 00:00:00 2001
From: Christopher Fujino <christopherfujino@gmail.com>
Date: Thu, 23 Nov 2023 11:56:14 -0800
Subject: [PATCH] [flutter_tools] Fix bad state future already completed in
 flutter logs (#138517)

Fixes https://github.com/flutter/flutter/issues/138436
---
 packages/flutter_tools/lib/executable.dart    |  5 +-
 .../flutter_tools/lib/src/commands/logs.dart  | 32 ++++++++----
 .../commands.shard/hermetic/logs_test.dart    | 50 ++++++++++++++++++-
 .../flutter_tools/test/src/fake_devices.dart  |  8 +++
 4 files changed, 82 insertions(+), 13 deletions(-)

diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart
index b7af00cba8..9927cf906b 100644
--- a/packages/flutter_tools/lib/executable.dart
+++ b/packages/flutter_tools/lib/executable.dart
@@ -221,7 +221,10 @@ List<FlutterCommand> generateCommands({
   InstallCommand(
     verboseHelp: verboseHelp,
   ),
-  LogsCommand(),
+  LogsCommand(
+    sigint: ProcessSignal.sigint,
+    sigterm: ProcessSignal.sigterm,
+  ),
   MakeHostAppEditableCommand(),
   PackagesCommand(),
   PrecacheCommand(
diff --git a/packages/flutter_tools/lib/src/commands/logs.dart b/packages/flutter_tools/lib/src/commands/logs.dart
index 519a79c9f5..49a3d5ba0f 100644
--- a/packages/flutter_tools/lib/src/commands/logs.dart
+++ b/packages/flutter_tools/lib/src/commands/logs.dart
@@ -12,7 +12,10 @@ import '../globals.dart' as globals;
 import '../runner/flutter_command.dart';
 
 class LogsCommand extends FlutterCommand {
-  LogsCommand() {
+  LogsCommand({
+    required this.sigint,
+    required this.sigterm,
+  }) {
     argParser.addFlag('clear',
       negatable: false,
       abbr: 'c',
@@ -38,6 +41,8 @@ class LogsCommand extends FlutterCommand {
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
 
   Device? device;
+  final ProcessSignal sigint;
+  final ProcessSignal sigterm;
 
   @override
   Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) async {
@@ -65,26 +70,31 @@ class LogsCommand extends FlutterCommand {
 
     final Completer<int> exitCompleter = Completer<int>();
 
+    // First check if we already completed by another branch before completing
+    // with [exitCode].
+    void maybeComplete([int exitCode = 0]) {
+      if (exitCompleter.isCompleted) {
+        return;
+      }
+      exitCompleter.complete(exitCode);
+    }
+
     // Start reading.
     final StreamSubscription<String> subscription = logReader.logLines.listen(
       (String message) => globals.printStatus(message, wrap: false),
-      onDone: () {
-        exitCompleter.complete(0);
-      },
-      onError: (dynamic error) {
-        exitCompleter.complete(error is int ? error : 1);
-      },
+      onDone: () => maybeComplete(),
+      onError: (dynamic error) => maybeComplete(error is int ? error : 1),
     );
 
     // When terminating, close down the log reader.
-    ProcessSignal.sigint.watch().listen((ProcessSignal signal) {
+    sigint.watch().listen((ProcessSignal signal) {
       subscription.cancel();
+      maybeComplete();
       globals.printStatus('');
-      exitCompleter.complete(0);
     });
-    ProcessSignal.sigterm.watch().listen((ProcessSignal signal) {
+    sigterm.watch().listen((ProcessSignal signal) {
       subscription.cancel();
-      exitCompleter.complete(0);
+      maybeComplete();
     });
 
     // Wait for the log reader to be finished.
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/logs_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/logs_test.dart
index 2aaa015bbd..fc7d105b35 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/logs_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/logs_test.dart
@@ -2,17 +2,30 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:async';
+
 import 'package:flutter_tools/src/base/common.dart';
+import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/logs.dart';
+import 'package:flutter_tools/src/device.dart';
+import 'package:test/fake.dart';
 
 import '../../src/context.dart';
+import '../../src/fake_devices.dart';
 import '../../src/test_flutter_command_runner.dart';
 
 void main() {
   group('logs', () {
+    late Platform platform;
+    late FakeDeviceManager deviceManager;
+    const String deviceId = 'abc123';
+
     setUp(() {
       Cache.disableLocking();
+      deviceManager = FakeDeviceManager();
+      platform = FakePlatform();
     });
 
     tearDown(() {
@@ -20,11 +33,46 @@ void main() {
     });
 
     testUsingContext('fail with a bad device id', () async {
-      final LogsCommand command = LogsCommand();
+      final LogsCommand command = LogsCommand(
+        sigterm: FakeProcessSignal(),
+        sigint: FakeProcessSignal(),
+      );
       await expectLater(
         () => createTestCommandRunner(command).run(<String>['-d', 'abc123', 'logs']),
         throwsA(isA<ToolExit>().having((ToolExit error) => error.exitCode, 'exitCode', anyOf(isNull, 1))),
       );
     });
+
+    testUsingContext('does not try to complete exitCompleter multiple times', () async {
+      final FakeDevice fakeDevice = FakeDevice('phone', deviceId);
+      deviceManager.attachedDevices.add(fakeDevice);
+      final FakeProcessSignal termSignal = FakeProcessSignal();
+      final FakeProcessSignal intSignal = FakeProcessSignal();
+      final LogsCommand command = LogsCommand(
+        sigterm: termSignal,
+        sigint: intSignal,
+      );
+      final Future<void> commandFuture = createTestCommandRunner(command).run(<String>['-d', deviceId, 'logs']);
+      intSignal.send(1);
+      termSignal.send(1);
+      await pumpEventQueue(times: 5);
+      await commandFuture;
+    }, overrides: <Type, Generator>{
+      Platform: () => platform,
+      DeviceManager: () => deviceManager,
+    });
   });
 }
+
+class FakeProcessSignal extends Fake implements ProcessSignal {
+  late final StreamController<ProcessSignal> _controller = StreamController<ProcessSignal>();
+
+  @override
+  Stream<ProcessSignal> watch() => _controller.stream;
+
+  @override
+  bool send(int pid) {
+    _controller.add(this);
+    return true;
+  }
+}
diff --git a/packages/flutter_tools/test/src/fake_devices.dart b/packages/flutter_tools/test/src/fake_devices.dart
index 33e6e1c3a0..b29169c90a 100644
--- a/packages/flutter_tools/test/src/fake_devices.dart
+++ b/packages/flutter_tools/test/src/fake_devices.dart
@@ -118,6 +118,7 @@ class FakeDevice extends Device {
     this.connectionInterface = DeviceConnectionInterface.attached,
     PlatformType type = PlatformType.web,
     LaunchResult? launchResult,
+    this.deviceLogReader,
   }) : _isSupported = isSupported,
       _isSupportedForProject = isSupportedForProject,
       _launchResult = launchResult ?? LaunchResult.succeeded(),
@@ -131,6 +132,7 @@ class FakeDevice extends Device {
   final bool _isSupported;
   final bool _isSupportedForProject;
   final LaunchResult _launchResult;
+  DeviceLogReader? deviceLogReader;
 
   @override
   final String name;
@@ -183,6 +185,12 @@ class FakeDevice extends Device {
 
   @override
   Future<String> sdkNameAndVersion = Future<String>.value('Test SDK (1.2.3)');
+
+  @override
+  FutureOr<DeviceLogReader> getLogReader({
+    ApplicationPackage? app,
+    bool includePastLogs = false,
+  }) => deviceLogReader ?? FakeDeviceLogReader();
 }
 
 /// Combines fake device with its canonical JSON representation.