[flutter_tool] Use a timeout for xcode showBuildSettings (#39280)

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

@@ -231,8 +231,14 @@ Future<RunResult> runAsync(
   String workingDirectory,
   bool allowReentrantFlutter = false,
   Map<String, String> environment,
+  Duration timeout,
+  int timeoutRetries = 0,
 }) async {
   _traceCommand(cmd, workingDirectory: workingDirectory);
+
+  // When there is no timeout, there's no need to kill a running process, so
+  // we can just use processManager.run().
+  if (timeout == null) {
     final ProcessResult results = await processManager.run(
       cmd,
       workingDirectory: workingDirectory,
@@ -241,6 +247,72 @@ Future<RunResult> runAsync(
     final RunResult runResults = RunResult(results, cmd);
     printTrace(runResults.toString());
     return runResults;
+  }
+
+  // When there is a timeout, we have to kill the running process, so we have
+  // to use processManager.start() through runCommand() above.
+  while (true) {
+    assert(timeoutRetries >= 0);
+    timeoutRetries = timeoutRetries - 1;
+
+    final Process process = await runCommand(
+        cmd,
+        workingDirectory: workingDirectory,
+        allowReentrantFlutter: allowReentrantFlutter,
+        environment: environment,
+    );
+
+    final StringBuffer stdoutBuffer = StringBuffer();
+    final StringBuffer stderrBuffer = StringBuffer();
+    final Future<void> stdoutFuture = process.stdout
+        .transform<String>(const Utf8Decoder(reportErrors: false))
+        .listen(stdoutBuffer.write)
+        .asFuture<void>(null);
+    final Future<void> stderrFuture = process.stderr
+        .transform<String>(const Utf8Decoder(reportErrors: false))
+        .listen(stderrBuffer.write)
+        .asFuture<void>(null);
+
+    int exitCode;
+    exitCode = await process.exitCode.timeout(timeout, onTimeout: () {
+      return null;
+    });
+
+    String stdoutString;
+    String stderrString;
+    try {
+      await Future.wait<void>(<Future<void>>[stdoutFuture, stderrFuture]);
+    } catch (_) {
+      // Ignore errors on the process' stdout and stderr streams. Just capture
+      // whatever we got, and use the exit code
+    }
+    stdoutString = stdoutBuffer.toString();
+    stderrString = stderrBuffer.toString();
+
+    final ProcessResult result = ProcessResult(
+        process.pid, exitCode ?? -1, stdoutString, stderrString);
+    final RunResult runResult = RunResult(result, cmd);
+
+    // If the process did not timeout. We are done.
+    if (exitCode != null) {
+      printTrace(runResult.toString());
+      return runResult;
+    }
+
+    // The process timed out. Kill it.
+    processManager.killPid(process.pid);
+
+    // If we are out of timeoutRetries, throw a ProcessException.
+    if (timeoutRetries < 0) {
+      throw ProcessException(cmd[0], cmd.sublist(1),
+          'Process "${cmd[0]}" timed out: $runResult', exitCode);
+    }
+
+    // Log the timeout with a trace message in verbose mode.
+    printTrace('Process "${cmd[0]}" timed out. $timeoutRetries attempts left: $runResult');
+  }
+
+  // Unreachable.
 }
 
 typedef RunResultChecker = bool Function(int);
@@ -251,12 +323,16 @@ Future<RunResult> runCheckedAsync(
   bool allowReentrantFlutter = false,
   Map<String, String> environment,
   RunResultChecker whiteListFailures,
+  Duration timeout,
+  int timeoutRetries = 0,
 }) async {
   final RunResult result = await runAsync(
     cmd,
     workingDirectory: workingDirectory,
     allowReentrantFlutter: allowReentrantFlutter,
     environment: environment,
+    timeout: timeout,
+    timeoutRetries: timeoutRetries,
   );
   if (result.exitCode != 0) {
     if (whiteListFailures == null || !whiteListFailures(result.exitCode)) {

packages/flutter_tools/lib/src/ios/xcodeproj.dart

@@ -10,6 +10,7 @@ import '../artifacts.dart';
 import '../base/context.dart';
 import '../base/file_system.dart';
 import '../base/io.dart';
+import '../base/logger.dart';
 import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/process.dart';
@@ -249,6 +250,8 @@ class XcodeProjectInterpreter {
     return _minorVersion;
   }
 
+  /// Synchronously retrieve xcode build settings. Prefer using the async
+  /// version below.
   Map<String, String> getBuildSettings(String projectPath, String target) {
     try {
       final String out = runCheckedSync(<String>[
@@ -266,6 +269,41 @@ class XcodeProjectInterpreter {
     }
   }
 
+  /// Asynchronously retrieve xcode build settings. This one is preferred for
+  /// new call-sites.
+  Future<Map<String, String>> getBuildSettingsAsync(
+      String projectPath, String target, {
+    Duration timeout = const Duration(minutes: 1),
+  }) async {
+    final Status status = Status.withSpinner(
+      timeout: timeoutConfiguration.fastOperation,
+    );
+    try {
+      // showBuildSettings is reported to ocassionally timeout. Here, we give it
+      // a lot of wiggle room (locally on Flutter Gallery, this takes ~1s).
+      // When there is a timeout, we retry once.
+      final RunResult result = await runCheckedAsync(<String>[
+          _executable,
+          '-project',
+          fs.path.absolute(projectPath),
+          '-target',
+          target,
+          '-showBuildSettings',
+        ],
+        workingDirectory: projectPath,
+        timeout: timeout,
+        timeoutRetries: 1,
+      );
+      final String out = result.stdout.trim();
+      return parseXcodeBuildSettings(out);
+    } catch(error) {
+      printTrace('Unexpected failure to get the build settings: $error.');
+      return const <String, String>{};
+    } finally {
+      status.stop();
+    }
+  }
+
   void cleanWorkspace(String workspacePath, String scheme) {
     runSync(<String>[
       _executable,

packages/flutter_tools/lib/src/macos/cocoapods.dart

@@ -192,7 +192,7 @@ class CocoaPods {
   /// Ensures the given Xcode-based sub-project of a parent Flutter project
   /// contains a suitable `Podfile` and that its `Flutter/Xxx.xcconfig` files
   /// include pods configuration.
-  void setupPodfile(XcodeBasedProject xcodeProject) {
+  Future<void> setupPodfile(XcodeBasedProject xcodeProject) async {
     if (!xcodeProjectInterpreter.isInstalled) {
       // Don't do anything for iOS when host platform doesn't support it.
       return;
@@ -202,15 +202,18 @@ class CocoaPods {
       return;
     }
     final File podfile = xcodeProject.podfile;
-    if (!podfile.existsSync()) {
+    if (podfile.existsSync()) {
+      addPodsDependencyToFlutterXcconfig(xcodeProject);
+      return;
+    }
     String podfileTemplateName;
     if (xcodeProject is MacOSProject) {
       podfileTemplateName = 'Podfile-macos';
     } else {
-        final bool isSwift = xcodeProjectInterpreter.getBuildSettings(
+      final bool isSwift = (await xcodeProjectInterpreter.getBuildSettingsAsync(
         runnerProject.path,
         'Runner',
-        ).containsKey('SWIFT_VERSION');
+      )).containsKey('SWIFT_VERSION');
       podfileTemplateName = isSwift ? 'Podfile-ios-swift' : 'Podfile-ios-objc';
     }
     final File podfileTemplate = fs.file(fs.path.join(
@@ -222,7 +225,6 @@ class CocoaPods {
       podfileTemplateName,
     ));
     podfileTemplate.copySync(podfile.path);
-    }
     addPodsDependencyToFlutterXcconfig(xcodeProject);
   }
 

packages/flutter_tools/lib/src/plugins.dart

@@ -371,7 +371,7 @@ Future<void> injectPlugins(FlutterProject project, {bool checkProjects = false})
   if (!project.isModule && (!checkProjects || subproject.existsSync())) {
     final CocoaPods cocoaPods = CocoaPods();
     if (plugins.isNotEmpty) {
-      cocoaPods.setupPodfile(subproject);
+      await cocoaPods.setupPodfile(subproject);
     }
     /// The user may have a custom maintained Podfile that they're running `pod install`
     /// on themselves.

packages/flutter_tools/lib/src/project.dart

@@ -22,15 +22,19 @@ import 'ios/xcodeproj.dart' as xcode;
 import 'plugins.dart';
 import 'template.dart';
 
-FlutterProjectFactory get projectFactory => context.get<FlutterProjectFactory>() ?? const FlutterProjectFactory();
+FlutterProjectFactory get projectFactory => context.get<FlutterProjectFactory>() ?? FlutterProjectFactory();
 
 class FlutterProjectFactory {
-  const FlutterProjectFactory();
+  FlutterProjectFactory();
+
+  final Map<String, FlutterProject> _projects =
+      <String, FlutterProject>{};
 
   /// Returns a [FlutterProject] view of the given directory or a ToolExit error,
   /// if `pubspec.yaml` or `example/pubspec.yaml` is invalid.
   FlutterProject fromDirectory(Directory directory) {
     assert(directory != null);
+    return _projects.putIfAbsent(directory.path, /* ifAbsent */ () {
       final FlutterManifest manifest = FlutterProject._readManifest(
         directory.childFile(bundle.defaultManifestPath).path,
       );
@@ -40,6 +44,7 @@ class FlutterProjectFactory {
             .path,
       );
       return FlutterProject(directory, manifest, exampleManifest);
+    });
   }
 }
 
@@ -394,9 +399,14 @@ class IosProject implements XcodeBasedProject {
   Map<String, String> get buildSettings {
     if (!xcode.xcodeProjectInterpreter.isInstalled)
       return null;
-    return xcode.xcodeProjectInterpreter.getBuildSettings(xcodeProject.path, _hostAppBundleName);
+    _buildSettings ??=
+        xcode.xcodeProjectInterpreter.getBuildSettings(xcodeProject.path,
+                                                       _hostAppBundleName);
+    return _buildSettings;
   }
 
+  Map<String, String> _buildSettings;
+
   Future<void> ensureReadyForPlatformSpecificTooling() async {
     _regenerateFromTemplateIfNeeded();
     if (!_flutterLibRoot.existsSync())

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

@@ -12,7 +12,9 @@ import 'package:process/process.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
-import '../../src/mocks.dart' show MockProcess, MockProcessManager;
+import '../../src/mocks.dart' show MockProcess,
+                                   MockProcessManager,
+                                   flakyProcessFactory;
 
 void main() {
   group('process exceptions', () {
@@ -28,6 +30,7 @@ void main() {
       expect(() async => await runCheckedAsync(<String>['false']), throwsA(isInstanceOf<ProcessException>()));
     }, overrides: <Type, Generator>{ProcessManager: () => mockProcessManager});
   });
+
   group('shutdownHooks', () {
     testUsingContext('runInExpectedOrder', () async {
       int i = 1;
@@ -60,6 +63,7 @@ void main() {
       expect(cleanup, 4);
     });
   });
+
   group('output formatting', () {
     MockProcessManager mockProcessManager;
     BufferLogger mockLogger;
@@ -90,6 +94,49 @@ void main() {
       Platform: () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false,
     });
   });
+
+  group('runAsync timeout and retry', () {
+    const Duration delay = Duration(seconds: 2);
+    MockProcessManager flakyProcessManager;
+
+    setUp(() {
+      // MockProcessManager has an implementation of start() that returns the
+      // result of processFactory.
+      flakyProcessManager = MockProcessManager();
+      flakyProcessManager.processFactory = flakyProcessFactory(1, delay: delay);
+    });
+
+    testUsingContext('flaky process fails without retry', () async {
+      final RunResult result = await runAsync(
+        <String>['dummy'],
+        timeout: delay + const Duration(seconds: 1),
+      );
+      expect(result.exitCode, -9);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => flakyProcessManager,
+    });
+
+    testUsingContext('flaky process succeeds with retry', () async {
+      final RunResult result = await runAsync(
+        <String>['dummy'],
+        timeout: delay - const Duration(milliseconds: 500),
+        timeoutRetries: 1,
+      );
+      expect(result.exitCode, 0);
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => flakyProcessManager,
+    });
+
+    testUsingContext('flaky process generates ProcessException on timeout', () async {
+      expect(() async => await runAsync(
+        <String>['dummy'],
+        timeout: delay - const Duration(milliseconds: 500),
+        timeoutRetries: 0,
+      ), throwsA(isInstanceOf<ProcessException>()));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => flakyProcessManager,
+    });
+  });
 }
 
 class PlainMockProcessManager extends Mock implements ProcessManager {}

packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart

@@ -17,19 +17,20 @@ import 'package:process/process.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
+import '../../src/mocks.dart' as mocks;
 import '../../src/pubspec_schema.dart';
 
 const String xcodebuild = '/usr/bin/xcodebuild';
 
 void main() {
   group('xcodebuild versioning', () {
-    MockProcessManager mockProcessManager;
+    mocks.MockProcessManager mockProcessManager;
     XcodeProjectInterpreter xcodeProjectInterpreter;
     FakePlatform macOS;
     FileSystem fs;
 
     setUp(() {
-      mockProcessManager = MockProcessManager();
+      mockProcessManager = mocks.MockProcessManager();
       xcodeProjectInterpreter = XcodeProjectInterpreter();
       macOS = fakePlatform('macos');
       fs = MemoryFileSystem();
@@ -149,6 +150,23 @@ void main() {
           .thenReturn(ProcessResult(0, 1, '', ''));
       expect(xcodeProjectInterpreter.getBuildSettings('', ''), const <String, String>{});
     });
+
+    testUsingContext('build settings flakes', () async {
+      const Duration delay = Duration(seconds: 1);
+      mockProcessManager.processFactory =
+          mocks.flakyProcessFactory(1, delay: delay + const Duration(seconds: 1));
+      expect(await xcodeProjectInterpreter.getBuildSettingsAsync(
+                 '', '', timeout: delay),
+             const <String, String>{});
+      // build settings times out and is killed once, then succeeds.
+      verify(mockProcessManager.killPid(any)).called(1);
+      // The verbose logs should tell us something timed out.
+      expect(testLogger.traceText, contains('timed out'));
+    }, overrides: <Type, Generator>{
+      Platform: () => macOS,
+      FileSystem: () => fs,
+      ProcessManager: () => mockProcessManager,
+    });
   });
   group('Xcode project properties', () {
     test('properties from default project can be parsed', () {

packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart

@@ -164,7 +164,7 @@ void main() {
 
   group('Setup Podfile', () {
     testUsingContext('creates objective-c Podfile when not present', () async {
-      cocoaPodsUnderTest.setupPodfile(projectUnderTest.ios);
+      await cocoaPodsUnderTest.setupPodfile(projectUnderTest.ios);
 
       expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Objective-C iOS podfile template');
     }, overrides: <Type, Generator>{
@@ -173,12 +173,13 @@ void main() {
 
     testUsingContext('creates swift Podfile if swift', () async {
       when(mockXcodeProjectInterpreter.isInstalled).thenReturn(true);
-      when(mockXcodeProjectInterpreter.getBuildSettings(any, any)).thenReturn(<String, String>{
+      when(mockXcodeProjectInterpreter.getBuildSettingsAsync(any, any))
+          .thenAnswer((_) async => <String, String>{
         'SWIFT_VERSION': '4.0',
       });
 
       final FlutterProject project = FlutterProject.fromPath('project');
-      cocoaPodsUnderTest.setupPodfile(project.ios);
+      await cocoaPodsUnderTest.setupPodfile(project.ios);
 
       expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Swift iOS podfile template');
     }, overrides: <Type, Generator>{
@@ -188,7 +189,7 @@ void main() {
 
     testUsingContext('creates macOS Podfile when not present', () async {
       projectUnderTest.macos.xcodeProject.createSync(recursive: true);
-      cocoaPodsUnderTest.setupPodfile(projectUnderTest.macos);
+      await cocoaPodsUnderTest.setupPodfile(projectUnderTest.macos);
 
       expect(projectUnderTest.macos.podfile.readAsStringSync(), 'macOS podfile template');
     }, overrides: <Type, Generator>{
@@ -199,7 +200,7 @@ void main() {
       projectUnderTest.ios.podfile..createSync()..writeAsStringSync('Existing Podfile');
 
       final FlutterProject project = FlutterProject.fromPath('project');
-      cocoaPodsUnderTest.setupPodfile(project.ios);
+      await cocoaPodsUnderTest.setupPodfile(project.ios);
 
       expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Existing Podfile');
     }, overrides: <Type, Generator>{
@@ -210,7 +211,7 @@ void main() {
       when(mockXcodeProjectInterpreter.isInstalled).thenReturn(false);
 
       final FlutterProject project = FlutterProject.fromPath('project');
-      cocoaPodsUnderTest.setupPodfile(project.ios);
+      await cocoaPodsUnderTest.setupPodfile(project.ios);
 
       expect(projectUnderTest.ios.podfile.existsSync(), false);
     }, overrides: <Type, Generator>{
@@ -228,7 +229,7 @@ void main() {
         ..writeAsStringSync('Existing release config');
 
       final FlutterProject project = FlutterProject.fromPath('project');
-      cocoaPodsUnderTest.setupPodfile(project.ios);
+      await cocoaPodsUnderTest.setupPodfile(project.ios);
 
       final String debugContents = projectUnderTest.ios.xcodeConfigFor('Debug').readAsStringSync();
       expect(debugContents, contains(

packages/flutter_tools/test/general.shard/project_test.dart

@@ -244,9 +244,11 @@ void main() {
     group('language', () {
       MockXcodeProjectInterpreter mockXcodeProjectInterpreter;
       MemoryFileSystem fs;
+      FlutterProjectFactory flutterProjectFactory;
       setUp(() {
         fs = MemoryFileSystem();
         mockXcodeProjectInterpreter = MockXcodeProjectInterpreter();
+        flutterProjectFactory = FlutterProjectFactory();
       });
 
       testInMemory('default host app language', () async {
@@ -273,6 +275,7 @@ apply plugin: 'kotlin-android'
       }, overrides: <Type, Generator>{
           FileSystem: () => fs,
           XcodeProjectInterpreter: () => mockXcodeProjectInterpreter,
+          FlutterProjectFactory: () => flutterProjectFactory,
       });
     });
 
@@ -280,10 +283,12 @@ apply plugin: 'kotlin-android'
       MemoryFileSystem fs;
       MockPlistUtils mockPlistUtils;
       MockXcodeProjectInterpreter mockXcodeProjectInterpreter;
+      FlutterProjectFactory flutterProjectFactory;
       setUp(() {
         fs = MemoryFileSystem();
         mockPlistUtils = MockPlistUtils();
         mockXcodeProjectInterpreter = MockXcodeProjectInterpreter();
+        flutterProjectFactory = FlutterProjectFactory();
       });
 
       void testWithMocks(String description, Future<void> testMethod()) {
@@ -291,6 +296,7 @@ apply plugin: 'kotlin-android'
           FileSystem: () => fs,
           PlistParser: () => mockPlistUtils,
           XcodeProjectInterpreter: () => mockXcodeProjectInterpreter,
+          FlutterProjectFactory: () => flutterProjectFactory,
         });
       }
 
@@ -425,9 +431,11 @@ apply plugin: 'kotlin-android'
 
   group('Regression test for invalid pubspec', () {
     Testbed testbed;
+    FlutterProjectFactory flutterProjectFactory;
 
     setUp(() {
       testbed = Testbed();
+      flutterProjectFactory = FlutterProjectFactory();
     });
 
     test('Handles asking for builders from an invalid pubspec', () => testbed.run(() {
@@ -439,6 +447,8 @@ apply plugin: 'kotlin-android'
       final FlutterProject flutterProject = FlutterProject.current();
 
       expect(flutterProject.builders, null);
+    }, overrides: <Type, Generator>{
+      FlutterProjectFactory: () => flutterProjectFactory,
     }));
 
     test('Handles asking for builders from a trivial pubspec', () => testbed.run(() {
@@ -451,6 +461,8 @@ name: foo_bar
       final FlutterProject flutterProject = FlutterProject.current();
 
       expect(flutterProject.builders, null);
+    }, overrides: <Type, Generator>{
+      FlutterProjectFactory: () => flutterProjectFactory,
     }));
   });
 }
@@ -510,12 +522,16 @@ void testInMemory(String description, Future<void> testMethod()) {
       .childDirectory('packages')
       .childDirectory('flutter_tools')
       .childDirectory('schema'), testFileSystem);
+
+  final FlutterProjectFactory flutterProjectFactory = FlutterProjectFactory();
+
   testUsingContext(
     description,
     testMethod,
     overrides: <Type, Generator>{
       FileSystem: () => testFileSystem,
       Cache: () => Cache(),
+      FlutterProjectFactory: () => flutterProjectFactory,
     },
   );
 }

packages/flutter_tools/test/src/context.dart

@@ -335,6 +335,15 @@ class FakeXcodeProjectInterpreter implements XcodeProjectInterpreter {
     return <String, String>{};
   }
 
+  @override
+  Future<Map<String, String>> getBuildSettingsAsync(
+    String projectPath,
+    String target, {
+    Duration timeout = const Duration(minutes: 1),
+  }) async {
+    return <String, String>{};
+  }
+
   @override
   void cleanWorkspace(String workspacePath, String scheme) {
   }

packages/flutter_tools/test/src/mocks.dart

@@ -182,6 +182,26 @@ class MockProcessManager extends Mock implements ProcessManager {
   }
 }
 
+/// A function that generates a process factory that gives processes that fail
+/// a given number of times before succeeding. The returned processes will
+/// fail after a delay if one is supplied.
+ProcessFactory flakyProcessFactory(int flakes, {Duration delay}) {
+  int flakesLeft = flakes;
+  return (List<String> command) {
+    if (flakesLeft == 0) {
+      return MockProcess(exitCode: Future<int>.value(0));
+    }
+    flakesLeft = flakesLeft - 1;
+    Future<int> exitFuture;
+    if (delay == null) {
+      exitFuture = Future<int>.value(-9);
+    } else {
+      exitFuture = Future<int>.delayed(delay, () => Future<int>.value(-9));
+    }
+    return MockProcess(exitCode: exitFuture);
+  };
+}
+
 /// A process that exits successfully with no output and ignores all input.
 class MockProcess extends Mock implements Process {
   MockProcess({