Release cache lock for commands after required artifacts are downloaded (#59012)

packages/flutter_tools/lib/src/cache.dart

@@ -166,7 +166,7 @@ class Cache {
   static RandomAccessFile _lock;
   static bool _lockEnabled = true;
 
-  /// Turn off the [lock]/[releaseLockEarly] mechanism.
+  /// Turn off the [lock]/[releaseLock] 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.
@@ -175,7 +175,7 @@ class Cache {
     _lockEnabled = false;
   }
 
-  /// Turn on the [lock]/[releaseLockEarly] mechanism.
+  /// Turn on the [lock]/[releaseLock] mechanism.
   ///
   /// This is used by the tests.
   @visibleForTesting
@@ -183,13 +183,20 @@ class Cache {
     _lockEnabled = true;
   }
 
+  /// Check if lock acquired, skipping FLUTTER_ALREADY_LOCKED reentrant checks.
+  ///
+  /// This is used by the tests.
+  @visibleForTesting
+  static bool isLocked() {
+    return _lock != null;
+  }
+
   /// Lock the cache directory.
   ///
-  /// This happens automatically on startup (see [FlutterCommandRunner.runCommand]).
+  /// This happens while required artifacts are updated
+  /// (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.
+  /// This uses normal POSIX flock semantics.
   static Future<void> lock() async {
     if (!_lockEnabled) {
       return;
@@ -222,8 +229,11 @@ class Cache {
     }
   }
 
-  /// Releases the lock. This is not necessary unless the process is long-lived.
-  static void releaseLockEarly() {
+  /// Releases the lock.
+  ///
+  /// This happens automatically on startup (see [FlutterCommand.verifyThenRunCommand])
+  /// after the command's required artifacts are updated.
+  static void releaseLock() {
     if (!_lockEnabled || _lock == null) {
       return;
     }

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

@@ -15,7 +15,6 @@ import '../base/logger.dart';
 import '../base/platform.dart';
 import '../base/terminal.dart';
 import '../base/utils.dart';
-import '../cache.dart';
 import '../dart/analysis.dart';
 import '../dart/sdk.dart' as sdk;
 import 'analyze_base.dart';
@@ -81,8 +80,6 @@ class AnalyzeContinuously extends AnalyzeBase {
     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/analyze_once.dart

@@ -14,7 +14,6 @@ import '../base/logger.dart';
 import '../base/platform.dart';
 import '../base/terminal.dart';
 import '../base/utils.dart';
-import '../cache.dart';
 import '../dart/analysis.dart';
 import '../dart/sdk.dart' as sdk;
 import 'analyze.dart';
@@ -127,8 +126,6 @@ class AnalyzeOnce extends AnalyzeBase {
         }
       }));
 
-      Cache.releaseLockEarly();
-
       // collect results
       timer = Stopwatch()..start();
       final String message = directories.length > 1

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

@@ -12,7 +12,6 @@ import '../base/common.dart';
 import '../base/context.dart';
 import '../base/file_system.dart';
 import '../base/io.dart';
-import '../cache.dart';
 import '../commands/daemon.dart';
 import '../compile.dart';
 import '../device.dart';
@@ -173,8 +172,6 @@ class AttachCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
-
     await _validateArguments();
 
     writePidFile(stringArg('pid-file'));

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

@@ -56,7 +56,6 @@ class BuildFuchsiaCommand extends BuildSubCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     final BuildInfo buildInfo = getBuildInfo();
     final FlutterProject flutterProject = FlutterProject.current();
     if (!globals.platform.isLinux && !globals.platform.isMacOS) {

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

@@ -166,8 +166,6 @@ class BuildIOSFrameworkCommand extends BuildSubCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
-
     final String outputArgument = stringArg('output')
         ?? globals.fs.path.join(globals.fs.currentDirectory.path, 'build', 'ios', 'framework');
 

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

@@ -47,7 +47,6 @@ class BuildLinuxCommand extends BuildSubCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     final BuildInfo buildInfo = getBuildInfo();
     final FlutterProject flutterProject = FlutterProject.current();
     if (!featureFlags.isLinuxEnabled) {

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

@@ -48,7 +48,6 @@ class BuildMacosCommand extends BuildSubCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     final BuildInfo buildInfo = getBuildInfo();
     final FlutterProject flutterProject = FlutterProject.current();
     if (!featureFlags.isMacOSEnabled) {

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

@@ -52,7 +52,6 @@ class BuildWindowsCommand extends BuildSubCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     final FlutterProject flutterProject = FlutterProject.current();
     final BuildInfo buildInfo = getBuildInfo();
     if (!featureFlags.isWindowsEnabled) {

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

@@ -262,8 +262,6 @@ class CreateCommand extends FlutterCommand {
   Future<FlutterCommandResult> runCommand() async {
     if (argResults['list-samples'] != null) {
       // _writeSamplesJson can potentially be long-lived.
-      Cache.releaseLockEarly();
-
       await _writeSamplesJson(stringArg('list-samples'));
       return FlutterCommandResult.success();
     }

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

@@ -16,7 +16,6 @@ import '../base/logger.dart';
 import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
-import '../cache.dart';
 import '../convert.dart';
 import '../device.dart';
 import '../emulator.dart';
@@ -52,7 +51,6 @@ class DaemonCommand extends FlutterCommand {
   Future<FlutterCommandResult> runCommand() async {
     globals.printStatus('Starting device daemon...');
     isRunningFromDaemon = true;
-    Cache.releaseLockEarly();
 
     final Daemon daemon = Daemon(
       stdinCommandStream, stdoutCommandResponse,

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

@@ -16,7 +16,6 @@ import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/process.dart';
 import '../build_info.dart';
-import '../cache.dart';
 import '../dart/package_map.dart';
 import '../dart/sdk.dart';
 import '../device.dart';
@@ -250,8 +249,6 @@ class DriveCommand extends RunCommandBase {
       observatoryUri = stringArg('use-existing-app');
     }
 
-    Cache.releaseLockEarly();
-
     final Map<String, String> environment = <String, String>{
       'VM_SERVICE_URL': observatoryUri,
     };

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

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import '../base/file_system.dart';
-import '../cache.dart';
 import '../codegen.dart';
 import '../convert.dart';
 import '../globals.dart' as globals;
@@ -25,7 +24,6 @@ class GenerateCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     final FlutterProject flutterProject = FlutterProject.current();
     globals.printError(<String>[
       '"flutter generate" is deprecated, use "dart pub run build_runner" instead. ',

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

@@ -8,7 +8,6 @@ import '../android/android_device.dart';
 import '../application_package.dart';
 import '../base/common.dart';
 import '../base/io.dart';
-import '../cache.dart';
 import '../device.dart';
 import '../globals.dart' as globals;
 import '../runner/flutter_command.dart';
@@ -54,8 +53,6 @@ class InstallCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts
       null, // Build info isn't relevant for install, will use whatever bundle was built last.
     );
 
-    Cache.releaseLockEarly();
-
     if (uninstallOnly) {
       await _uninstallApp(package);
     } else {

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

@@ -48,8 +48,6 @@ class LogsCommand extends FlutterCommand {
 
     final DeviceLogReader logReader = await device.getLogReader();
 
-    Cache.releaseLockEarly();
-
     globals.printStatus('Showing $logReader logs:');
 
     final Completer<int> exitCompleter = Completer<int>();

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

@@ -6,7 +6,6 @@ import 'dart:async';
 
 import '../base/common.dart';
 import '../base/os.dart';
-import '../cache.dart';
 import '../dart/pub.dart';
 import '../globals.dart' as globals;
 import '../project.dart';
@@ -163,7 +162,6 @@ class PackagesTestCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     await pub.batch(<String>['run', 'test', ...argResults.rest], context: PubContext.runTest, retry: false);
     return FlutterCommandResult.success();
   }
@@ -204,7 +202,6 @@ class PackagesPublishCommand extends FlutterCommand {
       if (boolArg('dry-run')) '--dry-run',
       if (boolArg('force')) '--force',
     ];
-    Cache.releaseLockEarly();
     await pub.interactively(<String>['publish', ...args], stdio: globals.stdio);
     return FlutterCommandResult.success();
   }
@@ -235,7 +232,6 @@ class PackagesForwardCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     await pub.interactively(<String>[_commandName, ...argResults.rest], stdio: globals.stdio);
     return FlutterCommandResult.success();
   }
@@ -263,7 +259,6 @@ class PackagesPassthroughCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
     await pub.interactively(argResults.rest, stdio: globals.stdio);
     return FlutterCommandResult.success();
   }

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

@@ -111,6 +111,11 @@ class PrecacheCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
+    // Re-lock the cache.
+    if (globals.platform.environment['FLUTTER_ALREADY_LOCKED'] != 'true') {
+      await Cache.lock();
+    }
+
     final bool includeAllPlatforms = boolArg('all-platforms');
     if (includeAllPlatforms) {
       globals.cache.includeAllPlatforms = true;

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

@@ -12,7 +12,6 @@ import '../base/file_system.dart';
 import '../base/io.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
-import '../cache.dart';
 import '../device.dart';
 import '../features.dart';
 import '../globals.dart' as globals;
@@ -403,8 +402,6 @@ class RunCommand extends RunCommandBase {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    Cache.releaseLockEarly();
-
     // Enable hot mode by default if `--no-hot` was not passed and we are in
     // debug mode.
     final bool hotMode = shouldUseHotMode();

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

@@ -156,7 +156,6 @@ class TestCommand extends FlutterCommand {
 
   @override
   Future<FlutterCommandResult> runCommand() async {
-    await globals.cache.updateAll(await requiredArtifacts);
     if (!globals.fs.isFileSync('pubspec.yaml')) {
       throwToolExit(
         'Error: No pubspec.yaml file found in the current working directory.\n'
@@ -237,8 +236,6 @@ class TestCommand extends FlutterCommand {
       watcher = collector;
     }
 
-    Cache.releaseLockEarly();
-
     // Run builders once before all tests.
     if (flutterProject.hasBuilders) {
       final CodegenDaemon codegenDaemon = await codeGenerator.daemon(flutterProject);

packages/flutter_tools/lib/src/dart/pub.dart

@@ -328,7 +328,6 @@ class _DefaultPub implements Pub {
     String directory,
     @required io.Stdio stdio,
   }) async {
-    Cache.releaseLockEarly();
     final io.Process process = await _processUtils.start(
       _pubCommand(arguments),
       workingDirectory: directory,

packages/flutter_tools/lib/src/desktop_device.dart

@@ -10,7 +10,6 @@ import 'application_package.dart';
 import 'base/common.dart';
 import 'base/io.dart';
 import 'build_info.dart';
-import 'cache.dart';
 import 'convert.dart';
 import 'device.dart';
 import 'globals.dart' as globals;
@@ -98,7 +97,6 @@ abstract class DesktopDevice extends Device {
     String userIdentifier,
   }) async {
     if (!prebuiltApplication) {
-      Cache.releaseLockEarly();
       await buildForDevice(
         package,
         buildInfo: debuggingOptions?.buildInfo,

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

@@ -731,7 +731,7 @@ abstract class FlutterCommand extends Command<void> {
 
   void _registerSignalHandlers(String commandPath, DateTime startTime) {
     final SignalHandler handler = (io.ProcessSignal s) {
-      Cache.releaseLockEarly();
+      Cache.releaseLock();
       _sendPostUsage(
         commandPath,
         const FlutterCommandResult(ExitStatus.killed),
@@ -806,8 +806,12 @@ abstract class FlutterCommand extends Command<void> {
 
     if (shouldRunPub) {
       await pub.get(context: PubContext.getVerifyContext(name));
+      // All done updating dependencies. Release the cache lock.
+      Cache.releaseLock();
       final FlutterProject project = FlutterProject.current();
       await project.ensureReadyForPlatformSpecificTooling(checkProjects: true);
+    } else {
+      Cache.releaseLock();
     }
 
     setupApplicationPackages();

packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart

@@ -24,7 +24,7 @@ void main() {
   setUp(() {
     cache = MockCache();
     // Release lock between test cases.
-    Cache.releaseLockEarly();
+    Cache.releaseLock();
 
     when(cache.isUpToDate()).thenReturn(false);
     when(cache.updateAll(any)).thenAnswer((Invocation invocation) {
@@ -37,6 +37,37 @@ void main() {
     when(masterFlutterVersion.isMaster).thenReturn(true);
   });
 
+  testUsingContext('precache should acquire lock', () async {
+    final PrecacheCommand command = PrecacheCommand();
+    applyMocksToCommand(command);
+    await createTestCommandRunner(command).run(const <String>['precache']);
+
+    expect(Cache.isLocked(), isTrue);
+    // Do not throw StateError, lock is acquired.
+    Cache.checkLockAcquired();
+  }, overrides: <Type, Generator>{
+    Cache: () => cache,
+  });
+
+  testUsingContext('precache should not re-entrantly acquire lock', () async {
+    final PrecacheCommand command = PrecacheCommand();
+    applyMocksToCommand(command);
+    await createTestCommandRunner(command).run(const <String>['precache']);
+
+    expect(Cache.isLocked(), isFalse);
+    // Do not throw StateError, acquired reentrantly with FLUTTER_ALREADY_LOCKED.
+    Cache.checkLockAcquired();
+  }, overrides: <Type, Generator>{
+    Cache: () => cache,
+    Platform: () => FakePlatform(
+      operatingSystem: 'windows',
+      environment: <String, String>{
+        'FLUTTER_ROOT': 'flutter',
+        'FLUTTER_ALREADY_LOCKED': 'true',
+      },
+    ),
+  });
+
   testUsingContext('precache downloads web artifacts on dev branch when feature is enabled.', () async {
     final PrecacheCommand command = PrecacheCommand();
     applyMocksToCommand(command);

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

@@ -43,7 +43,7 @@ void main() {
       // Restore locking to prevent potential side-effects in
       // tests outside this group (this option is globally shared).
       Cache.enableLocking();
-      Cache.releaseLockEarly();
+      Cache.releaseLock();
     });
 
     test('should throw when locking is not acquired', () {
@@ -60,7 +60,7 @@ void main() {
       when(mockFile.openSync(mode: anyNamed('mode'))).thenReturn(mockRandomAccessFile);
       await Cache.lock();
       Cache.checkLockAcquired();
-      Cache.releaseLockEarly();
+      Cache.releaseLock();
     }, overrides: <Type, Generator>{
       FileSystem: () => mockFileSystem,
       ProcessManager: () => FakeProcessManager.any(),

packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart

@@ -321,7 +321,7 @@ void main() {
         await completer.future;
 
         await Cache.lock();
-        Cache.releaseLockEarly();
+        Cache.releaseLock();
       }, overrides: <Type, Generator>{
         ProcessInfo: () => mockProcessInfo,
         Signals: () => FakeSignals(