fix checkLockAcquired: support re-entrant locking (#9272)
* fix checkLockAcquired: support re-entrant locking
* add test; address comments
* add comment
diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart
index a414421..5d10d2a 100644
--- a/packages/flutter_tools/lib/src/cache.dart
+++ b/packages/flutter_tools/lib/src/cache.dart
@@ -4,6 +4,8 @@
import 'dart:async';
+import 'package:meta/meta.dart';
+
import 'base/context.dart';
import 'base/file_system.dart';
import 'base/logger.dart';
@@ -33,10 +35,19 @@
///
/// 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.
+ @visibleForTesting
static void disableLocking() {
_lockEnabled = false;
}
+ /// Turn on the [lock]/[releaseLockEarly] mechanism.
+ ///
+ /// This is used by the tests.
+ @visibleForTesting
+ static void enableLocking() {
+ _lockEnabled = true;
+ }
+
/// Lock the cache directory.
///
/// This happens automatically on startup (see [FlutterCommandRunner.runCommand]).
@@ -48,7 +59,7 @@
if (!_lockEnabled)
return null;
assert(_lock == null);
- _lock = fs.file(fs.path.join(flutterRoot, 'bin', 'cache', 'lockfile')).openSync(mode: FileMode.WRITE);
+ _lock = await fs.file(fs.path.join(flutterRoot, 'bin', 'cache', 'lockfile')).open(mode: FileMode.WRITE);
bool locked = false;
bool printed = false;
while (!locked) {
@@ -77,7 +88,7 @@
/// Checks if the current process owns the lock for the cache directory at
/// this very moment; throws a [StateError] if it doesn't.
static void checkLockAcquired() {
- if (_lockEnabled && _lock == null) {
+ if (_lockEnabled && _lock == null && platform.environment['FLUTTER_ALREADY_LOCKED'] != 'true') {
throw new StateError(
'The current process does not own the lock for the cache directory. This is a bug in Flutter CLI tools.',
);
diff --git a/packages/flutter_tools/test/src/cache_test.dart b/packages/flutter_tools/test/src/cache_test.dart
new file mode 100644
index 0000000..5c7a401
--- /dev/null
+++ b/packages/flutter_tools/test/src/cache_test.dart
@@ -0,0 +1,67 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:file/file.dart';
+import 'package:file/memory.dart';
+import 'package:mockito/mockito.dart';
+import 'package:test/test.dart';
+import 'package:platform/platform.dart';
+
+import 'package:flutter_tools/src/cache.dart';
+
+import 'context.dart';
+
+void main() {
+ group('$Cache.checkLockAcquired', () {
+ setUp(() {
+ Cache.enableLocking();
+ });
+
+ tearDown(() {
+ // Restore locking to prevent potential side-effects in
+ // tests outside this group (this option is globally shared).
+ Cache.enableLocking();
+ });
+
+ test('should throw when locking is not acquired', () {
+ expect(() => Cache.checkLockAcquired(), throwsStateError);
+ });
+
+ test('should not throw when locking is disabled', () {
+ Cache.disableLocking();
+ Cache.checkLockAcquired();
+ });
+
+ testUsingContext('should not throw when lock is acquired', () async {
+ await Cache.lock();
+ Cache.checkLockAcquired();
+ }, overrides: <Type, Generator>{
+ FileSystem: () => new MockFileSystem(),
+ });
+
+ testUsingContext('should not throw when FLUTTER_ALREADY_LOCKED is set', () async {
+ Cache.checkLockAcquired();
+ }, overrides: <Type, Generator>{
+ Platform: () => new FakePlatform()..environment = <String, String>{'FLUTTER_ALREADY_LOCKED': 'true'},
+ });
+ });
+}
+
+class MockFileSystem extends MemoryFileSystem {
+ @override
+ File file(dynamic path) {
+ return new MockFile();
+ }
+}
+
+class MockFile extends Mock implements File {
+ @override
+ Future<RandomAccessFile> open({FileMode mode: FileMode.READ}) async {
+ return new MockRandomAccessFile();
+ }
+}
+
+class MockRandomAccessFile extends Mock implements RandomAccessFile {}
