Whitelist adb.exe heap corruption exit code. (#33951)

* Whitelist adb.exe heap corruption exit code.

In android platform tools 29.0.0 adb.exe shell seems to be exiting with heap corruption exit code, otherwise producing results as expected. This PR whitelists this exit code on Windows.

Fixes https://github.com/flutter/flutter/issues/33938.

* Fix condition

* Fix 'shell am start' command

* Fix stop command

* Refactor into runAdbMostlyChecked(Sync/Async)

* runAdbMostlyChecked -> runAdbChecked

packages/flutter_tools/lib/src/android/android_device.dart

@@ -14,6 +14,7 @@ import '../base/common.dart' show throwToolExit;
 import '../base/file_system.dart';
 import '../base/io.dart';
 import '../base/logger.dart';
+import '../base/platform.dart';
 import '../base/process.dart';
 import '../base/process_manager.dart';
 import '../build_info.dart';
@@ -42,6 +43,13 @@ const Map<String, _HardwareType> _knownHardware = <String, _HardwareType>{
   'samsungexynos9810': _HardwareType.physical,
 };
 
+bool allowHeapCorruptionOnWindows(int exitCode) {
+  // In platform tools 29.0.0 adb.exe seems to be ending with this heap
+  // corruption error code on seemingly successful termination.
+  // So we ignore this error on Windows.
+  return exitCode == -1073740940 && platform.isWindows;
+}
+
 class AndroidDevices extends PollingDeviceDiscovery {
   AndroidDevices() : super('Android devices');
 
@@ -89,10 +97,10 @@ class AndroidDevice extends Device {
           stdoutEncoding: latin1,
           stderrEncoding: latin1,
         );
-        if (result.exitCode == 0) {
+        if (result.exitCode == 0 || allowHeapCorruptionOnWindows(result.exitCode)) {
           _properties = parseAdbDeviceProperties(result.stdout);
         } else {
-          printError('Error retrieving device properties for $name:');
+          printError('Error ${result.exitCode} retrieving device properties for $name:');
           printError(result.stderr);
         }
       } on ProcessException catch (error) {
@@ -159,6 +167,28 @@ class AndroidDevice extends Device {
     return <String>[getAdbPath(androidSdk), '-s', id]..addAll(args);
   }
 
+  String runAdbCheckedSync(
+      List<String> params, {
+        String workingDirectory,
+        bool allowReentrantFlutter = false,
+        Map<String, String> environment}) {
+    return runCheckedSync(adbCommandForDevice(params), workingDirectory: workingDirectory,
+        allowReentrantFlutter: allowReentrantFlutter,
+        environment: environment,
+        whiteListFailures: allowHeapCorruptionOnWindows
+    );
+  }
+
+  Future<RunResult> runAdbCheckedAsync(
+      List<String> params, {
+        String workingDirectory,
+        bool allowReentrantFlutter = false,
+      }) async {
+    return runCheckedAsync(adbCommandForDevice(params), workingDirectory: workingDirectory,
+        allowReentrantFlutter: allowReentrantFlutter,
+        whiteListFailures: allowHeapCorruptionOnWindows);
+  }
+
   bool _isValidAdbVersion(String adbVersion) {
     // Sample output: 'Android Debug Bridge version 1.0.31'
     final Match versionFields = RegExp(r'(\d+)\.(\d+)\.(\d+)').firstMatch(adbVersion);
@@ -252,7 +282,7 @@ class AndroidDevice extends Device {
   Future<bool> isAppInstalled(ApplicationPackage app) async {
     // This call takes 400ms - 600ms.
     try {
-      final RunResult listOut = await runCheckedAsync(adbCommandForDevice(<String>['shell', 'pm', 'list', 'packages', app.id]));
+      final RunResult listOut = await runAdbCheckedAsync(<String>['shell', 'pm', 'list', 'packages', app.id]);
       return LineSplitter.split(listOut.stdout).contains('package:${app.id}');
     } catch (error) {
       printTrace('$error');
@@ -294,9 +324,9 @@ class AndroidDevice extends Device {
       return false;
     }
 
-    await runCheckedAsync(adbCommandForDevice(<String>[
+    await runAdbCheckedAsync(<String>[
       'shell', 'echo', '-n', _getSourceSha1(app), '>', _getDeviceSha1Path(app),
-    ]));
+    ]);
     return true;
   }
 
@@ -413,13 +443,13 @@ class AndroidDevice extends Device {
 
     List<String> cmd;
 
-    cmd = adbCommandForDevice(<String>[
+    cmd = <String>[
       'shell', 'am', 'start',
       '-a', 'android.intent.action.RUN',
       '-f', '0x20000000', // FLAG_ACTIVITY_SINGLE_TOP
       '--ez', 'enable-background-compilation', 'true',
       '--ez', 'enable-dart-profiling', 'true',
-    ]);
+    ];
 
     if (traceStartup)
       cmd.addAll(<String>['--ez', 'trace-startup', 'true']);
@@ -451,7 +481,7 @@ class AndroidDevice extends Device {
       }
     }
     cmd.add(apk.launchActivity);
-    final String result = (await runCheckedAsync(cmd)).stdout;
+    final String result = (await runAdbCheckedAsync(cmd)).stdout;
     // This invocation returns 0 even when it fails.
     if (result.contains('Error: ')) {
       printError(result.trim(), wrap: false);
@@ -491,7 +521,8 @@ class AndroidDevice extends Device {
   @override
   Future<bool> stopApp(ApplicationPackage app) {
     final List<String> command = adbCommandForDevice(<String>['shell', 'am', 'force-stop', app.id]);
-    return runCommandAndStreamOutput(command).then<bool>((int exitCode) => exitCode == 0);
+    return runCommandAndStreamOutput(command).then<bool>(
+        (int exitCode) => exitCode == 0 || allowHeapCorruptionOnWindows(exitCode));
   }
 
   @override
@@ -514,9 +545,9 @@ class AndroidDevice extends Device {
   /// Return the most recent timestamp in the Android log or null if there is
   /// no available timestamp. The format can be passed to logcat's -T option.
   String get lastLogcatTimestamp {
-    final String output = runCheckedSync(adbCommandForDevice(<String>[
+    final String output = runAdbCheckedSync(<String>[
       'shell', '-x', 'logcat', '-v', 'time', '-t', '1',
-    ]));
+    ]);
 
     final Match timeMatch = _timeRegExp.firstMatch(output);
     return timeMatch?.group(0);
@@ -531,9 +562,9 @@ class AndroidDevice extends Device {
   @override
   Future<void> takeScreenshot(File outputFile) async {
     const String remotePath = '/data/local/tmp/flutter_screenshot.png';
-    await runCheckedAsync(adbCommandForDevice(<String>['shell', 'screencap', '-p', remotePath]));
+    await runAdbCheckedAsync(<String>['shell', 'screencap', '-p', remotePath]);
     await runCheckedAsync(adbCommandForDevice(<String>['pull', remotePath, outputFile.path]));
-    await runCheckedAsync(adbCommandForDevice(<String>['shell', 'rm', remotePath]));
+    await runAdbCheckedAsync(<String>['shell', 'rm', remotePath]);
   }
 
   @override

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

@@ -239,11 +239,14 @@ Future<RunResult> runAsync(
   return runResults;
 }
 
+typedef RunResultChecker = bool Function(int);
+
 Future<RunResult> runCheckedAsync(
   List<String> cmd, {
   String workingDirectory,
   bool allowReentrantFlutter = false,
   Map<String, String> environment,
+  RunResultChecker whiteListFailures,
 }) async {
   final RunResult result = await runAsync(
     cmd,
@@ -252,8 +255,10 @@ Future<RunResult> runCheckedAsync(
     environment: environment,
   );
   if (result.exitCode != 0) {
-    throw ProcessException(cmd[0], cmd.sublist(1),
-      'Process "${cmd[0]}" exited abnormally:\n$result', result.exitCode);
+    if (whiteListFailures == null || !whiteListFailures(result.exitCode)) {
+      throw ProcessException(cmd[0], cmd.sublist(1),
+          'Process "${cmd[0]}" exited abnormally:\n$result', result.exitCode);
+    }
   }
   return result;
 }
@@ -287,6 +292,7 @@ String runCheckedSync(
   bool allowReentrantFlutter = false,
   bool hideStdout = false,
   Map<String, String> environment,
+  RunResultChecker whiteListFailures,
 }) {
   return _runWithLoggingSync(
     cmd,
@@ -296,6 +302,7 @@ String runCheckedSync(
     checked: true,
     noisyErrors: true,
     environment: environment,
+    whiteListFailures: whiteListFailures
   );
 }
 
@@ -330,6 +337,7 @@ String _runWithLoggingSync(
   bool allowReentrantFlutter = false,
   bool hideStdout = false,
   Map<String, String> environment,
+  RunResultChecker whiteListFailures,
 }) {
   _traceCommand(cmd, workingDirectory: workingDirectory);
   final ProcessResult results = processManager.runSync(
@@ -340,14 +348,19 @@ String _runWithLoggingSync(
 
   printTrace('Exit code ${results.exitCode} from: ${cmd.join(' ')}');
 
+  bool failedExitCode = results.exitCode != 0;
+  if (whiteListFailures != null && failedExitCode) {
+    failedExitCode = !whiteListFailures(results.exitCode);
+  }
+
   if (results.stdout.isNotEmpty && !hideStdout) {
-    if (results.exitCode != 0 && noisyErrors)
+    if (failedExitCode && noisyErrors)
       printStatus(results.stdout.trim());
     else
       printTrace(results.stdout.trim());
   }
 
-  if (results.exitCode != 0) {
+  if (failedExitCode) {
     if (results.stderr.isNotEmpty) {
       if (noisyErrors)
         printError(results.stderr.trim());

packages/flutter_tools/test/android/android_device_test.dart

@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/android/android_sdk.dart';
 import 'package:flutter_tools/src/base/config.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/project.dart';
 import 'package:mockito/mockito.dart';
@@ -106,6 +107,84 @@ Use the 'android' tool to install them:
     });
   });
 
+  group('adb.exe exiting with heap corruption on windows', () {
+    final ProcessManager mockProcessManager = MockProcessManager();
+    String hardware;
+    String buildCharacteristics;
+
+    setUp(() {
+      hardware = 'goldfish';
+      buildCharacteristics = 'unused';
+      exitCode = -1;
+      when(mockProcessManager.run(argThat(contains('getprop')),
+          stderrEncoding: anyNamed('stderrEncoding'),
+          stdoutEncoding: anyNamed('stdoutEncoding'))).thenAnswer((_) {
+        final StringBuffer buf = StringBuffer()
+          ..writeln('[ro.hardware]: [$hardware]')..writeln(
+              '[ro.build.characteristics]: [$buildCharacteristics]');
+        final ProcessResult result = ProcessResult(1, exitCode, buf.toString(), '');
+        return Future<ProcessResult>.value(result);
+      });
+    });
+
+    testUsingContext('nonHeapCorruptionErrorOnWindows', () async {
+      exitCode = -1073740941;
+      final AndroidDevice device = AndroidDevice('test');
+      expect(await device.isLocalEmulator, false);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+      Platform: () => FakePlatform(
+        operatingSystem: 'windows',
+        environment: <String, String>{
+          'ANDROID_HOME': '/',
+        },
+      ),
+    });
+
+    testUsingContext('heapCorruptionOnWindows', () async {
+      exitCode = -1073740940;
+      final AndroidDevice device = AndroidDevice('test');
+      expect(await device.isLocalEmulator, true);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+      Platform: () => FakePlatform(
+        operatingSystem: 'windows',
+        environment: <String, String>{
+          'ANDROID_HOME': '/',
+        },
+      ),
+    });
+
+    testUsingContext('heapCorruptionExitCodeOnLinux', () async {
+      exitCode = -1073740940;
+      final AndroidDevice device = AndroidDevice('test');
+      expect(await device.isLocalEmulator, false);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+      Platform: () => FakePlatform(
+        operatingSystem: 'linux',
+        environment: <String, String>{
+          'ANDROID_HOME': '/',
+        },
+      ),
+    });
+
+    testUsingContext('noErrorOnLinux', () async {
+      exitCode = 0;
+      final AndroidDevice device = AndroidDevice('test');
+      expect(await device.isLocalEmulator, true);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+      Platform: () => FakePlatform(
+        operatingSystem: 'linux',
+        environment: <String, String>{
+          'ANDROID_HOME': '/',
+        },
+      ),
+    });
+  });
+
+
   group('isLocalEmulator', () {
     final ProcessManager mockProcessManager = MockProcessManager();
     String hardware;