Stop stashing tmp directories for tests

Test actions create a `_tmp` directory that doesn't make sense to stash between different test actions when using `--reuse_sandbox_directories`. This change makes sure that doesn't happen by deleting the directory before stashing.

Closes #21412.

Fixes #21253.

PiperOrigin-RevId: 608566281
Change-Id: I7186f8e5eba7b3110b10963df69ec66cd1402b2c

src/main/java/com/google/devtools/build/lib/sandbox/SandboxStash.java

@@ -119,32 +119,44 @@ private boolean takeStashedSandboxInternal(
 
   /** Atomically moves the sandboxPath directory aside for later reuse. */
   static void stashSandbox(
-      Path path, String mnemonic, Map<String, String> environment, SandboxOutputs outputs) {
+      Path path,
+      String mnemonic,
+      Map<String, String> environment,
+      SandboxOutputs outputs,
+      TreeDeleter treeDeleter) {
     if (instance == null) {
       return;
     }
-    instance.stashSandboxInternal(path, mnemonic, environment, outputs);
+    instance.stashSandboxInternal(path, mnemonic, environment, outputs, treeDeleter);
   }
 
   private void stashSandboxInternal(
-      Path path, String mnemonic, Map<String, String> environment, SandboxOutputs outputs) {
+      Path path,
+      String mnemonic,
+      Map<String, String> environment,
+      SandboxOutputs outputs,
+      TreeDeleter treeDeleter) {
     Path sandboxes = getSandboxStashDir(mnemonic, path.getFileSystem());
     if (sandboxes == null || isTestXmlGenerationOrCoverageSpawn(mnemonic, outputs)) {
       return;
     }
-    String stashName;
-    synchronized (stash) {
-      stashName = Integer.toString(stash.incrementAndGet());
-    }
+    String stashName = Integer.toString(stash.incrementAndGet());
     Path stashPath = sandboxes.getChild(stashName);
     if (!path.exists()) {
       return;
     }
     try {
       stashPath.createDirectory();
       Path stashPathExecroot = stashPath.getChild("execroot");
+      if (isTestAction(mnemonic)) {
+        if (environment.get("TEST_TMPDIR").startsWith("_tmp")) {
+          treeDeleter.deleteTree(
+              path.getRelative("execroot/" + environment.get("TEST_WORKSPACE") + "/_tmp"));
+        }
+      }
       path.getChild("execroot").renameTo(stashPathExecroot);
       if (isTestAction(mnemonic)) {
+        // We only do this after the rename operation has succeeded
         stashPathToRunfilesDir.put(stashPathExecroot, getCurrentRunfilesDir(environment));
       }
     } catch (IOException e) {

src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java

@@ -102,7 +102,7 @@ protected void copyFile(Path source, Path target) throws IOException {
 
   @Override
   public void delete() {
-    SandboxStash.stashSandbox(sandboxPath, mnemonic, getEnvironment(), outputs);
+    SandboxStash.stashSandbox(sandboxPath, mnemonic, getEnvironment(), outputs, treeDeleter);
     super.delete();
   }
 

src/test/shell/integration/sandboxing_test.sh

@@ -951,7 +951,7 @@ EOF
   bazel shutdown
 }
 
-function test_runfiles_from_tests_get_reused() {
+function test_runfiles_from_tests_get_reused_and_tmp_clean() {
   mkdir pkg
   touch pkg/file.txt
   cat >pkg/reusing_test.bzl <<'EOF'
@@ -1001,18 +1001,28 @@ reused_runfiles_test(
 EOF
 
   test_output="reuse_test_output.txt"
+  local out_directory
   if is_bazel; then
     bazel coverage --test_output=streamed \
       --experimental_split_coverage_postprocessing=1 \
       --experimental_fetch_all_coverage_outputs //pkg:a > ${test_output} \
       || fail "Expected build to succeed"
+    out_directory="bazel-out"
   else
     bazel test --test_output=streamed //pkg:a > ${test_output} \
       || fail "Expected build to succeed"
+    out_directory="blaze-out"
   fi
   dir_inode_a=$(awk '/The directory inode is/ {print $5}' ${test_output})
   file_inode_a=$(awk '/The file inode is/ {print $5}' ${test_output})
 
+  local output_base="$(bazel info output_base)"
+  local stashed_test_dir="${output_base}/sandbox/sandbox_stash/TestRunner/6/execroot/$WORKSPACE_NAME"
+  [[ -d "${stashed_test_dir}/$out_directory" ]] \
+    || fail "${stashed_test_dir}/$out_directory directory not present"
+  [[ -d "${stashed_test_dir}/_tmp" ]] \
+      && fail "${stashed_test_dir}/_tmp directory is present"
+
   if is_bazel; then
     bazel coverage --test_output=streamed //pkg:b \
       --experimental_split_coverage_postprocessing=1 \