Delinting future awaits round 3 (#10791) * round 3 * partially address comments * merge * review notes * review * review * review
diff --git a/packages/flutter_tools/bin/fuchsia_builder.dart b/packages/flutter_tools/bin/fuchsia_builder.dart index e0d1ff8..9c8d146 100644 --- a/packages/flutter_tools/bin/fuchsia_builder.dart +++ b/packages/flutter_tools/bin/fuchsia_builder.dart
@@ -42,7 +42,7 @@ Future<Null> main(List<String> args) async { final AppContext executableContext = new AppContext(); executableContext.setVariable(Logger, new StdoutLogger()); - executableContext.runInZone(() { + await executableContext.runInZone(() { // Initialize the context with some defaults. context.putIfAbsent(Platform, () => const LocalPlatform()); context.putIfAbsent(FileSystem, () => const LocalFileSystem());
diff --git a/packages/flutter_tools/bin/fuchsia_tester.dart b/packages/flutter_tools/bin/fuchsia_tester.dart index 8abd2c2..d357834 100644 --- a/packages/flutter_tools/bin/fuchsia_tester.dart +++ b/packages/flutter_tools/bin/fuchsia_tester.dart
@@ -38,7 +38,7 @@ Future<Null> main(List<String> args) async { final AppContext executableContext = new AppContext(); executableContext.setVariable(Logger, new StdoutLogger()); - executableContext.runInZone(() { + await executableContext.runInZone(() { // Initialize the context with some defaults. context.putIfAbsent(Platform, () => const LocalPlatform()); context.putIfAbsent(FileSystem, () => const LocalFileSystem());
diff --git a/packages/flutter_tools/lib/src/commands/logs.dart b/packages/flutter_tools/lib/src/commands/logs.dart index d10eec8..dd52f2f 100644 --- a/packages/flutter_tools/lib/src/commands/logs.dart +++ b/packages/flutter_tools/lib/src/commands/logs.dart
@@ -73,7 +73,7 @@ // Wait for the log reader to be finished. final int result = await exitCompleter.future; - subscription.cancel(); + await subscription.cancel(); if (result != 0) throwToolExit('Error listening to $logReader logs.'); }
diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 17651d5..0d7e8d7 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart
@@ -340,7 +340,8 @@ // // Do not add more operations to the future. final Completer<Null> appStartedTimeRecorder = new Completer<Null>.sync(); - appStartedTimeRecorder.future.then( + // This callback can't throw. + appStartedTimeRecorder.future.then( // ignore: unawaited_futures (_) { appStartedTime = clock.now(); } );
diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index b95d1ae..938d5de 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart
@@ -286,7 +286,8 @@ } catch (e) { if (retry < kMaxRetries) { printTrace('Retrying writing "$deviceUri" to DevFS due to error: $e'); - _scheduleWrite(deviceUri, content, retry + 1); + // Synchronization is handled by the _completer below. + _scheduleWrite(deviceUri, content, retry + 1); // ignore: unawaited_futures return; } else { printError('Error writing "$deviceUri" to DevFS: $e');
diff --git a/packages/flutter_tools/lib/src/ios/code_signing.dart b/packages/flutter_tools/lib/src/ios/code_signing.dart index e45eda0..647015a 100644 --- a/packages/flutter_tools/lib/src/ios/code_signing.dart +++ b/packages/flutter_tools/lib/src/ios/code_signing.dart
@@ -142,11 +142,12 @@ ..close(); final String opensslOutput = await UTF8.decodeStream(opensslProcess.stdout); - opensslProcess.stderr.drain<String>(); + // Fire and forget discard of the stderr stream so we don't hold onto resources. + // Don't care about the result. + opensslProcess.stderr.drain<String>(); // ignore: unawaited_futures - if (await opensslProcess.exitCode != 0) { + if (await opensslProcess.exitCode != 0) return null; - } return _certificateOrganizationalUnitExtractionPattern .firstMatch(opensslOutput)
diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index be27c60..75b5ccb 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart
@@ -237,7 +237,7 @@ Future<bool> installApp(ApplicationPackage app) async { try { final IOSApp iosApp = app; - SimControl.instance.install(id, iosApp.simulatorBundlePath); + await SimControl.instance.install(id, iosApp.simulatorBundlePath); return true; } catch (e) { return false; @@ -382,7 +382,7 @@ printError('Error waiting for a debug connection: $error'); return new LaunchResult.failed(); } finally { - observatoryDiscovery.cancel(); + await observatoryDiscovery.cancel(); } } @@ -556,7 +556,9 @@ _systemProcess.stderr.transform(UTF8.decoder).transform(const LineSplitter()).listen(_onSystemLine); } - _deviceProcess.exitCode.whenComplete(() { + // We don't want to wait for the process or its callback. Best effort + // cleanup in the callback. + _deviceProcess.exitCode.whenComplete(() { // ignore: unawaited_futures if (_linesController.hasListener) _linesController.close(); });
diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 3e1db2a..2c5f646 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -99,8 +99,10 @@ if (flutterViews == null || flutterViews.isEmpty) return; for (FlutterView view in flutterViews) { - if (view != null && view.uiIsolate != null) - view.uiIsolate.flutterExit(); + if (view != null && view.uiIsolate != null) { + // Manage waits specifically below. + view.uiIsolate.flutterExit(); // ignore: unawaited_futures + } } await new Future<Null>.delayed(const Duration(milliseconds: 100)); } @@ -563,8 +565,9 @@ } Future<Null> stopEchoingDeviceLog() async { - for (FlutterDevice device in flutterDevices) - device.stopEchoingDeviceLog(); + await Future.wait( + flutterDevices.map((FlutterDevice device) => device.stopEchoingDeviceLog()) + ); } /// If the [reloadSources] parameter is not null the 'reloadSources' service @@ -591,7 +594,10 @@ // Listen for service protocol connection to close. for (FlutterDevice device in flutterDevices) { for (VMService service in device.vmServices) { - service.done.then<Null>( + // This hooks up callbacks for when the connection stops in the future. + // We don't want to wait for them. We don't handle errors in those callbacks' + // futures either because they just print to logger and is not critical. + service.done.then<Null>( // ignore: unawaited_futures _serviceProtocolDone, onError: _serviceProtocolError ).whenComplete(_serviceDisconnected);
diff --git a/packages/flutter_tools/lib/src/test/coverage_collector.dart b/packages/flutter_tools/lib/src/test/coverage_collector.dart index 21b80e0..1925ed5 100644 --- a/packages/flutter_tools/lib/src/test/coverage_collector.dart +++ b/packages/flutter_tools/lib/src/test/coverage_collector.dart
@@ -42,7 +42,10 @@ final int pid = process.pid; int exitCode; - process.exitCode.then<Null>((int code) { + // Synchronization is enforced by the API contract. Error handling + // synchronization is done in the code below where `exitCode` is checked. + // Callback cannot throw. + process.exitCode.then<Null>((int code) { // ignore: unawaited_futures exitCode = code; }); if (exitCode != null)
diff --git a/packages/flutter_tools/lib/src/test/flutter_platform.dart b/packages/flutter_tools/lib/src/test/flutter_platform.dart index e1bd527..2f26b6f 100644 --- a/packages/flutter_tools/lib/src/test/flutter_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart
@@ -153,7 +153,8 @@ bool subprocessActive = false; bool controllerSinkClosed = false; try { - controller.sink.done.whenComplete(() { controllerSinkClosed = true; }); + // Callback can't throw since it's just setting a variable. + controller.sink.done.whenComplete(() { controllerSinkClosed = true; }); // ignore: unawaited_futures // Prepare our WebSocket server to talk to the engine subproces. final HttpServer server = await HttpServer.bind(host, 0); @@ -272,7 +273,8 @@ subprocessActive = false; final String message = _getErrorMessage(_getExitCodeMessage(exitCode, 'before connecting to test harness'), testPath, shellPath); controller.sink.addError(message); - controller.sink.close(); + // Awaited for with 'sink.done' below. + controller.sink.close(); // ignore: unawaited_futures printTrace('test $ourTestCount: waiting for controller sink to close'); await controller.sink.done; break; @@ -280,7 +282,8 @@ printTrace('test $ourTestCount: timed out waiting for process with pid ${process.pid} to connect to test harness'); final String message = _getErrorMessage('Test never connected to test harness.', testPath, shellPath); controller.sink.addError(message); - controller.sink.close(); + // Awaited for with 'sink.done' below. + controller.sink.close(); // ignore: unawaited_futures printTrace('test $ourTestCount: waiting for controller sink to close'); await controller.sink.done; break; @@ -332,8 +335,10 @@ testDone.future.then<_TestResult>((Null _) { return _TestResult.testBailed; }), ]); - harnessToTest.cancel(); - testToHarness.cancel(); + await Future.wait(<Future<Null>>[ + harnessToTest.cancel(), + testToHarness.cancel(), + ]); switch (testResult) { case _TestResult.crashed: @@ -342,7 +347,8 @@ subprocessActive = false; final String message = _getErrorMessage(_getExitCodeMessage(exitCode, 'before test harness closed its WebSocket'), testPath, shellPath); controller.sink.addError(message); - controller.sink.close(); + // Awaited for with 'sink.done' below. + controller.sink.close(); // ignore: unawaited_futures printTrace('test $ourTestCount: waiting for controller sink to close'); await controller.sink.done; break; @@ -384,7 +390,8 @@ } } if (!controllerSinkClosed) { - controller.sink.close(); + // Waiting below with await. + controller.sink.close(); // ignore: unawaited_futures printTrace('test $ourTestCount: waiting for controller sink to close'); await controller.sink.done; }