Avoid race condition for hosted source on package extraction (#3277)
diff --git a/lib/src/command/cache_repair.dart b/lib/src/command/cache_repair.dart
index 41bae2b..74968be 100644
--- a/lib/src/command/cache_repair.dart
+++ b/lib/src/command/cache_repair.dart
@@ -23,6 +23,8 @@
@override
Future<void> runProtected() async {
+ // Delete any eventual temp-files left in the cache.
+ cache.deleteTempDir();
// Repair every cached source.
final repairResults = (await Future.wait(
cache.sources.all.map(cache.source).map((source) async {
diff --git a/lib/src/io.dart b/lib/src/io.dart
index d351e2b..6d0bf83 100644
--- a/lib/src/io.dart
+++ b/lib/src/io.dart
@@ -361,52 +361,54 @@
/// when we try to delete or move something while it's being scanned. To
/// mitigate that, on Windows, this will retry the operation a few times if it
/// fails.
-void _attempt(String description, void Function() operation) {
+///
+/// For some operations it makes sense to handle ERROR_DIR_NOT_EMPTY
+/// differently. They can pass [ignoreEmptyDir] = `true`.
+void _attempt(String description, void Function() operation,
+ {bool ignoreEmptyDir = false}) {
if (!Platform.isWindows) {
operation();
return;
}
String? getErrorReason(FileSystemException error) {
+ // ERROR_ACCESS_DENIED
if (error.osError?.errorCode == 5) {
return 'access was denied';
}
+ // ERROR_SHARING_VIOLATION
if (error.osError?.errorCode == 32) {
return 'it was in use by another process';
}
- if (error.osError?.errorCode == 145) {
+ // ERROR_DIR_NOT_EMPTY
+ if (!ignoreEmptyDir && isDirectoryNotEmptyException(error)) {
return 'of dart-lang/sdk#25353';
}
return null;
}
- for (var i = 0; i < 2; i++) {
+ for (var i = 0; i < 3; i++) {
try {
operation();
- return;
+ break;
} on FileSystemException catch (error) {
var reason = getErrorReason(error);
if (reason == null) rethrow;
- log.io('Pub failed to $description because $reason. '
- 'Retrying in 50ms.');
- sleep(Duration(milliseconds: 50));
+ if (i < 2) {
+ log.io('Pub failed to $description because $reason. '
+ 'Retrying in 50ms.');
+ sleep(Duration(milliseconds: 50));
+ } else {
+ fail('Pub failed to $description because $reason.\n'
+ 'This may be caused by a virus scanner or having a file\n'
+ 'in the directory open in another application.');
+ }
}
}
-
- try {
- operation();
- } on FileSystemException catch (error) {
- var reason = getErrorReason(error);
- if (reason == null) rethrow;
-
- fail('Pub failed to $description because $reason.\n'
- 'This may be caused by a virus scanner or having a file\n'
- 'in the directory open in another application.');
- }
}
/// Deletes whatever's at [path], whether it's a file, directory, or symlink.
@@ -451,14 +453,25 @@
void renameDir(String from, String to) {
_attempt('rename directory', () {
log.io('Renaming directory $from to $to.');
- try {
- Directory(from).renameSync(to);
- } on IOException {
- // Ensure that [to] isn't left in an inconsistent state. See issue 12436.
- if (entryExists(to)) deleteEntry(to);
- rethrow;
- }
- });
+ Directory(from).renameSync(to);
+ }, ignoreEmptyDir: true);
+}
+
+bool isDirectoryNotEmptyException(FileSystemException e) {
+ final errorCode = e.osError?.errorCode;
+ return
+ // On Linux rename will fail with ENOTEMPTY if directory exists:
+ // https://man7.org/linux/man-pages/man2/rename.2.html
+ // #define ENOTEMPTY 39 /* Directory not empty */
+ // https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno.h#n20
+ (Platform.isLinux && errorCode == 39) ||
+ // On Windows this may fail with ERROR_DIR_NOT_EMPTY
+ // https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
+ (Platform.isWindows && errorCode == 145) ||
+ // On MacOS rename will fail with ENOTEMPTY if directory exists.
+ // #define ENOTEMPTY 66 /* Directory not empty */
+ // https://github.com/apple-oss-distributions/xnu/blob/bb611c8fecc755a0d8e56e2fa51513527c5b7a0e/bsd/sys/errno.h#L190
+ (Platform.isMacOS && errorCode == 66);
}
/// Creates a new symlink at path [symlink] that points to [target].
diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart
index 1cb1950..e371363 100644
--- a/lib/src/source/hosted.dart
+++ b/lib/src/source/hosted.dart
@@ -704,6 +704,7 @@
packages.map((package) async {
var id = source.idFor(package.name, package.version, url: url);
try {
+ deleteEntry(package.dir);
await _download(id, package.dir);
return RepairResult(id, success: true);
} catch (error, stackTrace) {
@@ -812,14 +813,23 @@
var tempDir = systemCache.createTempDir();
await extractTarGz(readBinaryFileAsSream(archivePath), tempDir);
- // Remove the existing directory if it exists. This will happen if
- // we're forcing a download to repair the cache.
- if (dirExists(destPath)) deleteEntry(destPath);
-
// Now that the get has succeeded, move it to the real location in the
- // cache. This ensures that we don't leave half-busted ghost
- // directories in the user's pub cache if a get fails.
- renameDir(tempDir, destPath);
+ // cache.
+ //
+ // If this fails with a "directory not empty" exception we assume that
+ // another pub process has installed the same package version while we
+ // downloaded.
+ try {
+ renameDir(tempDir, destPath);
+ } on io.FileSystemException catch (e) {
+ tryDeleteEntry(tempDir);
+ if (!isDirectoryNotEmptyException(e)) {
+ rethrow;
+ }
+ log.fine('''
+Destination directory $destPath already existed.
+Assuming a concurrent pub invocation installed it.''');
+ }
});
}
