diff --git a/packages/flutter_tools/lib/executable.dart b/packages/flutter_tools/lib/executable.dart index 546dc98ae73f..a26aeb523172 100644 --- a/packages/flutter_tools/lib/executable.dart +++ b/packages/flutter_tools/lib/executable.dart @@ -127,6 +127,7 @@ Future main(List args) async { }, PreRunValidator: () => PreRunValidator(fileSystem: globals.fs), }, + shutdownHooks: globals.shutdownHooks, ); } diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index cda24d777003..9da36edd0f61 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -34,6 +34,7 @@ Future run( bool? reportCrashes, String? flutterVersion, Map? 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 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 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 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 _handleToolError( List 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 -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 _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 _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 _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 _createLocalCrashReport(CrashDetails details) async { return crashFile; } -Future _exit(int code) async { +Future _exit(int code, {required ShutdownHooks shutdownHooks}) async { // Prints the welcome message if needed. globals.flutterUsage.printWelcome(); @@ -241,7 +243,7 @@ Future _exit(int code) async { } // Run shutdown hooks before flushing logs - await globals.shutdownHooks!.runShutdownHooks(); + await shutdownHooks.runShutdownHooks(globals.logger); final Completer completer = Completer(); diff --git a/packages/flutter_tools/lib/src/base/file_system.dart b/packages/flutter_tools/lib/src/base/file_system.dart index 7feb83fea14e..558028903429 100644 --- a/packages/flutter_tools/lib/src/base/file_system.dart +++ b/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 fatalSignals = Signals.defaultExitSignals, - }) : this(signals, fatalSignals, null); + }) : this(signals, fatalSignals, ShutdownHooks()); Directory? _systemTemp; final Map _signalTokens = {}; - final ShutdownHooks? _shutdownHooks; + + final ShutdownHooks shutdownHooks; Future 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, ); } diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index f37989fb5f37..5670a075f14d 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/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 Function(); +typedef ShutdownHook = FutureOr 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 Function(); // for more details. abstract class ShutdownHooks { - factory ShutdownHooks({ - required Logger logger, - }) => _DefaultShutdownHooks( - logger: logger, - ); + factory ShutdownHooks() => _DefaultShutdownHooks(); /// Registers a [ShutdownHook] to be executed before the VM exits. void addShutdownHook( ShutdownHook shutdownHook ); + @visibleForTesting + List 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 runShutdownHooks(); + /// + /// This class is constructed before the [Logger], so it cannot be direct + /// injected in the constructor. + Future runShutdownHooks(Logger logger); } class _DefaultShutdownHooks implements ShutdownHooks { - _DefaultShutdownHooks({ - required Logger logger, - }) : _logger = logger; + _DefaultShutdownHooks(); - final Logger _logger; - final List _shutdownHooks = []; + @override + final List registeredHooks = []; bool _shutdownHooksRunning = false; @@ -58,16 +59,18 @@ class _DefaultShutdownHooks implements ShutdownHooks { ShutdownHook shutdownHook ) { assert(!_shutdownHooksRunning); - _shutdownHooks.add(shutdownHook); + registeredHooks.add(shutdownHook); } @override - Future runShutdownHooks() async { - _logger.printTrace('Running shutdown hooks'); + Future runShutdownHooks(Logger logger) async { + logger.printTrace( + 'Running ${registeredHooks.length} shutdown hook${registeredHooks.length == 1 ? '' : 's'}', + ); _shutdownHooksRunning = true; try { final List> futures = >[]; - for (final ShutdownHook shutdownHook in _shutdownHooks) { + for (final ShutdownHook shutdownHook in registeredHooks) { final FutureOr result = shutdownHook(); if (result is Future) { futures.add(result); @@ -77,7 +80,7 @@ class _DefaultShutdownHooks implements ShutdownHooks { } finally { _shutdownHooksRunning = false; } - _logger.printTrace('Shutdown hooks complete'); + logger.printTrace('Shutdown hooks complete'); } } diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index d35bdb5f3d14..ca5369c55d34 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -313,7 +313,6 @@ Future runInContext( platform: globals.platform, usage: globals.flutterUsage, ), - ShutdownHooks: () => ShutdownHooks(logger: globals.logger), Stdio: () => Stdio(), SystemClock: () => const SystemClock(), Usage: () => Usage( diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index 778790fc53d3..260eaf56653e 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -248,7 +248,11 @@ PlistParser? _plistInstance; /// The global template renderer. TemplateRenderer get templateRenderer => context.get()!; -ShutdownHooks? get shutdownHooks => context.get(); +/// 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 diff --git a/packages/flutter_tools/test/general.shard/base/file_system_test.dart b/packages/flutter_tools/test/general.shard/base/file_system_test.dart index a3bbad5e198b..e1bd3c3d7364 100644 --- a/packages/flutter_tools/test/general.shard/base/file_system_test.dart +++ b/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 completer = Completer(); final Signals signals = Signals.test(); diff --git a/packages/flutter_tools/test/general.shard/base/process_test.dart b/packages/flutter_tools/test/general.shard/base/process_test.dart index 208ceb22e2fc..51e6c2d1020b 100644 --- a/packages/flutter_tools/test/general.shard/base/process_test.dart +++ b/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); }); diff --git a/packages/flutter_tools/test/general.shard/runner/runner_test.dart b/packages/flutter_tools/test/general.shard/runner/runner_test.dart index fc8d7fd6c7bf..4c59a501039b 100644 --- a/packages/flutter_tools/test/general.shard/runner/runner_test.dart +++ b/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>( () { - unawaited(runner.run( - ['crash'], - () => [ - CrashingFlutterCommand(), - ], - // This flutterVersion disables crash reporting. - flutterVersion: '[user-branch]/', - reportCrashes: true, - )); - return null; + unawaited(runner.run( + ['crash'], + () => [ + CrashingFlutterCommand(), + ], + // This flutterVersion disables crash reporting. + flutterVersion: '[user-branch]/', + reportCrashes: true, + shutdownHooks: ShutdownHooks(), + )); + return null; }, onError: (Object error, StackTrace stack) { // ignore: deprecated_member_use expect(firstExitCode, isNotNull); diff --git a/packages/flutter_tools/test/integration.shard/command_output_test.dart b/packages/flutter_tools/test/integration.shard/command_output_test.dart index 0023ce8753b8..158f34807763 100644 --- a/packages/flutter_tools/test/integration.shard/command_output_test.dart +++ b/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 { diff --git a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart b/packages/flutter_tools/test/integration.shard/overall_experience_test.dart index 887f9b7ab145..1d1511f8d84d 100644 --- a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart +++ b/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([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([RegExp(r'^Restarted application in .+m?s.$'), 'called main', 'called paint'], handler: (String line) { return 'q'; }), const Barrier('Application finished.'),