diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 7ccb9b1d9c8799..7e9b206a718169 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD @@ -60,6 +60,7 @@ java_library( "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//third_party:error_prone_annotations", + "//third_party:flogger", "//third_party:guava", "//third_party:java-diff-utils", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java index 131ca7373637a1..07992edb074199 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Maps; +import com.google.common.flogger.GoogleLogger; import com.google.common.util.concurrent.Futures; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; @@ -80,6 +81,7 @@ import java.nio.file.InvalidPathException; import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Base64; @@ -149,6 +151,8 @@ private interface AsyncTask extends SilentCloseable { void close(); } + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + /** Max. length of command line args added as a profiler description. */ private static final int MAX_PROFILE_ARGS_LEN = 512; @@ -1110,21 +1114,55 @@ public StructImpl downloadAndExtract( } StructImpl downloadResult = calculateDownloadResult(checksum, downloadedPath); - try { - if (downloadDirectory.exists()) { - downloadDirectory.deleteTree(); + deleteTreeWithRetries(downloadDirectory); + return downloadResult; + } + + /** + * This method wraps the deleteTree method in a retry loop, to solve an issue when trying to + * recursively clean up temporary directories during dependency downloads when they are stored on + * filesystems where unlinking a file may not be immediately reflected in a list of its parent + * directory. Specifically, the symptom of this problem was the entire bazel build aborting + * because during the cleanup of a dependency download (e.g Rust crate), there was an IOException + * because the parent directory being removed was "not empty" (yet). Please see + * https://github.com/bazelbuild/bazel/issues/23687 and + * https://github.com/bazelbuild/bazel/issues/20013 for further details. + * + * @param downloadDirectory + * @throws RepositoryFunctionException + */ + private static void deleteTreeWithRetries(Path downloadDirectory) + throws RepositoryFunctionException { + Instant start = Instant.now(); + Instant deadline = start.plus(Duration.ofSeconds(5)); + + for (int attempts = 1; ; attempts++) { + try { + if (downloadDirectory.exists()) { + downloadDirectory.deleteTree(); + } + if (attempts > 1) { + long elapsedMillis = Duration.between(start, Instant.now()).toMillis(); + logger.atInfo().log( + "Deleting %s took %d attempts over %dms.", + downloadDirectory.getPathString(), attempts, elapsedMillis); + } + break; + } catch (IOException e) { + if (Instant.now().isAfter(deadline)) { + throw new RepositoryFunctionException( + new IOException( + "Couldn't delete temporary directory (" + + downloadDirectory.getPathString() + + ") after " + + attempts + + " attempts: " + + e.getMessage(), + e), + Transience.TRANSIENT); + } } - } catch (IOException e) { - throw new RepositoryFunctionException( - new IOException( - "Couldn't delete temporary directory (" - + downloadDirectory.getPathString() - + "): " - + e.getMessage(), - e), - Transience.TRANSIENT); } - return downloadResult; } @StarlarkMethod(