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 015b609ec2290a..bb92bbbe232063 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/syntax", "//third_party:auto_value", "//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 688ac7f97160ef..0248461ed72bbf 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; @@ -79,6 +80,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; @@ -148,6 +150,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(