Skip to content

Commit

Permalink
Switch many Device methods to be async (#9587)
Browse files Browse the repository at this point in the history 
`adb` can sometimes hang, which will in turn hang the Dart isolate if
we're using `Process.runSync()`. This changes many of the `Device` methods
to return `Future<T>` in order to allow them to use the async process
methods. A future change will add timeouts to the associated calls so
that we can properly alert the user to the hung `adb` process.

This is work towards #7102, #9567
  • Loading branch information
@tvolkert
tvolkert committed Apr 26, 2017
1 parent 596eb03 commit 60c5ffc
Show file tree
Hide file tree
Showing 29 changed files with 250 additions and 242 deletions.
82 changes: 41 additions & 41 deletions packages/flutter_tools/lib/src/android/android_device.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class AndroidDevice extends Device {
bool _isLocalEmulator;
TargetPlatform _platform;

String _getProperty(String name) {
Future<String> _getProperty(String name) async {
if (_properties == null) {
_properties = <String, String>{};

Expand Down Expand Up @@ -79,19 +79,19 @@ class AndroidDevice extends Device {
}

@override
bool get isLocalEmulator {
Future<bool> get isLocalEmulator async {
if (_isLocalEmulator == null) {
final String characteristics = _getProperty('ro.build.characteristics');
final String characteristics = await _getProperty('ro.build.characteristics');
_isLocalEmulator = characteristics != null && characteristics.contains('emulator');
}
return _isLocalEmulator;
}

@override
TargetPlatform get targetPlatform {
Future<TargetPlatform> get targetPlatform async {
if (_platform == null) {
// http://developer.android.com/ndk/guides/abis.html (x86, armeabi-v7a, ...)
switch (_getProperty('ro.product.cpu.abi')) {
switch (await _getProperty('ro.product.cpu.abi')) {
case 'x86_64':
_platform = TargetPlatform.android_x64;
break;
Expand All @@ -108,11 +108,12 @@ class AndroidDevice extends Device {
}

@override
String get sdkNameAndVersion => 'Android $_sdkVersion (API $_apiVersion)';
Future<String> get sdkNameAndVersion async =>
'Android ${await _sdkVersion} (API ${await _apiVersion})';

String get _sdkVersion => _getProperty('ro.build.version.release');
Future<String> get _sdkVersion => _getProperty('ro.build.version.release');

String get _apiVersion => _getProperty('ro.build.version.sdk');
Future<String> get _apiVersion => _getProperty('ro.build.version.sdk');

_AdbLogReader _logReader;
_AndroidDevicePortForwarder _portForwarder;
Expand Down Expand Up @@ -160,16 +161,16 @@ class AndroidDevice extends Device {
return false;
}

bool _checkForSupportedAndroidVersion() {
Future<bool> _checkForSupportedAndroidVersion() async {
try {
// If the server is automatically restarted, then we get irrelevant
// output lines like this, which we want to ignore:
// adb server is out of date. killing..
// * daemon started successfully *
runCheckedSync(<String>[getAdbPath(androidSdk), 'start-server']);
await runCheckedAsync(<String>[getAdbPath(androidSdk), 'start-server']);

// Sample output: '22'
final String sdkVersion = _getProperty('ro.build.version.sdk');
final String sdkVersion = await _getProperty('ro.build.version.sdk');

final int sdkVersionParsed = int.parse(sdkVersion, onError: (String source) => null);
if (sdkVersionParsed == null) {
Expand All @@ -195,8 +196,9 @@ class AndroidDevice extends Device {
return '/data/local/tmp/sky.${app.id}.sha1';
}

String _getDeviceApkSha1(ApplicationPackage app) {
return runSync(adbCommandForDevice(<String>['shell', 'cat', _getDeviceSha1Path(app)]));
Future<String> _getDeviceApkSha1(ApplicationPackage app) async {
final RunResult result = await runAsync(adbCommandForDevice(<String>['shell', 'cat', _getDeviceSha1Path(app)]));
return result.stdout;
}

String _getSourceSha1(ApplicationPackage app) {
Expand All @@ -209,15 +211,15 @@ class AndroidDevice extends Device {
String get name => modelID;

@override
bool isAppInstalled(ApplicationPackage app) {
Future<bool> isAppInstalled(ApplicationPackage app) async {
// This call takes 400ms - 600ms.
final String listOut = runCheckedSync(adbCommandForDevice(<String>['shell', 'pm', 'list', 'packages', app.id]));
return LineSplitter.split(listOut).contains("package:${app.id}");
final RunResult listOut = await runCheckedAsync(adbCommandForDevice(<String>['shell', 'pm', 'list', 'packages', app.id]));
return LineSplitter.split(listOut.stdout).contains("package:${app.id}");
}

@override
bool isLatestBuildInstalled(ApplicationPackage app) {
final String installedSha1 = _getDeviceApkSha1(app);
Future<bool> isLatestBuildInstalled(ApplicationPackage app) async {
final String installedSha1 = await _getDeviceApkSha1(app);
return installedSha1.isNotEmpty && installedSha1 == _getSourceSha1(app);
}

Expand All @@ -229,7 +231,7 @@ class AndroidDevice extends Device {
return false;
}

if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion())
if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion())
return false;

final Status status = logger.startProgress('Installing ${apk.apkPath}...', expectSlowOperation: true);
Expand All @@ -249,11 +251,11 @@ class AndroidDevice extends Device {
}

@override
bool uninstallApp(ApplicationPackage app) {
if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion())
Future<bool> uninstallApp(ApplicationPackage app) async {
if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion())
return false;

final String uninstallOut = runCheckedSync(adbCommandForDevice(<String>['uninstall', app.id]));
final String uninstallOut = (await runCheckedAsync(adbCommandForDevice(<String>['uninstall', app.id]))).stdout;
final RegExp failureExp = new RegExp(r'^Failure.*$', multiLine: true);
final String failure = failureExp.stringMatch(uninstallOut);
if (failure != null) {
Expand All @@ -265,9 +267,9 @@ class AndroidDevice extends Device {
}

Future<bool> _installLatestApp(ApplicationPackage package) async {
final bool wasInstalled = isAppInstalled(package);
final bool wasInstalled = await isAppInstalled(package);
if (wasInstalled) {
if (isLatestBuildInstalled(package)) {
if (await isLatestBuildInstalled(package)) {
printTrace('Latest build already installed.');
return true;
}
Expand All @@ -277,7 +279,7 @@ class AndroidDevice extends Device {
printTrace('Warning: Failed to install APK.');
if (wasInstalled) {
printStatus('Uninstalling old version...');
if (!uninstallApp(package)) {
if (!await uninstallApp(package)) {
printError('Error: Uninstalling old version failed.');
return false;
}
Expand All @@ -304,10 +306,10 @@ class AndroidDevice extends Device {
bool prebuiltApplication: false,
bool applicationNeedsRebuild: false,
}) async {
if (!_checkForSupportedAdbVersion() || !_checkForSupportedAndroidVersion())
if (!_checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion())
return new LaunchResult.failed();

if (targetPlatform != TargetPlatform.android_arm && mode != BuildMode.debug) {
if (await targetPlatform != TargetPlatform.android_arm && mode != BuildMode.debug) {
printError('Profile and release builds are only supported on ARM targets.');
return new LaunchResult.failed();
}
Expand Down Expand Up @@ -369,7 +371,7 @@ class AndroidDevice extends Device {
cmd.addAll(<String>['--ez', 'use-test-fonts', 'true']);
}
cmd.add(apk.launchActivity);
final String result = runCheckedSync(cmd);
final String result = (await runCheckedAsync(cmd)).stdout;
// This invocation returns 0 even when it fails.
if (result.contains('Error: ')) {
printError(result.trim());
Expand Down Expand Up @@ -455,16 +457,15 @@ class AndroidDevice extends Device {
bool get supportsScreenshot => true;

@override
Future<Null> takeScreenshot(File outputFile) {
Future<Null> takeScreenshot(File outputFile) async {
const String remotePath = '/data/local/tmp/flutter_screenshot.png';
runCheckedSync(adbCommandForDevice(<String>['shell', 'screencap', '-p', remotePath]));
runCheckedSync(adbCommandForDevice(<String>['pull', remotePath, outputFile.path]));
runCheckedSync(adbCommandForDevice(<String>['shell', 'rm', remotePath]));
return new Future<Null>.value();
await runCheckedAsync(adbCommandForDevice(<String>['shell', 'screencap', '-p', remotePath]));
await runCheckedAsync(adbCommandForDevice(<String>['pull', remotePath, outputFile.path]));
await runCheckedAsync(adbCommandForDevice(<String>['shell', 'rm', remotePath]));
}

@override
Future<List<DiscoveredApp>> discoverApps() {
Future<List<DiscoveredApp>> discoverApps() async {
final RegExp discoverExp = new RegExp(r'DISCOVER: (.*)');
final List<DiscoveredApp> result = <DiscoveredApp>[];
final StreamSubscription<String> logs = getLogReader().logLines.listen((String line) {
Expand All @@ -475,14 +476,13 @@ class AndroidDevice extends Device {
}
});

runCheckedSync(adbCommandForDevice(<String>[
await runCheckedAsync(adbCommandForDevice(<String>[
'shell', 'am', 'broadcast', '-a', 'io.flutter.view.DISCOVER'
]));

return new Future<List<DiscoveredApp>>.delayed(const Duration(seconds: 1), () {
logs.cancel();
return result;
});
await new Future<Null>.delayed(const Duration(seconds: 1));
logs.cancel();
return result;
}
}

Expand Down Expand Up @@ -744,7 +744,7 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder {
hostPort = await portScanner.findAvailablePort();
}

runCheckedSync(device.adbCommandForDevice(
await runCheckedAsync(device.adbCommandForDevice(
<String>['forward', 'tcp:$hostPort', 'tcp:$devicePort']
));

Expand All @@ -753,7 +753,7 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder {

@override
Future<Null> unforward(ForwardedPort forwardedPort) async {
runCheckedSync(device.adbCommandForDevice(
await runCheckedAsync(device.adbCommandForDevice(
<String>['forward', '--remove', 'tcp:${forwardedPort.hostPort}']
));
}
Expand Down
19 changes: 9 additions & 10 deletions packages/flutter_tools/lib/src/base/process.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,15 @@ bool exitsHappy(List<String> cli) {
}
}

Future<bool> exitsHappyAsync(List<String> cli) async {
_traceCommand(cli);
try {
return (await processManager.run(cli)).exitCode == 0;
} catch (error) {
return false;
}
}

/// Run cmd and return stdout.
///
/// Throws an error if cmd exits with a non-zero value.
Expand All @@ -241,16 +250,6 @@ String runCheckedSync(List<String> cmd, {
);
}

/// Run cmd and return stdout on success.
///
/// Throws the standard error output if cmd exits with a non-zero value.
String runSyncAndThrowStdErrOnError(List<String> cmd) {
return _runWithLoggingSync(cmd,
checked: true,
throwStandardErrorOnError: true,
hideStdout: true);
}

/// Run cmd and return stdout.
String runSync(List<String> cmd, {
String workingDirectory,
Expand Down
16 changes: 8 additions & 8 deletions packages/flutter_tools/lib/src/commands/build_aot.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,24 +273,24 @@ Future<String> _buildAotSnapshot(
final List<String> commonBuildOptions = <String>['-arch', 'arm64', '-miphoneos-version-min=8.0'];

if (interpreter) {
runCheckedSync(<String>['mv', vmSnapshotData, fs.path.join(outputDir.path, kVmSnapshotData)]);
runCheckedSync(<String>['mv', isolateSnapshotData, fs.path.join(outputDir.path, kIsolateSnapshotData)]);
await runCheckedAsync(<String>['mv', vmSnapshotData, fs.path.join(outputDir.path, kVmSnapshotData)]);
await runCheckedAsync(<String>['mv', isolateSnapshotData, fs.path.join(outputDir.path, kIsolateSnapshotData)]);

runCheckedSync(<String>[
await runCheckedAsync(<String>[
'xxd', '--include', kVmSnapshotData, fs.path.basename(kVmSnapshotDataC)
], workingDirectory: outputDir.path);
runCheckedSync(<String>[
await runCheckedAsync(<String>[
'xxd', '--include', kIsolateSnapshotData, fs.path.basename(kIsolateSnapshotDataC)
], workingDirectory: outputDir.path);

runCheckedSync(<String>['xcrun', 'cc']
await runCheckedAsync(<String>['xcrun', 'cc']
..addAll(commonBuildOptions)
..addAll(<String>['-c', kVmSnapshotDataC, '-o', kVmSnapshotDataO]));
runCheckedSync(<String>['xcrun', 'cc']
await runCheckedAsync(<String>['xcrun', 'cc']
..addAll(commonBuildOptions)
..addAll(<String>['-c', kIsolateSnapshotDataC, '-o', kIsolateSnapshotDataO]));
} else {
runCheckedSync(<String>['xcrun', 'cc']
await runCheckedAsync(<String>['xcrun', 'cc']
..addAll(commonBuildOptions)
..addAll(<String>['-c', assembly, '-o', assemblyO]));
}
Expand All @@ -313,7 +313,7 @@ Future<String> _buildAotSnapshot(
} else {
linkCommand.add(assemblyO);
}
runCheckedSync(linkCommand);
await runCheckedAsync(linkCommand);
}

return outputPath;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/commands/config.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class ConfigCommand extends FlutterCommand {

/// Return `null` to disable tracking of the `config` command.
@override
String get usagePath => null;
Future<String> get usagePath => null;

@override
Future<Null> runCommand() async {
Expand Down
31 changes: 17 additions & 14 deletions packages/flutter_tools/lib/src/commands/daemon.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ class AppDomain extends Domain {
String packagesFilePath,
String projectAssets,
}) async {
if (device.isLocalEmulator && !isEmulatorBuildMode(options.buildMode))
if (await device.isLocalEmulator && !isEmulatorBuildMode(options.buildMode))
throw '${toTitleCase(getModeName(options.buildMode))} mode is not supported for emulators.';

// We change the current working directory for the duration of the `start` command.
Expand Down Expand Up @@ -381,7 +381,7 @@ class AppDomain extends Domain {
_sendAppEvent(app, 'start', <String, dynamic>{
'deviceId': device.id,
'directory': projectDirectory,
'supportsRestart': isRestartSupported(enableHotReload, device)
'supportsRestart': isRestartSupported(enableHotReload, device),
});

Completer<DebugConnectionInfo> connectionInfoCompleter;
Expand Down Expand Up @@ -505,6 +505,8 @@ class AppDomain extends Domain {
}
}

typedef void _DeviceEventHandler(Device device);

/// This domain lets callers list and monitor connected devices.
///
/// It exports a `getDevices()` call, as well as firing `device.added` and
Expand All @@ -530,15 +532,20 @@ class DeviceDomain extends Domain {
_discoverers.add(deviceDiscovery);

for (PollingDeviceDiscovery discoverer in _discoverers) {
discoverer.onAdded.listen((Device device) {
sendEvent('device.added', _deviceToMap(device));
});
discoverer.onRemoved.listen((Device device) {
sendEvent('device.removed', _deviceToMap(device));
});
discoverer.onAdded.listen(_onDeviceEvent('device.added'));
discoverer.onRemoved.listen(_onDeviceEvent('device.removed'));
}
}

Future<Null> _deviceEvents = new Future<Null>.value();
_DeviceEventHandler _onDeviceEvent(String eventName) {
return (Device device) {
_deviceEvents = _deviceEvents.then((_) async {
sendEvent(eventName, await _deviceToMap(device));
});
};
}

final List<PollingDeviceDiscovery> _discoverers = <PollingDeviceDiscovery>[];

Future<List<Device>> getDevices([Map<String, dynamic> args]) {
Expand Down Expand Up @@ -638,18 +645,16 @@ void stdoutCommandResponse(Map<String, dynamic> command) {
}

dynamic _jsonEncodeObject(dynamic object) {
if (object is Device)
return _deviceToMap(object);
if (object is OperationResult)
return _operationResultToMap(object);
return object;
}

Map<String, dynamic> _deviceToMap(Device device) {
Future<Map<String, dynamic>> _deviceToMap(Device device) async {
return <String, dynamic>{
'id': device.id,
'name': device.name,
'platform': getNameForTargetPlatform(device.targetPlatform),
'platform': getNameForTargetPlatform(await device.targetPlatform),
'emulator': device.isLocalEmulator

This comment has been minimized.

Sign in to view

Copy link
@devoncarew

devoncarew Apr 26, 2017

Member

@tvolkert, I think this line is missing an await

This comment has been minimized.

Sign in to view

Copy link
@tvolkert

tvolkert Apr 26, 2017

Author Contributor

Sent #9617 to fix
};
}
Expand All @@ -664,8 +669,6 @@ Map<String, dynamic> _operationResultToMap(OperationResult result) {
dynamic _toJsonable(dynamic obj) {
if (obj is String || obj is int || obj is bool || obj is Map<dynamic, dynamic> || obj is List<dynamic> || obj == null)
return obj;
if (obj is Device)
return obj;
if (obj is OperationResult)
return obj;
return '$obj';
Expand Down
Loading

0 comments on commit 60c5ffc

Please sign in to comment.