[flutter_tools] Instantiate shutdown hooks before localfilesystem (#110693)

packages/flutter_tools/lib/executable.dart

@@ -127,6 +127,7 @@ Future<void> main(List<String> args) async {
       },
       PreRunValidator: () => PreRunValidator(fileSystem: globals.fs),
     },
+    shutdownHooks: globals.shutdownHooks,
   );
 }
 

packages/flutter_tools/lib/runner.dart

@@ -34,6 +34,7 @@ Future<int> run(
     bool? reportCrashes,
     String? flutterVersion,
     Map<Type, Generator>? overrides,
+    required ShutdownHooks shutdownHooks,
   }) async {
   if (muteCommandLogging) {
     // Remove the verbose option; for help and doctor, users don't need to see
@@ -64,7 +65,7 @@ Future<int> run(
         // Triggering [runZoned]'s error callback does not necessarily mean that
         // we stopped executing the body. See https://github.com/dart-lang/sdk/issues/42150.
         if (firstError == null) {
-          return await _exit(0);
+          return await _exit(0, shutdownHooks: shutdownHooks);
         }
 
         // We already hit some error, so don't return success. The error path
@@ -74,7 +75,7 @@ Future<int> run(
         // This catches all exceptions to send to crash logging, etc.
         firstError = error;
         firstStackTrace = stackTrace;
-        return _handleToolError(error, stackTrace, verbose, args, reportCrashes!, getVersion);
+        return _handleToolError(error, stackTrace, verbose, args, reportCrashes!, getVersion, shutdownHooks);
       }
     }, onError: (Object error, StackTrace stackTrace) async { // ignore: deprecated_member_use
       // If sending a crash report throws an error into the zone, we don't want
@@ -82,7 +83,7 @@ Future<int> run(
       // to send the original error that triggered the crash report.
       firstError ??= error;
       firstStackTrace ??= stackTrace;
-      await _handleToolError(firstError!, firstStackTrace, verbose, args, reportCrashes!, getVersion);
+      await _handleToolError(firstError!, firstStackTrace, verbose, args, reportCrashes!, getVersion, shutdownHooks);
     });
   }, overrides: overrides);
 }
