Lock flutter tool while updating artifacts (#4476)

This prevents multiple simultaneous runs of the analyzer from stomping
over each other (e.g. multiple runs of 'update-packages'). Certain
long-lived commands (like analyze, run, logs) are exempted once they've
done enough work to be safe from most stomping action.

This still doesn't make us entirely safe from craziness, e.g. if you're
half way through an 'update-packages' run and you call 'git pull', who
knows what state you'll end up in. But there's only so much one can do.

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

bin/cache/.gitignore

@@ -3,3 +3,4 @@
 artifacts/
 dart-sdk/
 pkg/
+lockfile

bin/flutter

@@ -30,16 +30,22 @@ DART_SDK_PATH="$FLUTTER_ROOT/bin/cache/dart-sdk"
 
 DART="$DART_SDK_PATH/bin/dart"
 
-REVISION=`(cd "$FLUTTER_ROOT"; git rev-parse HEAD)`
-if [ ! -f "$SNAPSHOT_PATH" ] || [ ! -f "$STAMP_PATH" ] || [ `cat "$STAMP_PATH"` != "$REVISION" ] || [ "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]; then
-  "$FLUTTER_ROOT/bin/cache/update_dart_sdk.sh"
+(
+  if hash flock 2>/dev/null; then
+    flock 3 # ensures that we don't simultaneously update Dart in multiple parallel instances
+    # some platforms (e.g. Mac) don't have flock or any reliable alternative
+  fi
+  REVISION=`(cd "$FLUTTER_ROOT"; git rev-parse HEAD)`
+  if [ ! -f "$SNAPSHOT_PATH" ] || [ ! -f "$STAMP_PATH" ] || [ `cat "$STAMP_PATH"` != "$REVISION" ] || [ "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]; then
+    "$FLUTTER_ROOT/bin/cache/update_dart_sdk.sh"
 
-  echo Building flutter tool...
-  FLUTTER_DIR="$FLUTTER_ROOT/packages/flutter"
-  (cd "$FLUTTER_TOOLS_DIR"; "../../bin/cache/dart-sdk/bin/pub" get --verbosity=warning)
-  "$DART" --snapshot="$SNAPSHOT_PATH" --packages="$FLUTTER_TOOLS_DIR/.packages" "$SCRIPT_PATH"
-  echo $REVISION > "$STAMP_PATH"
-fi
+    echo Building flutter tool...
+    FLUTTER_DIR="$FLUTTER_ROOT/packages/flutter"
+    (cd "$FLUTTER_TOOLS_DIR"; "../../bin/cache/dart-sdk/bin/pub" get --verbosity=warning)
+    "$DART" --snapshot="$SNAPSHOT_PATH" --packages="$FLUTTER_TOOLS_DIR/.packages" "$SCRIPT_PATH"
+    echo $REVISION > "$STAMP_PATH"
+  fi
+) 3< $PROG_NAME
 
 if [ $FLUTTER_DEV ]; then
   "$DART" --packages="$FLUTTER_TOOLS_DIR/.packages" -c "$SCRIPT_PATH" "$@"

dev/automated_tests/flutter_test/test_async_utils_guarded_expectation.txt

@@ -2,10 +2,10 @@
 ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
 The following assertion was thrown running a test:
 Guarded function conflict\. You must use "await" with all Future-returning test APIs\.
-The guarded "guardedHelper" function was called from .*dev/manual_tests/test_data/test_async_utils_guarded_test\.dart on line [0-9]+\.
-Then, the "expect" function was called from .*dev/manual_tests/test_data/test_async_utils_guarded_test\.dart on line [0-9]+\.
+The guarded "guardedHelper" function was called from .*dev/automated_tests/flutter_test/test_async_utils_guarded_test\.dart on line [0-9]+\.
+Then, the "expect" function was called from .*dev/automated_tests/flutter_test/test_async_utils_guarded_test\.dart on line [0-9]+\.
 The first function \(guardedHelper\) had not yet finished executing at the time that the second function \(expect\) was called\. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called\. Typically, this is achieved by putting an "await" statement in front of the call to the first\.
-If you are confident that all test APIs are being called using "await", and this expect\(\) call is not being invoked at the top level but is itself being called from some sort of callback registered before the guardedHelper method was called, then consider using expectSync\(\) instead\.
+If you are confident that all test APIs are being called using "await", and this expect\(\) call is not being called at the top level but is itself being called from some sort of callback registered before the guardedHelper method was called, then consider using expectSync\(\) instead\.
 
 When the first function \(guardedHelper\) was called, this was the stack:
 <<skip until matching line>>
@@ -14,6 +14,8 @@ When the first function \(guardedHelper\) was called, this was the stack:
 When the exception was thrown, this was the stack:
 <<skip until matching line>>
 \(elided .+\)
+
+
 ════════════════════════════════════════════════════════════════════════════════════════════════════
 .*(this line has more of the test framework's output)?
   Test failed\. See exception logs above\.

dev/automated_tests/flutter_test/test_async_utils_unguarded_expectation.txt

@@ -1,9 +1,9 @@
-.*..:.. \+0: - TestAsyncUtils - handling unguarded async helper functions *
+.*(this line contains the test framework's output with the clock and so forth)?
 ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
 The following assertion was thrown running a test:
 Guarded function conflict\. You must use "await" with all Future-returning test APIs\.
-The guarded method "pump" from class WidgetTester was called from .*dev/flutter/dev/manual_tests/test_data/test_async_utils_unguarded_test.dart on line [0-9]+\.
-Then, it was called again from .*dev/manual_tests/test_data/test_async_utils_unguarded_test.dart on line [0-9]+\.
+The guarded method "pump" from class WidgetTester was called from .*dev/automated_tests/flutter_test/test_async_utils_unguarded_test.dart on line [0-9]+\.
+Then, it was called again from .*dev/automated_tests/flutter_test/test_async_utils_unguarded_test.dart on line [0-9]+\.
 The first method had not yet finished executing at the time that the second method was called\. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called\. Typically, this is achieved by putting an "await" statement in front of the call to the first\.
 
 When the first method was called, this was the stack:
@@ -13,6 +13,8 @@ When the first method was called, this was the stack:
 When the exception was thrown, this was the stack:
 <<skip until matching line>>
 (elided [0-9]+ frames from .+)
+
+
 ════════════════════════════════════════════════════════════════════════════════════════════════════
 .*..:.. \+0 -1: - TestAsyncUtils - handling unguarded async helper functions *
   Test failed. See exception logs above.

packages/flutter_tools/lib/src/cache.dart

@@ -24,6 +24,54 @@ class Cache {
   // Initialized by FlutterCommandRunner on startup.
   static String flutterRoot;
 
+  static RandomAccessFile _lock;
+  static bool _lockEnabled = true;
+
+  /// Turn off the [lock]/[releaseLockEarly] mechanism.
+  ///
+  /// This is used by the tests since they run simultaneously and all in one
+  /// process and so it would be a mess if they had to use the lock.
+  static void disableLocking() {
+    _lockEnabled = false;
+  }
+
+  /// Lock the cache directory.
+  ///
+  /// This happens automatically on startup (see [FlutterCommandRunner.runCommand]).
+  ///
+  /// Normally the lock will be held until the process exits (this uses normal
+  /// POSIX flock semantics). Long-lived commands should release the lock by
+  /// calling [Cache.releaseLockEarly] once they are no longer touching the cache.
+  static Future<Null> lock() async {
+    if (!_lockEnabled)
+      return null;
+    assert(_lock == null);
+    _lock = new File(path.join(flutterRoot, 'bin', 'cache', 'lockfile')).openSync(mode: FileMode.WRITE);
+    bool locked = false;
+    bool printed = false;
+    while (!locked) {
+      try {
+        await _lock.lock();
+        locked = true;
+      } on FileSystemException {
+        if (!printed) {
+          printStatus('Waiting to be able to obtain lock of Flutter cache directory...');
+          printed = true;
+        }
+        await new Future/*<Null>*/.delayed(const Duration(milliseconds: 50));
+      }
+    }
+  }
+
+  /// Releases the lock. This is not necessary unless the process is long-lived.
+  static void releaseLockEarly() {
+    if (!_lockEnabled)
+      return;
+    assert(_lock != null);
+    _lock.closeSync();
+    _lock = null;
+  }
+
   static String _engineRevision;
 
   static String get engineRevision {

packages/flutter_tools/lib/src/commands/analyze.dart

@@ -190,6 +190,8 @@ class AnalyzeCommand extends FlutterCommand {
       packages['sky_services'] = path.join(tools.engineBuildPath, 'gen/dart-pkg/sky_services/lib');
     }
 
+    Cache.releaseLockEarly();
+
     if (argResults['preamble']) {
       if (dartFiles.length == 1) {
         logger.printStatus('Analyzing ${path.relative(dartFiles.first.path)}...');
@@ -316,6 +318,8 @@ class AnalyzeCommand extends FlutterCommand {
     server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing));
     server.onErrors.listen(_handleAnalysisErrors);
 
+    Cache.releaseLockEarly();
+
     await server.start();
     final int exitCode = await server.onExit;
 

packages/flutter_tools/lib/src/commands/daemon.dart

@@ -10,6 +10,7 @@ import '../android/android_device.dart';
 import '../base/context.dart';
 import '../base/logger.dart';
 import '../build_info.dart';
+import '../cache.dart';
 import '../device.dart';
 import '../globals.dart';
 import '../ios/devices.dart';
@@ -48,6 +49,8 @@ class DaemonCommand extends FlutterCommand {
     NotifyingLogger notifyingLogger = new NotifyingLogger();
     appContext[Logger] = notifyingLogger;
 
+    Cache.releaseLockEarly();
+
     return appContext.runInZone(() {
       Stream<Map<String, dynamic>> commandStream = stdin
         .transform(UTF8.decoder)

packages/flutter_tools/lib/src/commands/drive.dart

@@ -16,6 +16,7 @@ import '../base/logger.dart';
 import '../base/os.dart';
 import '../base/process.dart';
 import '../build_info.dart';
+import '../cache.dart';
 import '../dart/sdk.dart';
 import '../device.dart';
 import '../globals.dart';
@@ -135,6 +136,8 @@ class DriveCommand extends RunCommandBase {
       status.stop(showElapsedTime: true);
     }
 
+    Cache.releaseLockEarly();
+
     try {
       return await testRunner(<String>[testFile])
         .catchError((dynamic error, dynamic stackTrace) {

packages/flutter_tools/lib/src/commands/install.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 
 import '../application_package.dart';
+import '../cache.dart';
 import '../device.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
@@ -24,6 +25,8 @@ class InstallCommand extends FlutterCommand {
     Device device = deviceForCommand;
     ApplicationPackage package = applicationPackages.getPackageForPlatform(device.platform);
 
+    Cache.releaseLockEarly();
+
     printStatus('Installing $package to $device...');
 
     return installApp(device, package) ? 0 : 2;

packages/flutter_tools/lib/src/commands/listen.dart

@@ -7,6 +7,7 @@ import 'dart:io';
 
 import '../base/os.dart';
 import '../base/process.dart';
+import '../cache.dart';
 import '../device.dart';
 import '../globals.dart';
 import 'run.dart';
@@ -44,6 +45,8 @@ class ListenCommand extends RunCommandBase {
     if (watchCommand == null)
       return 1;
 
+    Cache.releaseLockEarly();
+
     printStatus('Listening for changes in '
       '${directories.map((String name) => "'$name${Platform.pathSeparator}'").join(', ')}'
       '.');

packages/flutter_tools/lib/src/commands/logs.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:io';
 
+import '../cache.dart';
 import '../device.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
@@ -39,6 +40,8 @@ class LogsCommand extends FlutterCommand {
 
     DeviceLogReader logReader = device.logReader;
 
+    Cache.releaseLockEarly();
+
     printStatus('Showing $logReader logs:');
 
     Completer<int> exitCompleter = new Completer<int>();

packages/flutter_tools/lib/src/commands/refresh.dart

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 
 import '../android/android_device.dart';
 import '../application_package.dart';
+import '../cache.dart';
 import '../flx.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
@@ -46,6 +47,8 @@ class RefreshCommand extends FlutterCommand {
         return result;
       }
 
+      Cache.releaseLockEarly();
+
       AndroidDevice device = deviceForCommand;
 
       String activity = argResults['activity'];

packages/flutter_tools/lib/src/commands/run.dart

@@ -9,6 +9,7 @@ import '../application_package.dart';
 import '../base/common.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
+import '../cache.dart';
 import '../device.dart';
 import '../globals.dart';
 import '../observatory.dart';
@@ -109,6 +110,8 @@ class RunCommand extends RunCommandBase {
       );
     }
 
+    Cache.releaseLockEarly();
+
     if (argResults['resident']) {
       RunAndStayResident runner = new RunAndStayResident(
         deviceForCommand,

packages/flutter_tools/lib/src/commands/run_mojo.dart

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 
 import '../base/process.dart';
 import '../build_info.dart';
+import '../cache.dart';
 import '../flx.dart' as flx;
 import '../globals.dart';
 import '../run.dart';
@@ -151,6 +152,8 @@ class RunMojoCommand extends FlutterCommand {
         return result;
     }
 
+    Cache.releaseLockEarly();
+
     return await runCommandAndStreamOutput(await _getShellConfig(targetApp));
   }
 }

packages/flutter_tools/lib/src/commands/test.dart

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 import 'package:test/src/executable.dart' as executable; // ignore: implementation_imports
 
 import '../base/logger.dart';
+import '../cache.dart';
 import '../dart/package_map.dart';
 import '../globals.dart';
 import '../runner/flutter_command.dart';
@@ -101,6 +102,9 @@ class TestCommand extends FlutterCommand {
         printError('Cannot find Flutter shell at ${loader.shellPath}');
       return 1;
     }
+
+    Cache.releaseLockEarly();
+
     return await _runTests(testArgs, testDir);
   }
 }

packages/flutter_tools/lib/src/commands/trace.dart

@@ -8,6 +8,7 @@ import 'dart:io';
 
 import '../base/common.dart';
 import '../base/utils.dart';
+import '../cache.dart';
 import '../globals.dart';
 import '../observatory.dart';
 import '../runner/flutter_command.dart';
@@ -57,6 +58,8 @@ class TraceCommand extends FlutterCommand {
       return 1;
     }
 
+    Cache.releaseLockEarly();
+
     if ((!argResults['start'] && !argResults['stop']) ||
         (argResults['start'] && argResults['stop'])) {
       // Setting neither flags or both flags means do both commands and wait

packages/flutter_tools/lib/src/runner/flutter_command_runner.dart

@@ -128,7 +128,7 @@ class FlutterCommandRunner extends CommandRunner {
   }
 
   @override
-  Future<int> runCommand(ArgResults globalResults) {
+  Future<int> runCommand(ArgResults globalResults) async {
     // Check for verbose.
     if (globalResults['verbose'])
       context[Logger] = new VerboseLogger();
@@ -142,11 +142,13 @@ class FlutterCommandRunner extends CommandRunner {
     // enginePath's initialiser uses it).
     Cache.flutterRoot = path.normalize(path.absolute(globalResults['flutter-root']));
 
+    await Cache.lock();
+
     if (globalResults['suppress-analytics'])
       flutterUsage.suppressAnalytics = true;
 
     if (!_checkFlutterCopy())
-      return new Future<int>.value(1);
+      return 1;
 
     if (globalResults.wasParsed('packages'))
       PackageMap.globalPackagesPath = path.normalize(path.absolute(globalResults['packages']));
@@ -173,10 +175,10 @@ class FlutterCommandRunner extends CommandRunner {
     if (globalResults['version']) {
       flutterUsage.sendCommand('version');
       printStatus(FlutterVersion.getVersion(Cache.flutterRoot).toString());
-      return new Future<int>.value(0);
+      return 0;
     }
 
-    return super.runCommand(globalResults);
+    return await super.runCommand(globalResults);
   }
 
   String _tryEnginePath(String enginePath) {

packages/flutter_tools/test/all.dart

@@ -7,6 +7,8 @@
 // doesn't support running without symlinks. We can delete these files once that
 // fix lands.
 
+import 'package:flutter_tools/src/cache.dart';
+
 import 'adb_test.dart' as adb_test;
 import 'analytics_test.dart' as analytics_test;
 import 'analyze_duplicate_names_test.dart' as analyze_duplicate_names_test;
@@ -18,6 +20,7 @@ import 'context_test.dart' as context_test;
 import 'create_test.dart' as create_test;
 import 'daemon_test.dart' as daemon_test;
 import 'device_test.dart' as device_test;
+// import 'devices_test.dart' as devices_test;
 import 'drive_test.dart' as drive_test;
 import 'install_test.dart' as install_test;
 import 'listen_test.dart' as listen_test;
@@ -26,11 +29,13 @@ import 'os_utils_test.dart' as os_utils_test;
 import 'protocol_discovery_test.dart' as protocol_discovery_test;
 import 'run_test.dart' as run_test;
 import 'stop_test.dart' as stop_test;
+import 'test_test.dart' as test_test;
 import 'toolchain_test.dart' as toolchain_test;
 import 'trace_test.dart' as trace_test;
 import 'upgrade_test.dart' as upgrade_test;
 
 void main() {
+  Cache.disableLocking();
   adb_test.main();
   analytics_test.main();
   analyze_duplicate_names_test.main();
@@ -42,6 +47,7 @@ void main() {
   create_test.main();
   daemon_test.main();
   device_test.main();
+  // devices_test.main(); // https://github.com/flutter/flutter/issues/4480
   drive_test.main();
   install_test.main();
   listen_test.main();
@@ -50,6 +56,7 @@ void main() {
   protocol_discovery_test.main();
   run_test.main();
   stop_test.main();
+  test_test.main();
   toolchain_test.main();
   trace_test.main();
   upgrade_test.main();

packages/flutter_tools/test/test_test.dart

@@ -18,16 +18,16 @@ void main() {
   group('test', () {
     testUsingContext('TestAsyncUtils guarded function test', () async {
       Cache.flutterRoot = '../..';
-      return _testFile('test_async_utils_guarded');
+      return _testFile('test_async_utils_guarded', 1);
     });
     testUsingContext('TestAsyncUtils unguarded function test', () async {
       Cache.flutterRoot = '../..';
-      return _testFile('test_async_utils_unguarded');
+      return _testFile('test_async_utils_unguarded', 1);
     });
   }, timeout: new Timeout(const Duration(seconds: 5)));
 }
 
-Future<Null> _testFile(String testName) async {
+Future<Null> _testFile(String testName, int wantedExitCode) async {
   final String manualTestsDirectory = path.join('..', '..', 'dev', 'automated_tests');
   final String fullTestName = path.join(manualTestsDirectory, 'flutter_test', '${testName}_test.dart');
   final File testFile = new File(fullTestName);
@@ -40,11 +40,12 @@ Future<Null> _testFile(String testName) async {
     <String>[
       path.absolute(path.join('bin', 'flutter_tools.dart')),
       'test',
+      '--no-color',
       fullTestName
     ],
     workingDirectory: manualTestsDirectory
   );
-  expect(exec.exitCode, 0);
+  expect(exec.exitCode, wantedExitCode);
   final List<String> output = exec.stdout.split('\n');
   final List<String> expectations = new File(fullTestExpectation).readAsLinesSync();
   bool allowSkip = false;