If a temporary directory name clashes in the LocalSpawnRunner, try ge…

…nerating another temporary directory name instead of throwing an exception.

RELNOTES: None.
PiperOrigin-RevId: 178190769

src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java

@@ -18,6 +18,7 @@
 import static java.util.logging.Level.SEVERE;
 import static java.util.logging.Level.WARNING;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.io.ByteStreams;
@@ -48,6 +49,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.LongSupplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -131,21 +133,27 @@ public SpawnResult exec(
     }
   }
 
-  private static Path createActionTemp(Path execRoot) throws IOException {
-    String idStr =
-        // Make the name unique among other executor threads.
-        Long.toHexString(Thread.currentThread().getId())
-            + "_"
-            // Make the name unique among other temp directories that this thread has ever created.
-            // On Windows, file and directory deletion is asynchronous, meaning the previous temp
-            // directory name isn't immediately available for the next action that this thread runs.
-            // See https://github.com/bazelbuild/bazel/issues/4035
-            + Long.toHexString(ThreadLocalRandom.current().nextLong());
-    Path result = execRoot.getRelative("tmp" + idStr);
-    if (!result.exists() && !result.createDirectory()) {
-      throw new IOException(String.format("Could not create temp directory '%s'", result));
+  @VisibleForTesting
+  static Path createActionTemp(Path execRoot, LongSupplier randomLongGenerator) throws IOException {
+    Path tempDirPath;
+    do {
+      String idStr =
+          // Make the name unique among other executor threads.
+          Long.toHexString(Thread.currentThread().getId())
+              + "_"
+              // Make the name unique among other temp directories that this thread has ever
+              // created.
+              // On Windows, file and directory deletion is asynchronous, meaning the previous temp
+              // directory name isn't immediately available for the next action that this thread
+              // runs.
+              // See https://github.com/bazelbuild/bazel/issues/4035
+              + Long.toHexString(randomLongGenerator.getAsLong());
+      tempDirPath = execRoot.getRelative("tmp" + idStr);
+    } while (tempDirPath.exists());
+    if (!tempDirPath.createDirectory()) {
+      throw new IOException(String.format("Could not create temp directory '%s'", tempDirPath));
     }
-    return result;
+    return tempDirPath;
   }
 
   private final class SubprocessHandler {
@@ -255,7 +263,7 @@ private SpawnResult start() throws InterruptedException, IOException {
       stepLog(INFO, "running locally");
       setState(State.LOCAL_ACTION_RUNNING);
 
-      Path tmpDir = createActionTemp(execRoot);
+      Path tmpDir = createActionTemp(execRoot, () -> ThreadLocalRandom.current().nextLong());
       try {
         Command cmd;
         OutputStream stdOut = ByteStreams.nullOutputStream();

src/test/java/com/google/devtools/build/lib/BUILD

@@ -1210,6 +1210,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
         "//src/main/java/com/google/devtools/common/options",
+        "//src/test/java/com/google/devtools/build/lib:testutil",
         "//third_party:guava",
         "//third_party:junit4",
         "//third_party:mockito",

src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java

@@ -16,6 +16,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.argThat;
@@ -60,6 +61,7 @@
 import java.util.List;
 import java.util.SortedMap;
 import java.util.TreeMap;
+import java.util.function.LongSupplier;
 import java.util.logging.Filter;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
@@ -571,4 +573,41 @@ public void useCorrectExtensionOnWindows() throws Exception {
                 "/bin/echo",
                 "Hi!"));
   }
+
+  @Test
+  public void testCreateActionTemp_exceptionIfUnableToCreateDir() throws IOException {
+    Path execRoot = fs.getPath("/execroot");
+    assertThat(execRoot.createDirectory()).isTrue();
+    assertThat(execRoot.exists()).isTrue();
+    execRoot.setWritable(false);
+
+    assertThrows(IOException.class, () -> LocalSpawnRunner.createActionTemp(execRoot, () -> 0));
+  }
+
+  @Test
+  public void testCreateActionTemp_retriesIfNameClashes() throws IOException {
+    Path execRoot = fs.getPath("/execroot");
+    assertThat(execRoot.createDirectory()).isTrue();
+    assertThat(execRoot.exists()).isTrue();
+
+    Path tempPath1 = LocalSpawnRunner.createActionTemp(execRoot, () -> 0);
+    Path tempPath2 = LocalSpawnRunner.createActionTemp(execRoot, new IncreasingSequenceSupplier(0));
+
+    assertThat(tempPath1).isNotEqualTo(tempPath2);
+    assertThat(tempPath1.exists()).isTrue();
+    assertThat(tempPath2.exists()).isTrue();
+  }
+
+  private static class IncreasingSequenceSupplier implements LongSupplier {
+    private long currentElement;
+
+    public IncreasingSequenceSupplier(long startingElement) {
+      this.currentElement = startingElement;
+    }
+
+    @Override
+    public long getAsLong() {
+      return this.currentElement++;
+    }
+  }
 }