@@ -94,12 +95,13 @@ Future<int> _handleToolError(
   List<String> args,
   bool reportCrashes,
   String Function() getFlutterVersion,
+  ShutdownHooks shutdownHooks,
 ) async {
   if (error is UsageException) {
     globals.printError('${error.message}\n');
     globals.printError("Run 'flutter -h' (or 'flutter <command> -h') for available flutter commands and options.");
     // Argument error exit code.
-    return _exit(64);
+    return _exit(64, shutdownHooks: shutdownHooks);
   } else if (error is ToolExit) {
     if (error.message != null) {
       globals.printError(error.message!);
@@ -107,14 +109,14 @@ Future<int> _handleToolError(
     if (verbose) {
       globals.printError('\n$stackTrace\n');
     }
-    return _exit(error.exitCode ?? 1);
+    return _exit(error.exitCode ?? 1, shutdownHooks: shutdownHooks);
   } else if (error is ProcessExit) {
     // We've caught an exit code.
     if (error.immediate) {
       exit(error.exitCode);
       return error.exitCode;
     } else {
-      return _exit(error.exitCode);
+      return _exit(error.exitCode, shutdownHooks: shutdownHooks);
     }
   } else {
     // We've crashed; emit a log report.
@@ -124,7 +126,7 @@ Future<int> _handleToolError(
       // Print the stack trace on the bots - don't write a crash report.
       globals.stdio.stderrWrite('$error\n');
       globals.stdio.stderrWrite('$stackTrace\n');
-      return _exit(1);
+      return _exit(1, shutdownHooks: shutdownHooks);
     }
 
     // Report to both [Usage] and [CrashReportSender].
@@ -165,7 +167,7 @@ Future<int> _handleToolError(
       final File file = await _createLocalCrashReport(details);
       await globals.crashReporter!.informUser(details, file);
 
-      return _exit(1);
+      return _exit(1, shutdownHooks: shutdownHooks);
     // This catch catches all exceptions to ensure the message below is printed.
     } catch (error, st) { // ignore: avoid_catches_without_on_clauses
       globals.stdio.stderrWrite(
@@ -228,7 +230,7 @@ Future<File> _createLocalCrashReport(CrashDetails details) async {
   return crashFile;
 }
 
-Future<int> _exit(int code) async {
+Future<int> _exit(int code, {required ShutdownHooks shutdownHooks}) async {
   // Prints the welcome message if needed.
   globals.flutterUsage.printWelcome();
 
@@ -241,7 +243,7 @@ Future<int> _exit(int code) async {
   }
 
   // Run shutdown hooks before flushing logs
-  await globals.shutdownHooks!.runShutdownHooks();
+  await shutdownHooks.runShutdownHooks(globals.logger);
 
   final Completer<void> completer = Completer<void>();
 

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

@@ -177,17 +177,18 @@ File getUniqueFile(Directory dir, String baseName, String ext) {
 /// directories and files that the tool creates under the system temporary
 /// directory when the tool exits either normally or when killed by a signal.
 class LocalFileSystem extends local_fs.LocalFileSystem {
-  LocalFileSystem(this._signals, this._fatalSignals, this._shutdownHooks);
+  LocalFileSystem(this._signals, this._fatalSignals, this.shutdownHooks);
 
   @visibleForTesting
   LocalFileSystem.test({
     required Signals signals,
     List<ProcessSignal> fatalSignals = Signals.defaultExitSignals,
-  }) : this(signals, fatalSignals, null);
+  }) : this(signals, fatalSignals, ShutdownHooks());
 
   Directory? _systemTemp;
   final Map<ProcessSignal, Object> _signalTokens = <ProcessSignal, Object>{};
-  final ShutdownHooks? _shutdownHooks;
+
+  final ShutdownHooks shutdownHooks;
 
   Future<void> dispose() async {
     _tryToDeleteTemp();
@@ -206,7 +207,7 @@ class LocalFileSystem extends local_fs.LocalFileSystem {
         _systemTemp?.deleteSync(recursive: true);
       }
     } on FileSystemException {
-      // ignore.
+      // ignore
     }
     _systemTemp = null;
   }
@@ -239,7 +240,7 @@ class LocalFileSystem extends local_fs.LocalFileSystem {
       }
       // Make sure that the temporary directory is cleaned up when the tool
       // exits normally.
-      _shutdownHooks?.addShutdownHook(
+      shutdownHooks.addShutdownHook(
         _tryToDeleteTemp,
       );
     }

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

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 
 import '../convert.dart';
@@ -13,7 +14,7 @@ import 'logger.dart';
 typedef StringConverter = String? Function(String string);
 
 /// A function that will be run before the VM exits.
-typedef ShutdownHook = FutureOr<dynamic> Function();
+typedef ShutdownHook = FutureOr<void> Function();
 
 // TODO(ianh): We have way too many ways to run subprocesses in this project.
 // Convert most of these into one or more lightweight wrappers around the
@@ -22,17 +23,16 @@ typedef ShutdownHook = FutureOr<dynamic> Function();
 // for more details.
 
 abstract class ShutdownHooks {
-  factory ShutdownHooks({
+  factory ShutdownHooks() => _DefaultShutdownHooks();
-    required Logger logger,
-  }) => _DefaultShutdownHooks(
-    logger: logger,
-  );
 
   /// Registers a [ShutdownHook] to be executed before the VM exits.
   void addShutdownHook(
     ShutdownHook shutdownHook
   );
 
+  @visibleForTesting
+  List<ShutdownHook> get registeredHooks;
+
   /// Runs all registered shutdown hooks and returns a future that completes when
   /// all such hooks have finished.
   ///
@@ -40,16 +40,17 @@ abstract class ShutdownHooks {
   /// hooks within a given stage will be started in parallel and will be
   /// guaranteed to run to completion before shutdown hooks in the next stage are
   /// started.
-  Future<void> runShutdownHooks();
+  ///
+  /// This class is constructed before the [Logger], so it cannot be direct
+  /// injected in the constructor.
+  Future<void> runShutdownHooks(Logger logger);
 }
 
 class _DefaultShutdownHooks implements ShutdownHooks {
-  _DefaultShutdownHooks({
+  _DefaultShutdownHooks();
-    required Logger logger,
-  }) : _logger = logger;
 
-  final Logger _logger;
+  @override
-  final List<ShutdownHook> _shutdownHooks = <ShutdownHook>[];
+  final List<ShutdownHook> registeredHooks = <ShutdownHook>[];
 
   bool _shutdownHooksRunning = false;
 
@@ -58,16 +59,18 @@ class _DefaultShutdownHooks implements ShutdownHooks {
     ShutdownHook shutdownHook
   ) {
     assert(!_shutdownHooksRunning);
-    _shutdownHooks.add(shutdownHook);
+    registeredHooks.add(shutdownHook);
   }
 
   @override
-  Future<void> runShutdownHooks() async {
+  Future<void> runShutdownHooks(Logger logger) async {
-    _logger.printTrace('Running shutdown hooks');
+    logger.printTrace(
+      'Running ${registeredHooks.length} shutdown hook${registeredHooks.length == 1 ? '' : 's'}',
+    );
     _shutdownHooksRunning = true;
     try {
       final List<Future<dynamic>> futures = <Future<dynamic>>[];
-      for (final ShutdownHook shutdownHook in _shutdownHooks) {
+      for (final ShutdownHook shutdownHook in registeredHooks) {
         final FutureOr<dynamic> result = shutdownHook();
         if (result is Future<dynamic>) {
           futures.add(result);
@@ -77,7 +80,7 @@ class _DefaultShutdownHooks implements ShutdownHooks {
     } finally {
       _shutdownHooksRunning = false;
     }
-    _logger.printTrace('Shutdown hooks complete');
+    logger.printTrace('Shutdown hooks complete');
   }
 }
 

packages/flutter_tools/lib/src/context_runner.dart

@@ -313,7 +313,6 @@ Future<T> runInContext<T>(
         platform: globals.platform,
         usage: globals.flutterUsage,
       ),
-      ShutdownHooks: () => ShutdownHooks(logger: globals.logger),
       Stdio: () => Stdio(),
       SystemClock: () => const SystemClock(),
       Usage: () => Usage(

packages/flutter_tools/lib/src/globals.dart

@@ -248,7 +248,11 @@ PlistParser? _plistInstance;
 /// The global template renderer.
 TemplateRenderer get templateRenderer => context.get<TemplateRenderer>()!;
 
-ShutdownHooks? get shutdownHooks => context.get<ShutdownHooks>();
+/// Global [ShutdownHooks] that should be run before the tool process exits.
+///
+/// This is depended on by [localFileSystem] which is called before any
+/// [Context] is set up, and thus this cannot be a Context getter.
+final ShutdownHooks shutdownHooks = ShutdownHooks();
 
 // Unless we're in a test of this class's signal handling features, we must
 // have only one instance created with the singleton LocalSignals instance

packages/flutter_tools/test/general.shard/base/file_system_test.dart

@@ -10,6 +10,7 @@ import 'package:file_testing/file_testing.dart';
 import 'package:flutter_tools/src/base/common.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/signals.dart';
 import 'package:test/fake.dart';
@@ -162,6 +163,21 @@ void main() {
       signalUnderTest = ProcessSignal(fakeSignal);
     });
 
+    testWithoutContext('runs shutdown hooks', () async {
+      final Signals signals = Signals.test();
+      final LocalFileSystem localFileSystem = LocalFileSystem.test(
+        signals: signals,
+      );
+      final Directory temp = localFileSystem.systemTempDirectory;
+
+      expect(temp.existsSync(), isTrue);
+      expect(localFileSystem.shutdownHooks.registeredHooks, hasLength(1));
+      final BufferLogger logger = BufferLogger.test();
+      await localFileSystem.shutdownHooks.runShutdownHooks(logger);
+      expect(temp.existsSync(), isFalse);
+      expect(logger.traceText, contains('Running 1 shutdown hook'));
+    });
+
     testWithoutContext('deletes system temp entry on a fatal signal', () async {
       final Completer<void> completer = Completer<void>();
       final Signals signals = Signals.test();

packages/flutter_tools/test/general.shard/base/process_test.dart

@@ -42,13 +42,13 @@ void main() {
       int i = 1;
       int? cleanup;
 
-      final ShutdownHooks shutdownHooks = ShutdownHooks(logger: BufferLogger.test());
+      final ShutdownHooks shutdownHooks = ShutdownHooks();
 
       shutdownHooks.addShutdownHook(() async {
         cleanup = i++;
       });
 
-      await shutdownHooks.runShutdownHooks();
+      await shutdownHooks.runShutdownHooks(BufferLogger.test());
 
       expect(cleanup, 1);
     });

packages/flutter_tools/test/general.shard/runner/runner_test.dart

@@ -13,6 +13,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart' as io;
 import 'package:flutter_tools/src/base/net.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/process.dart';
 import 'package:flutter_tools/src/base/user_messages.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
@@ -68,6 +69,7 @@ void main() {
             // This flutterVersion disables crash reporting.
             flutterVersion: '[user-branch]/',
             reportCrashes: true,
+            shutdownHooks: ShutdownHooks(),
           ));
           return null;
         },
@@ -120,6 +122,7 @@ void main() {
             // This flutterVersion disables crash reporting.
             flutterVersion: '[user-branch]/',
             reportCrashes: true,
+            shutdownHooks: ShutdownHooks(),
           ));
           return null;
         },
@@ -159,16 +162,17 @@ void main() {
       // catch it in a zone.
       unawaited(runZoned<Future<void>>(
         () {
-        unawaited(runner.run(
+          unawaited(runner.run(
-          <String>['crash'],
+            <String>['crash'],
-          () => <FlutterCommand>[
+            () => <FlutterCommand>[
-            CrashingFlutterCommand(),
+              CrashingFlutterCommand(),
-          ],
+            ],
-          // This flutterVersion disables crash reporting.
+            // This flutterVersion disables crash reporting.
-          flutterVersion: '[user-branch]/',
+            flutterVersion: '[user-branch]/',
-          reportCrashes: true,
+            reportCrashes: true,
-        ));
+            shutdownHooks: ShutdownHooks(),
-        return null;
+          ));
+          return null;
         },
         onError: (Object error, StackTrace stack) { // ignore: deprecated_member_use
           expect(firstExitCode, isNotNull);

packages/flutter_tools/test/integration.shard/command_output_test.dart

@@ -68,7 +68,7 @@ void main() {
     ]);
 
     // Check for message only printed in verbose mode.
-    expect(result.stdout, contains('Running shutdown hooks'));
+    expect(result.stdout, contains('Shutdown hooks complete'));
   });
 
   testWithoutContext('flutter config contains all features', () async {

packages/flutter_tools/test/integration.shard/overall_experience_test.dart

@@ -384,7 +384,8 @@ void main() {
             return null;
           }),
           Barrier('Performing hot restart...'.padRight(progressMessageWidth)),
-          Multiple(<Pattern>[RegExp(r'^Restarted application in [0-9]+ms.$'), 'called main', 'called paint'], handler: (String line) {
+          // This could look like 'Restarted application in 1,237ms.'
+          Multiple(<Pattern>[RegExp(r'^Restarted application in .+m?s.$'), 'called main', 'called paint'], handler: (String line) {
             return 'q';
           }),
           const Barrier('Application finished.'),