Internal change

PiperOrigin-RevId: 191642942

10 files changed, 157 insertions, 119 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
index 9fee6544dd..33b21209da 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -50,11 +50,13 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
   private static final String SANDBOX_DEBUG_SUGGESTION =
       "\n\nUse --sandbox_debug to see verbose messages from the sandbox";
 
+  private final Path sandboxBase;
   private final SandboxOptions sandboxOptions;
   private final boolean verboseFailures;
   private final ImmutableSet<Path> inaccessiblePaths;
 
-  public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv) {
+  public AbstractSandboxSpawnRunner(CommandEnvironment cmdEnv, Path sandboxBase) {
+    this.sandboxBase = sandboxBase;
     this.sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class);
     this.verboseFailures = cmdEnv.getOptions().getOptions(ExecutionOptions.class).verboseFailures;
     this.inaccessiblePaths =
@@ -86,6 +88,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       SandboxedSpawn sandbox,
       SpawnExecutionPolicy policy,
       Path execRoot,
+      Path tmpDir,
       Duration timeout,
       Path statisticsPath)
       throws IOException, InterruptedException {
@@ -94,7 +97,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       OutErr outErr = policy.getFileOutErr();
       policy.prefetchInputs();
 
-      SpawnResult result = run(originalSpawn, sandbox, outErr, timeout, execRoot, statisticsPath);
+      SpawnResult result =
+          run(originalSpawn, sandbox, outErr, timeout, execRoot, tmpDir, statisticsPath);
 
       policy.lockOutputFiles();
       try {
@@ -117,6 +121,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       OutErr outErr,
       Duration timeout,
       Path execRoot,
+      Path tmpDir,
       Path statisticsPath)
       throws IOException, InterruptedException {
     Command cmd = new Command(
@@ -140,6 +145,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     long startTime = System.currentTimeMillis();
     CommandResult commandResult;
     try {
+      if (!tmpDir.exists() && !tmpDir.createDirectory()) {
+        throw new IOException(String.format("Could not create temp directory '%s'", tmpDir));
+      }
       commandResult = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException();
@@ -206,6 +214,17 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
   }
 
   /**
+   * Returns a temporary directory that should be used as the sandbox directory for a single action.
+   */
+  protected Path getSandboxRoot() throws IOException {
+    return sandboxBase.getRelative(
+        java.nio.file.Files.createTempDirectory(
+                java.nio.file.Paths.get(sandboxBase.getPathString()), "")
+            .getFileName()
+            .toString());
+  }
+
+  /**
    * Gets the list of directories that the spawn will assume to be writable.
    *
    * @throws IOException because we might resolve symlinks, which throws {@link IOException}.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
index d20eec70b1..1060034ccc 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteStreams;
+import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
@@ -95,7 +96,6 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   private final Path execRoot;
   private final boolean allowNetwork;
   private final Path processWrapper;
-  private final Path sandboxBase;
   private final Duration timeoutKillDelay;
   private final @Nullable SandboxfsProcess sandboxfsProcess;
 
@@ -123,14 +123,13 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
       Duration timeoutKillDelay,
       @Nullable SandboxfsProcess sandboxfsProcess)
       throws IOException {
-    super(cmdEnv);
+    super(cmdEnv, sandboxBase);
     this.execRoot = cmdEnv.getExecRoot();
     this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem());
     this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv);
     this.localEnvProvider =
         new XcodeLocalEnvProvider(cmdEnv.getRuntime().getProductName(), cmdEnv.getClientEnv());
-    this.sandboxBase = sandboxBase;
     this.timeoutKillDelay = timeoutKillDelay;
     this.sandboxfsProcess = sandboxfsProcess;
   }
@@ -194,19 +193,21 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
   @Override
   protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy)
-      throws IOException, InterruptedException {
+      throws ExecException, IOException, InterruptedException {
     // Each invocation of "exec" gets its own sandbox.
-    Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId()));
-    sandboxPath.createDirectory();
-
-    // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like
-    // the normal execroot does.
+    Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
-    sandboxExecRoot.getParentDirectory().createDirectory();
-    sandboxExecRoot.createDirectory();
+
+    // Each sandboxed action runs in its own directory so we don't need to make the temp directory's
+    // name unique (like we have to with standalone execution strategy).
+    //
+    // Note that, for sandboxfs-based executions, this temp directory lives outside of the sandboxfs
+    // instance. This is perfectly fine (because sandbox-exec controls accesses to this directory)
+    // and is actually desirable for performance reasons.
+    Path tmpDir = sandboxPath.getRelative("tmp");
 
     Map<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp");
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString());
 
     final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
     ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment);
@@ -287,7 +288,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
             }
           };
     }
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath);
   }
 
   private void writeConfig(
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
index a4c05ea5a7..fc8a3c3fad 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
@@ -33,6 +33,6 @@ final class DarwinSandboxedStrategy extends AbstractSpawnStrategy {
 
   @Override
   public String toString() {
-    return "darwin-sandbox";
+    return "sandboxed";
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index 0d6c7e1410..e4c5ba6524 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -76,7 +76,6 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   private final Path execRoot;
   private final boolean allowNetwork;
   private final Path linuxSandbox;
-  private final Path sandboxBase;
   private final Path inaccessibleHelperFile;
   private final Path inaccessibleHelperDir;
   private final LocalEnvProvider localEnvProvider;
@@ -97,13 +96,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
       Path inaccessibleHelperFile,
       Path inaccessibleHelperDir,
       Duration timeoutKillDelay) {
-    super(cmdEnv);
+    super(cmdEnv, sandboxBase);
     this.fileSystem = cmdEnv.getRuntime().getFileSystem();
     this.blazeDirs = cmdEnv.getDirectories();
     this.execRoot = cmdEnv.getExecRoot();
     this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.linuxSandbox = LinuxSandboxUtil.getLinuxSandbox(cmdEnv);
-    this.sandboxBase = sandboxBase;
     this.inaccessibleHelperFile = inaccessibleHelperFile;
     this.inaccessibleHelperDir = inaccessibleHelperDir;
     this.timeoutKillDelay = timeoutKillDelay;
@@ -113,18 +111,16 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   @Override
   protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy)
       throws IOException, ExecException, InterruptedException {
-    // Each invocation of "exec" gets its own sandbox base, execroot and temporary directory.
-    Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId()));
-    sandboxPath.createDirectory();
-
-    // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like
-    // the normal execroot does.
+    // Each invocation of "exec" gets its own sandbox.
+    Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
-    sandboxExecRoot.getParentDirectory().createDirectory();
-    sandboxExecRoot.createDirectory();
+
+    // Each sandboxed action runs in its own execroot, so we don't need to make the temp directory's
+    // name unique (like we have to with standalone execution strategy).
+    Path tmpDir = sandboxExecRoot.getRelative("tmp");
 
     Map<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp");
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString());
 
     ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment);
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
@@ -137,13 +133,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
             .setBindMounts(getReadOnlyBindMounts(blazeDirs, sandboxExecRoot))
             .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname)
             .setCreateNetworkNamespace(!(allowNetwork || Spawns.requiresNetwork(spawn)))
-            .setUseDebugMode(getSandboxOptions().sandboxDebug)
-            .setKillDelay(timeoutKillDelay);
+            .setUseDebugMode(getSandboxOptions().sandboxDebug);
 
     if (!timeout.isZero()) {
       commandLineBuilder.setTimeout(timeout);
     }
-
+    commandLineBuilder.setKillDelay(timeoutKillDelay);
     if (spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_FAKEROOT)) {
       commandLineBuilder.setUseFakeRoot(true);
     } else if (getSandboxOptions().sandboxFakeUsername) {
@@ -166,7 +161,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
             outputs,
             writableDirs);
 
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index 1993fb8c20..bbebefa81d 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -37,7 +37,7 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy {
 
   @Override
   public String toString() {
-    return "linux-sandbox";
+    return "sandboxed";
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
index e79b2abcd7..fdc4abfdd7 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -35,9 +35,8 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
     return OS.isPosixCompatible() && ProcessWrapperUtil.isSupported(cmdEnv);
   }
 
-  private final Path processWrapper;
   private final Path execRoot;
-  private final Path sandboxBase;
+  private final Path processWrapper;
   private final LocalEnvProvider localEnvProvider;
   private final Duration timeoutKillDelay;
 
@@ -51,32 +50,29 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
    */
   ProcessWrapperSandboxedSpawnRunner(
       CommandEnvironment cmdEnv, Path sandboxBase, String productName, Duration timeoutKillDelay) {
-    super(cmdEnv);
-    this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv);
+    super(cmdEnv, sandboxBase);
     this.execRoot = cmdEnv.getExecRoot();
+    this.timeoutKillDelay = timeoutKillDelay;
+    this.processWrapper = ProcessWrapperUtil.getProcessWrapper(cmdEnv);
     this.localEnvProvider =
         OS.getCurrent() == OS.DARWIN
             ? new XcodeLocalEnvProvider(productName, cmdEnv.getClientEnv())
             : new PosixLocalEnvProvider(cmdEnv.getClientEnv());
-    this.sandboxBase = sandboxBase;
-    this.timeoutKillDelay = timeoutKillDelay;
   }
 
   @Override
   protected SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy)
       throws ExecException, IOException, InterruptedException {
     // Each invocation of "exec" gets its own sandbox.
-    Path sandboxPath = sandboxBase.getRelative(Integer.toString(policy.getId()));
-    sandboxPath.createDirectory();
-
-    // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like
-    // the normal execroot does.
+    Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
-    sandboxExecRoot.getParentDirectory().createDirectory();
-    sandboxExecRoot.createDirectory();
+
+    // Each sandboxed action runs in its own execroot, so we don't need to make the temp directory's
+    // name unique (like we have to with standalone execution strategy).
+    Path tmpDir = sandboxExecRoot.getRelative("tmp");
 
     Map<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, "/tmp");
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir.getPathString());
 
     Duration timeout = policy.getTimeout();
     ProcessWrapperUtil.CommandLineBuilder commandLineBuilder =
