Clean up sandbox stashing directories on Bazel server shutdown. When using --reuse_sandbox_directories, stashed sandbox directories cannot be reused across server instances and were previously left on disk indefinitely when the server shut down or when developers switched workspaces. This led to unbounded disk accumulation over time. Fixes https://github.com/bazelbuild/bazel/issues/27626 RELNOTES: Clean up unused stashed sandbox directories when shutting down the Bazel server. PiperOrigin-RevId: 972583325 Change-Id: I54268307ffa6f0d2bd8371477cd07d90ea959459

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java b/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java
index c8245f5..172c7b4 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java

@@ -89,17 +89,21 @@
 
   @Override
   public void deleteTree(Path path) throws IOException {
-    if (!trashBaseCreated) {
-      trashBase.createDirectory();
-      trashBaseCreated = true;
-    }
     if (!path.exists()) {
       return;
     }
+    if (path.getFileSystem() != trashBase.getFileSystem()) {
+      path.deleteTree();
+      return;
+    }
+    if (!trashBaseCreated) {
+      trashBase.createDirectoryAndParents();
+      trashBaseCreated = true;
+    }
     Path trashPath = trashBase.getRelative(Integer.toString(trashCount.getAndIncrement()));
     try {
       path.renameTo(trashPath);
-    } catch (IOException e) {
+    } catch (IOException | IllegalArgumentException e) {
       logger.atWarning().withCause(e).log(
           "Failed to rename %s -> %s for asynchronous removal. Removing synchronously.",
           path, trashPath);

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
index bc7a5e4..bb1ae75 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java

@@ -485,6 +485,10 @@
     // Try to clean up as much garbage as possible, if there happens to be any. This will delay
     // server termination but it's the nice thing to do. If the user gets impatient, they can always
     // kill us again.
+    if (sandboxBase != null) {
+      SandboxStash.clean(treeDeleter, sandboxBase);
+    }
+
     if (treeDeleter != null) {
       try {
         treeDeleter.shutdown();

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStash.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStash.java
index ff88f32..d1a7d3e 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStash.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStash.java

@@ -363,16 +363,11 @@
             new SandboxStash(
                 workspaceName, sandboxBase, options.getExperimentalInMemorySandboxStashes());
       } else {
-        if (!Objects.equals(workspaceName, instance.workspaceName)) {
-          Path stashBase = getStashBase(instance.sandboxBase);
-          try (SilentCloseable c = Profiler.instance().profile("treeDeleter.deleteTree")) {
-            for (Path directoryEntry : stashBase.getDirectoryEntries()) {
-              treeDeleter.deleteTree(directoryEntry);
-            }
-          } catch (IOException e) {
-            instance.turnOffReuse(
-                "Unable to clear old sandbox stash %s: %s\n", stashBase, e.getMessage());
-          }
+        if (!Objects.equals(workspaceName, instance.workspaceName)
+            || !Objects.equals(sandboxBase, instance.sandboxBase)) {
+          clean(
+              !Objects.equals(sandboxBase, instance.sandboxBase) ? null : treeDeleter,
+              instance.sandboxBase);
           instance =
               new SandboxStash(
                   workspaceName, sandboxBase, options.getExperimentalInMemorySandboxStashes());
@@ -380,6 +375,11 @@
         instance.inMemoryStashes = options.getExperimentalInMemorySandboxStashes();
       }
     } else {
+      if (instance != null) {
+        clean(
+            !Objects.equals(sandboxBase, instance.sandboxBase) ? null : treeDeleter,
+            instance.sandboxBase);
+      }
       instance = null;
     }
   }
@@ -411,40 +411,31 @@
   }
 
   /** Cleans up the entire current stash, if any. Cleaning may be asynchronous. */
-  static void clean(TreeDeleter treeDeleter, Path sandboxBase) {
-    Path stashDir = getStashBase(sandboxBase);
-    try {
-      if (!stashDir.isDirectory()) {
-        return;
-      }
-    } catch (IOException e) {
-      logger.atWarning().withCause(e).log("Failed to stat sandbox stash %s", stashDir);
-      return;
-    }
-    Path stashTrashDir = stashDir.getChild("__trash");
-    try {
-      stashDir.renameTo(stashTrashDir);
-    } catch (IOException e) {
-      // If we couldn't move the stashdir away for deletion, we need to delete it synchronously
-      // in place, so we can't use the treeDeleter.
-      treeDeleter = null;
-      stashTrashDir = stashDir;
-    }
-    try {
-      if (treeDeleter != null) {
-        treeDeleter.deleteTree(stashTrashDir);
-      } else {
-        stashTrashDir.deleteTree();
-      }
-    } catch (IOException e) {
-      logger.atWarning().withCause(e).log("Failed to clean sandbox stash %s", stashDir);
-    }
+  static void clean(@Nullable TreeDeleter treeDeleter, Path sandboxBase) {
+    try (SilentCloseable c = Profiler.instance().profile("SandboxStash.clean")) {
+      Path stashDir = getStashBase(sandboxBase);
+      cleanDir(stashDir, treeDeleter);
+      Path tmpStashDir = sandboxBase.getChild(TEMPORARY_SANDBOX_STASH_BASE);
+      cleanDir(tmpStashDir, treeDeleter);
 
-    if (instance != null) {
-      instance.stashPathToRunfilesDir.clear();
-      instance.pathToContents.clear();
-      instance.sandboxToTarget.clear();
-      instance.pathToLastModified.clear();
+      if (instance != null) {
+        instance.stashPathToRunfilesDir.clear();
+        instance.pathToContents.clear();
+        instance.sandboxToTarget.clear();
+        instance.pathToLastModified.clear();
+      }
+    }
+  }
+
+  private static void cleanDir(Path dir, @Nullable TreeDeleter treeDeleter) {
+    try (SilentCloseable c = Profiler.instance().profile("treeDeleter.deleteTree")) {
+      if (treeDeleter != null) {
+        treeDeleter.deleteTree(dir);
+      } else {
+        dir.deleteTree();
+      }
+    } catch (IOException e) {
+      logger.atWarning().withCause(e).log("Failed to clean sandbox stash %s", dir);
     }
   }
 

diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java
index 08769ae..f4f39be 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxHelpersTest.java

@@ -508,5 +508,22 @@
 
     assertThat(trashBase.getDirectoryEntries()).isEmpty();
   }
+
+  @Test
+  public void asynchronousTreeDeleter_differentFileSystem_deletesSynchronously() throws Exception {
+    FileSystem fs1 = new InMemoryFileSystem(DigestHashFunction.SHA256);
+    FileSystem fs2 = new InMemoryFileSystem(DigestHashFunction.SHA256);
+    Path trashBase = fs1.getPath("/trash");
+    Path dir = fs2.getPath("/dir");
+    dir.createDirectoryAndParents();
+    dir.getChild("file.txt").createDirectoryAndParents();
+
+    AsynchronousTreeDeleter deleter = new AsynchronousTreeDeleter(trashBase);
+    deleter.deleteTree(dir);
+    deleter.shutdown();
+
+    assertThat(dir.exists()).isFalse();
+    assertThat(trashBase.exists()).isFalse();
+  }
 }
 

diff --git a/src/test/shell/integration/sandboxing_test.sh b/src/test/shell/integration/sandboxing_test.sh
index eadc733..cd6bbbd 100755
--- a/src/test/shell/integration/sandboxing_test.sh
+++ b/src/test/shell/integration/sandboxing_test.sh

@@ -164,11 +164,8 @@
     || fail "${sandbox_base} contains stale dirs"
 
   bazel shutdown
-  [[ -d "${sandbox_base}/sandbox_stash" ]] \
-    || fail "${sandbox_base}/_moved_trash_dir directory not present"
-  bazel build
   [[ ! -d "${sandbox_base}/sandbox_stash" ]] \
-    || fail "${sandbox_base}/_moved_trash_dir directory not present"
+    || fail "${sandbox_base}/sandbox_stash directory should be removed on shutdown"
   [[ $(ls -1 ${sandbox_base} | wc -l | tr -d ' ') == 1 ]] \
     || fail "${sandbox_base} contains stale dirs"
 }
@@ -973,10 +970,85 @@
   [[ ! -d "${sandbox_stash}" ]] \
     || fail "${sandbox_stash} present after clean"
 
-  bazel build //pkg:a >"${TEST_log}" 2>&1 \
+  bazel shutdown
+  [[ ! -d "${sandbox_stash}" ]] \
+    || fail "${sandbox_stash} present after shutdown"
+}
+
+function test_sandbox_reuse_stashes_cleaned_on_shutdown() {
+  mkdir -p pkg
+  cat >pkg/BUILD <<'EOF'
+genrule(
+  name = "a",
+  srcs = [ "a.txt" ],
+  outs = [ "aout.txt" ],
+  cmd = "wc $(location :a.txt) > $@",
+)
+EOF
+  echo A > pkg/a.txt
+  local output_base="$(bazel info output_base)"
+
+  bazel build --reuse_sandbox_directories //pkg:a >"${TEST_log}" 2>&1 \
     || fail "Expected build to succeed"
 
+  local sandbox_stash="${output_base}/sandbox/sandbox_stash"
+  [[ -d "${sandbox_stash}" ]] \
+    || fail "${sandbox_stash} not present during server lifetime"
+
   bazel shutdown
+
+  [[ ! -d "${sandbox_stash}" ]] \
+    || fail "${sandbox_stash} should be cleaned up after bazel shutdown"
+}
+
+function test_sandbox_reuse_stashes_cleaned_on_shutdown_multi_action() {
+  mkdir -p pkg
+  cat >pkg/custom_rule.bzl <<'EOF'
+def _custom_rule_impl(ctx):
+  out_a = ctx.actions.declare_file(ctx.label.name + "_A.txt")
+  ctx.actions.run_shell(
+      inputs = ctx.files.srcs,
+      outputs = [out_a],
+      command = "touch {}".format(out_a.path),
+      progress_message = "Action A: {}".format(out_a.basename),
+  )
+  out_b = ctx.actions.declare_file(ctx.label.name + "_B.txt")
+  ctx.actions.run_shell(
+      inputs = [out_a],
+      outputs = [out_b],
+      command = "touch {}".format(out_b.path),
+      progress_message = "Action B: {}".format(out_b.basename),
+  )
+  return DefaultInfo(files = depset([out_b]))
+
+custom_rule = rule(
+    implementation = _custom_rule_impl,
+    attrs = {
+        "srcs": attr.label_list(allow_files = True),
+    },
+)
+EOF
+  cat >pkg/BUILD <<'EOF'
+load(":custom_rule.bzl", "custom_rule")
+custom_rule(
+    name = "dummy_pipeline",
+    srcs = ["input.txt"],
+)
+EOF
+  touch pkg/input.txt
+  local output_base="$(bazel info output_base)"
+
+  bazel build --reuse_sandbox_directories //pkg:dummy_pipeline >"${TEST_log}" 2>&1 \
+    || fail "Expected build to succeed"
+
+  local sandbox_stash="${output_base}/sandbox/sandbox_stash"
+  [[ -d "${sandbox_stash}" ]] \
+    || fail "${sandbox_stash} not present after build"
+
+  bazel shutdown
+
+  [[ ! -d "${sandbox_stash}" ]] \
+    || fail "${sandbox_stash} should be cleaned up after bazel shutdown"
 }
 
 # This test does not currently work in Blaze. Not due to the inaccessible dirs