@@ -101,7 +97,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
             SandboxHelpers.getOutputFiles(spawn),
             getWritableDirs(sandboxExecRoot, environment));
 
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout, statisticsPath);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout, statisticsPath);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
index 096f564ad8..83c279a726 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java
@@ -33,6 +33,6 @@ final class ProcessWrapperSandboxedStrategy extends AbstractSpawnStrategy {
 
   @Override
   public String toString() {
-    return "processwrapper-sandbox";
+    return "sandboxed";
   }
 }
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 a8fb190e48..3065393a96 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
@@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.eventbus.Subscribe;
-import com.google.devtools.build.lib.actions.ExecutorInitException;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
@@ -29,6 +28,8 @@ import com.google.devtools.build.lib.exec.ExecutorBuilder;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -66,16 +67,14 @@ public final class SandboxModule extends BlazeModule {
   }
 
   /** Computes the path to the sandbox base tree for the given running command. */
-  private static Path computeSandboxBase(SandboxOptions options, CommandEnvironment env)
-      throws IOException {
+  private static Path computeSandboxBase(SandboxOptions options, CommandEnvironment env) {
     if (options.sandboxBase.isEmpty()) {
       return env.getOutputBase().getRelative("sandbox");
     } else {
       String dirName = String.format("%s-sandbox.%s", env.getRuntime().getProductName(),
           Fingerprint.md5Digest(env.getOutputBase().toString()));
       FileSystem fileSystem = env.getRuntime().getFileSystem();
-      Path resolvedSandboxBase = fileSystem.getPath(options.sandboxBase).resolveSymbolicLinks();
-      return resolvedSandboxBase.getRelative(dirName);
+      return fileSystem.getPath(options.sandboxBase).getRelative(dirName);
     }
   }
 
@@ -92,30 +91,18 @@ public final class SandboxModule extends BlazeModule {
   }
 
   @Override
-  public void executorInit(CommandEnvironment cmdEnv, BuildRequest request, ExecutorBuilder builder)
-      throws ExecutorInitException {
+  public void executorInit(
+      CommandEnvironment cmdEnv, BuildRequest request, ExecutorBuilder builder) {
     checkNotNull(env, "env not initialized; was beforeCommand called?");
 
     SandboxOptions options = env.getOptions().getOptions(SandboxOptions.class);
     checkNotNull(options, "We were told to initialize the executor but the SandboxOptions are "
         + "not present; were they registered for all build commands?");
 
-    try {
-      sandboxBase = computeSandboxBase(options, env);
-    } catch (IOException e) {
-      throw new ExecutorInitException(
-          "--experimental_sandbox_base points to an invalid directory", e);
-    }
+    sandboxBase = computeSandboxBase(options, env);
 
     ActionContextProvider provider;
     try {
-      // Ensure that each build starts with a clean sandbox base directory. Otherwise using the `id`
-      // that is provided by SpawnExecutionPolicy#getId to compute a base directory for a sandbox
-      // might result in an already existing directory.
-      if (sandboxBase.exists()) {
-        FileSystemUtils.deleteTree(sandboxBase);
-      }
-
       sandboxBase.createDirectoryAndParents();
       if (options.useSandboxfs) {
         Path mountPoint = sandboxBase.getRelative("sandboxfs");
@@ -130,7 +117,11 @@ public final class SandboxModule extends BlazeModule {
         provider = SandboxActionContextProvider.create(cmdEnv, sandboxBase, null);
       }
     } catch (IOException e) {
-      throw new ExecutorInitException("Failed to initialize sandbox", e);
+      env.getBlazeModuleEnvironment().exit(
+          new AbruptExitException(
+              "Failed to initialize sandbox: " + e,
+              ExitCode.LOCAL_ENVIRONMENTAL_ERROR));
+      return;
     }
     builder.addActionContextProvider(provider);
     builder.addActionContextConsumer(new SandboxActionContextConsumer(cmdEnv));
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java
index aba4454334..e6408dc3cc 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java
@@ -14,11 +14,10 @@
 
 package com.google.devtools.build.lib.sandbox;
 
-import com.google.devtools.build.lib.vfs.FileStatus;
+import com.google.common.io.Files;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
@@ -77,33 +76,19 @@ interface SandboxedSpawn {
     for (PathFragment output : outputs) {
       Path source = sourceRoot.getRelative(output);
       Path target = targetRoot.getRelative(output);
-
-      FileStatus stat = source.statNullable(Symlinks.NOFOLLOW);
-      if (stat != null) {
+      if (source.isFile() || source.isSymbolicLink()) {
         // Ensure the target directory exists in the target. The directories for the action outputs
         // have already been created, but the spawn outputs may be different from the overall action
         // outputs. This is the case for test actions.
-        Path parentDir = target.getParentDirectory();
-        if (parentDir != null) {
-          parentDir.createDirectoryAndParents();
-        }
-
-        if (stat.isSymbolicLink()) {
-          try {
-            source.renameTo(target);
-          } catch (IOException e) {
-            target.createSymbolicLink(source.readSymbolicLink());
-          }
-        } else if (stat.isFile()) {
-          FileSystemUtils.moveFile(source, target);
-        } else if (stat.isDirectory()) {
-          try {
-            source.renameTo(target);
-          } catch (IOException e) {
-            // Failed to move directory directly, thus move it recursively.
-            target.createDirectory();
-            FileSystemUtils.moveTreesBelow(source, target);
-          }
+        target.getParentDirectory().createDirectoryAndParents();
+        Files.move(source.getPathFile(), target.getPathFile());
+      } else if (source.isDirectory()) {
+        try {
+          source.renameTo(target);
+        } catch (IOException e) {
+          // Failed to move directory directly, thus move it recursively.
+          target.createDirectory();
+          FileSystemUtils.moveTreesBelow(source, target);
         }
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
index d2806db1a1..f4d41e38a3 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java
@@ -15,13 +15,14 @@
 package com.google.devtools.build.lib.sandbox;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.IOException;
 import java.util.Collection;
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -74,8 +75,36 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn {
 
   @Override
   public void createFileSystem() throws IOException {
-    createDirectories();
+    Set<Path> createdDirs = new HashSet<>();
+    cleanFileSystem(inputs.keySet());
+    createDirectoryAndParentsWithCache(createdDirs, sandboxExecRoot);
+    createParentDirectoriesForInputs(createdDirs, inputs.keySet());
     createInputs(inputs);
+    createWritableDirectories(createdDirs, writableDirs);
+    createDirectoriesForOutputs(createdDirs, outputs);
+  }
+
+  private void cleanFileSystem(Set<PathFragment> allowedFiles) throws IOException {
+    if (sandboxExecRoot.exists(Symlinks.NOFOLLOW)) {
+      deleteExceptAllowedFiles(sandboxExecRoot, allowedFiles);
+    }
+  }
+
+  private void deleteExceptAllowedFiles(Path root, Set<PathFragment> allowedFiles)
+      throws IOException {
+    for (Path p : root.getDirectoryEntries()) {
+      FileStatus stat = p.stat(Symlinks.NOFOLLOW);
+      if (!stat.isDirectory()) {
+        if (!allowedFiles.contains(p.relativeTo(sandboxExecRoot))) {
+          p.delete();
+        }
+      } else {
+        deleteExceptAllowedFiles(p, allowedFiles);
+        if (p.readdir(Symlinks.NOFOLLOW).isEmpty()) {
+          p.delete();
+        }
+      }
+    }
   }
 
   /**
@@ -90,32 +119,29 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn {
    * directories, too, because we'll get an IOException with EEXIST if inputs happen to be nested
    * once we start creating the symlinks for all inputs.
    */
-  private void createDirectories() throws IOException {
-    LinkedHashSet<Path> dirsToCreate = new LinkedHashSet<>();
-
-    for (PathFragment path : Iterables.concat(inputs.keySet(), outputs)) {
-      Preconditions.checkArgument(!path.isAbsolute());
-      Preconditions.checkArgument(!path.containsUplevelReferences());
-      for (int i = 0; i < path.segmentCount(); i++) {
-        dirsToCreate.add(sandboxExecRoot.getRelative(path.subFragment(0, i)));
-      }
-    }
-
-    for (Path path : dirsToCreate) {
-      path.createDirectory();
-    }
-
-    for (Path dir : writableDirs) {
-      if (dir.startsWith(sandboxExecRoot)) {
-        dir.createDirectoryAndParents();
-      }
+  private void createParentDirectoriesForInputs(Set<Path> createdDirs, Set<PathFragment> inputs)
+      throws IOException {
+    for (PathFragment inputPath : inputs) {
+      Path dir = sandboxExecRoot.getRelative(inputPath).getParentDirectory();
+      Preconditions.checkArgument(
+          dir.startsWith(sandboxExecRoot), "Bad relative path: '%s'", inputPath);
+      createDirectoryAndParentsWithCache(createdDirs, dir);
     }
   }
 
-  protected void createInputs(Map<PathFragment, Path> inputs) throws IOException {
+  private void createInputs(Map<PathFragment, Path> inputs) throws IOException {
     // All input files are relative to the execroot.
     for (Entry<PathFragment, Path> entry : inputs.entrySet()) {
       Path key = sandboxExecRoot.getRelative(entry.getKey());
+      FileStatus keyStat = key.statNullable(Symlinks.NOFOLLOW);
+      if (keyStat != null) {
+        if (keyStat.isSymbolicLink()
+            && entry.getValue() != null
+            && key.readSymbolicLink().equals(entry.getValue().asFragment())) {
+          continue;
+        }
+        key.delete();
+      }
       // A null value means that we're supposed to create an empty file as the input.
       if (entry.getValue() != null) {
         key.createSymbolicLink(entry.getValue());
@@ -125,6 +151,24 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn {
     }
   }
 
+  private void createWritableDirectories(Set<Path> createdDirs, Set<Path> writableDirs)
+      throws IOException {
+    for (Path writablePath : writableDirs) {
+      if (writablePath.startsWith(sandboxExecRoot)) {
+        createDirectoryAndParentsWithCache(createdDirs, writablePath);
+      }
+    }
+  }
+
+  /** Prepare the output directories in the sandbox. */
+  private void createDirectoriesForOutputs(Set<Path> createdDirs, Collection<PathFragment> outputs)
+      throws IOException {
+    for (PathFragment output : outputs) {
+      createDirectoryAndParentsWithCache(
+          createdDirs, sandboxExecRoot.getRelative(output.getParentDirectory()));
+    }
+  }
+
   @Override
   public void copyOutputs(Path execRoot) throws IOException {
     SandboxedSpawn.moveOutputs(outputs, sandboxExecRoot, execRoot);
@@ -144,4 +188,11 @@ public class SymlinkedSandboxedSpawn implements SandboxedSpawn {
       // on here.
     }
   }
+
+  private static void createDirectoryAndParentsWithCache(Set<Path> cache, Path dir)
+      throws IOException {
+    if (cache.add(dir)) {
+      dir.createDirectoryAndParents();
+    }
+  }
 }