Actions now have a temp envvar.

Every build and test action that creates a Spawn
will now have platform-specific environment
variables for temp directories:
- on Windows: TMP and TEMP
- on Linux/Darwin: TMPDIR

This is particularly important on Windows where
e.g. Java programs cannot create temp directories
unless there's a valid TMP or TEMP environment
variable set.

Fixes:
- https://github.com/bazelbuild/bazel/issues/1590
- https://github.com/bazelbuild/bazel/issues/2349
- https://github.com/bazelbuild/bazel/issues/2870

Change-Id: Ib758307daf6b3a51b0f71ae5e65e5bb564dad643
PiperOrigin-RevId: 172326371

11 files changed, 256 insertions, 127 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java
index 57f1082978..e979958a17 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/apple/XCodeLocalEnvProvider.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec.apple;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
@@ -48,16 +49,19 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider {
 
   @Override
   public Map<String, String> rewriteLocalEnv(
-      Map<String, String> env, Path execRoot, String productName) throws IOException {
+      Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException {
     boolean containsXcodeVersion = env.containsKey(AppleConfiguration.XCODE_VERSION_ENV_NAME);
     boolean containsAppleSdkVersion =
         env.containsKey(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME);
+
+    ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder();
+    newEnvBuilder.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
+    newEnvBuilder.put("TMPDIR", tmpDir.getPathString());
+
     if (!containsXcodeVersion && !containsAppleSdkVersion) {
-      return env;
+      return newEnvBuilder.build();
     }
 
-    ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder();
-    newEnvBuilder.putAll(env);
     // Empty developer dir indicates to use the system default.
     // TODO(bazel-team): Bazel's view of the xcode version and developer dir should be explicitly
     // set for build hermeticity.
@@ -78,6 +82,7 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider {
           "SDKROOT",
           getSdkRoot(execRoot, developerDir, iosSdkVersion, appleSdkPlatform, productName));
     }
+
     return newEnvBuilder.build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java
index 52610761c5..e0bda380c3 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalEnvProvider.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.exec.local;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
 import java.util.Map;
@@ -22,15 +24,46 @@ import java.util.Map;
  * probably should not exist, but is currently necessary for our local MacOS support.
  */
 public interface LocalEnvProvider {
-  public static final LocalEnvProvider UNMODIFIED = new LocalEnvProvider() {
-    @Override
-    public Map<String, String> rewriteLocalEnv(
-        Map<String, String> env, Path execRoot, String productName) throws IOException {
-      return env;
-    }
-  };
+
+  public static final LocalEnvProvider UNMODIFIED =
+      new LocalEnvProvider() {
+        @Override
+        public Map<String, String> rewriteLocalEnv(
+            Map<String, String> env, Path execRoot, Path tmpDir, String productName)
+            throws IOException {
+          return env;
+        }
+      };
+
+  public static final LocalEnvProvider ADD_TEMP_POSIX =
+      new LocalEnvProvider() {
+        @Override
+        public Map<String, String> rewriteLocalEnv(
+            Map<String, String> env, Path execRoot, Path tmpDir, String productName)
+            throws IOException {
+          ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
+          result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
+          result.put("TMPDIR", tmpDir.getPathString());
+          return result.build();
+        }
+      };
+
+  public static final LocalEnvProvider ADD_TEMP_WINDOWS =
+      new LocalEnvProvider() {
+        @Override
+        public Map<String, String> rewriteLocalEnv(
+            Map<String, String> env, Path execRoot, Path tmpDir, String productName)
+            throws IOException {
+          ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
+          result.putAll(Maps.filterKeys(env, k -> !k.equals("TMP") && !k.equals("TEMP")));
+          String tmpPath = tmpDir.getPathString().replace('/', '\\');
+          result.put("TMP", tmpPath);
+          result.put("TEMP", tmpPath);
+          return result.build();
+        }
+      };
 
   /** Rewrites the environment if necessary. */
-  Map<String, String> rewriteLocalEnv(Map<String, String> env, Path execRoot, String productName)
-      throws IOException;
+  Map<String, String> rewriteLocalEnv(
+      Map<String, String> env, Path execRoot, Path tmpDir, String productName) throws IOException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index 09b69604f3..6aadb75dba 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.exec.local;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.SEVERE;
+import static java.util.logging.Level.WARNING;
 
 import com.google.common.base.Joiner;
 import com.google.common.io.ByteStreams;
@@ -37,6 +38,7 @@ import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -127,6 +129,23 @@ public final class LocalSpawnRunner implements SpawnRunner {
     }
   }
 
+  private static Path createActionTemp(Path execRoot) throws IOException {
+    // Use this executor thread's ID as the directory name's suffix.
+    // The ID is safe for the following reasons:
+    // - being a thread ID, it's guaranteed to be unique among other action executor threads
+    // - this thread will only execute one action at a time, so there's no risk of concurrently
+    //   running actions using the same temp directory.
+    // The next action that this thread executes can reuse the temp directory name. The only caveat
+    // is, if {@link #start} fails to delete directory after the action is done, the next
+    // action will see stale files in its temp directory.
+    String idStr = Long.toHexString(Thread.currentThread().getId());
+    Path result = execRoot.getRelative("tmp" + idStr);
+    if (!result.exists() && !result.createDirectory()) {
+      throw new IOException(String.format("Could not create temp directory '%s'", result));
+    }
+    return result;
+  }
+
   private final class SubprocessHandler {
     private final Spawn spawn;
     private final SpawnExecutionPolicy policy;
@@ -234,76 +253,103 @@ public final class LocalSpawnRunner implements SpawnRunner {
       stepLog(INFO, "running locally");
       setState(State.LOCAL_ACTION_RUNNING);
 
-      Command cmd;
-      OutputStream stdOut = ByteStreams.nullOutputStream();
-      OutputStream stdErr = ByteStreams.nullOutputStream();
-      if (useProcessWrapper) {
-        // If the process wrapper is enabled, we use its timeout feature, which first interrupts the
-        // subprocess and only kills it after a grace period so that the subprocess can output a
-        // stack trace, test log or similar, which is incredibly helpful for debugging. The process
-        // wrapper also supports output file redirection, so we don't need to stream the output
-        // through this process.
-        List<String> cmdLine = new ArrayList<>();
-        cmdLine.add(processWrapper);
-        cmdLine.add("--timeout=" + policy.getTimeout().getSeconds());
-        cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds);
-        cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath()));
-        cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath()));
-        cmdLine.addAll(spawn.getArguments());
-        cmd = new Command(
-            cmdLine.toArray(new String[0]),
-            localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName),
-            execRoot.getPathFile());
-      } else {
-        stdOut = outErr.getOutputStream();
-        stdErr = outErr.getErrorStream();
-        cmd = new Command(
-            spawn.getArguments().toArray(new String[0]),
-            localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName),
-            execRoot.getPathFile(),
-            policy.getTimeout());
-      }
-
-      long startTime = System.currentTimeMillis();
-      CommandResult result;
+      Path tmpDir = createActionTemp(execRoot);
       try {
-        result = cmd.execute(stdOut, stdErr);
-        if (Thread.currentThread().isInterrupted()) {
-          throw new InterruptedException();
+        Command cmd;
+        OutputStream stdOut = ByteStreams.nullOutputStream();
+        OutputStream stdErr = ByteStreams.nullOutputStream();
+        if (useProcessWrapper) {
+          // If the process wrapper is enabled, we use its timeout feature, which first interrupts
+          // the subprocess and only kills it after a grace period so that the subprocess can output
+          // a stack trace, test log or similar, which is incredibly helpful for debugging. The
+          // process wrapper also supports output file redirection, so we don't need to stream the
+          // output through this process.
+          List<String> cmdLine = new ArrayList<>();
+          cmdLine.add(processWrapper);
+          cmdLine.add("--timeout=" + policy.getTimeout().getSeconds());
+          cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds);
+          cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath()));
+          cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath()));
+          cmdLine.addAll(spawn.getArguments());
+          cmd =
+              new Command(
+                  cmdLine.toArray(new String[0]),
+                  localEnvProvider.rewriteLocalEnv(
+                      spawn.getEnvironment(), execRoot, tmpDir, productName),
+                  execRoot.getPathFile());
+        } else {
+          stdOut = outErr.getOutputStream();
+          stdErr = outErr.getErrorStream();
+          cmd =
+              new Command(
+                  spawn.getArguments().toArray(new String[0]),
+                  localEnvProvider.rewriteLocalEnv(
+                      spawn.getEnvironment(), execRoot, tmpDir, productName),
+                  execRoot.getPathFile(),
+                  policy.getTimeout());
         }
-      } catch (AbnormalTerminationException e) {
-        if (Thread.currentThread().isInterrupted()) {
-          throw new InterruptedException();
+
+        long startTime = System.currentTimeMillis();
+        CommandResult result = null;
+        try {
+          result = cmd.execute(stdOut, stdErr);
+          if (Thread.currentThread().isInterrupted()) {
+            throw new InterruptedException();
+          }
+        } catch (AbnormalTerminationException e) {
+          if (Thread.currentThread().isInterrupted()) {
+            throw new InterruptedException();
+          }
+          result = e.getResult();
+        } catch (CommandException e) {
+          // At the time this comment was written, this must be a ExecFailedException encapsulating
+          // an IOException from the underlying Subprocess.Factory.
+          String msg = e.getMessage() == null ? e.getClass().getName() : e.getMessage();
+          setState(State.PERMANENT_ERROR);
+          outErr
+              .getErrorStream()
+              .write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8));
+          outErr.getErrorStream().flush();
+          return new SpawnResult.Builder()
+              .setStatus(Status.EXECUTION_FAILED)
+              .setExitCode(LOCAL_EXEC_ERROR)
+              .setExecutorHostname(hostName)
+              .build();
         }
-        result = e.getResult();
-      } catch (CommandException e) {
-        // At the time this comment was written, this must be a ExecFailedException encapsulating an
-        // IOException from the underlying Subprocess.Factory.
-        String msg = e.getMessage() == null ? e.getClass().getName() : e.getMessage();
-        setState(State.PERMANENT_ERROR);
-        outErr.getErrorStream().write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8));
-        outErr.getErrorStream().flush();
+        setState(State.SUCCESS);
+        long wallTimeMillis = System.currentTimeMillis() - startTime;
+        boolean wasTimeout =
+            result.getTerminationStatus().timedout()
+                || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis));
+        Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS;
+        int exitCode =
+            status == Status.TIMEOUT
+                ? POSIX_TIMEOUT_EXIT_CODE
+                : result.getTerminationStatus().getRawExitCode();
         return new SpawnResult.Builder()
-            .setStatus(Status.EXECUTION_FAILED)
-            .setExitCode(LOCAL_EXEC_ERROR)
+            .setStatus(status)
+            .setExitCode(exitCode)
             .setExecutorHostname(hostName)
+            .setWallTimeMillis(wallTimeMillis)
             .build();
+      } finally {
+        // Delete the temp directory tree, so the next action that this thread executes will get a
+        // fresh, empty temp directory.
+        // File deletion tends to be slow on Windows, so deleting this tree may take several
+        // seconds. Delete it after having measured the wallTimeMillis.
+        try {
+          FileSystemUtils.deleteTree(tmpDir);
+        } catch (IOException ignored) {
+          // We can't handle this exception in any meaningful way, nor should we, but let's log it.
+          stepLog(
+              WARNING,
+              String.format(
+                  "failed to delete temp directory '%s'; this might indicate that the action "
+                      + "created subprocesses that didn't terminate and hold files open in that "
+                      + "directory",
+                  tmpDir));
+        }
       }
-      setState(State.SUCCESS);
-
-      long wallTimeMillis = System.currentTimeMillis() - startTime;
-      boolean wasTimeout = result.getTerminationStatus().timedout()
-          || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis));
-      Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS;
-      int exitCode = status == Status.TIMEOUT
-          ? POSIX_TIMEOUT_EXIT_CODE
-          : result.getTerminationStatus().getRawExitCode();
-      return new SpawnResult.Builder()
-          .setStatus(status)
-          .setExitCode(exitCode)
-          .setExecutorHostname(hostName)
-          .setWallTimeMillis(wallTimeMillis)
-          .build();
     }
 
     private String getPathOrDevNull(Path path) {
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 7eac6bb6be..b538fed1c6 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
@@ -77,6 +77,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     }
   }
 
+  // TODO(laszlocsomor): refactor this class to make `actuallyExec`'s contract clearer: the caller
+  // of `actuallyExec` should not depend on `actuallyExec` calling `runSpawn` because it's easy to
+  // forget to do so in `actuallyExec`'s implementations.
   protected abstract SpawnResult actuallyExec(Spawn spawn, SpawnExecutionPolicy policy)
       throws ExecException, InterruptedException, IOException;
 
@@ -85,14 +88,15 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       SandboxedSpawn sandbox,
       SpawnExecutionPolicy policy,
       Path execRoot,
+      Path tmpDir,
       Duration timeout)
-          throws ExecException, IOException, InterruptedException {
+      throws ExecException, IOException, InterruptedException {
     try {
       sandbox.createFileSystem();
       OutErr outErr = policy.getFileOutErr();
       policy.prefetchInputs();
 
-      SpawnResult result = run(sandbox, outErr, timeout);
+      SpawnResult result = run(sandbox, outErr, timeout, tmpDir);
 
       policy.lockOutputFiles();
       try {
@@ -126,7 +130,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     }
   }
 
-  private final SpawnResult run(SandboxedSpawn sandbox, OutErr outErr, Duration timeout)
+  private final SpawnResult run(
+      SandboxedSpawn sandbox, OutErr outErr, Duration timeout, Path tmpDir)
       throws IOException, InterruptedException {
     Command cmd = new Command(
         sandbox.getArguments().toArray(new String[0]),
@@ -136,6 +141,9 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     long startTime = System.currentTimeMillis();
     CommandResult result;
     try {
+      if (!tmpDir.exists() && !tmpDir.createDirectory()) {
+        throw new IOException(String.format("Could not create temp directory '%s'", tmpDir));
+      }
       result = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException();
@@ -190,8 +198,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
    *
    * @throws IOException because we might resolve symlinks, which throws {@link IOException}.
    */
-  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
-      throws IOException {
+  protected ImmutableSet<Path> getWritableDirs(
+      Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException {
     // We have to make the TEST_TMPDIR directory writable if it is specified.
     ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();
     writablePaths.add(sandboxExecRoot);
@@ -208,6 +216,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
       }
     }
 
+    writablePaths.add(tmpDir);
+
     FileSystem fileSystem = sandboxExecRoot.getFileSystem();
     for (String writablePath : sandboxOptions.sandboxWritablePath) {
       Path path = fileSystem.getPath(writablePath);
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 6227ac4915..cc3150db9f 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
@@ -128,15 +128,17 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     HashSet<Path> writableDirs = new HashSet<>();
 
     addPathToSetIfExists(fs, writableDirs, "/dev");
-    addPathToSetIfExists(fs, writableDirs, System.getenv("TMPDIR"));
     addPathToSetIfExists(fs, writableDirs, "/tmp");
     addPathToSetIfExists(fs, writableDirs, "/private/tmp");
     addPathToSetIfExists(fs, writableDirs, "/private/var/tmp");
 
-    // On macOS, in addition to what is specified in $TMPDIR, two other temporary directories may be
-    // written to by processes. We have to get their location by calling "getconf".
+    // On macOS, processes may write to not only $TMPDIR but also to two other temporary
+    // directories. We have to get their location by calling "getconf".
     addPathToSetIfExists(fs, writableDirs, getConfStr("DARWIN_USER_TEMP_DIR"));
     addPathToSetIfExists(fs, writableDirs, getConfStr("DARWIN_USER_CACHE_DIR"));
+    // We don't add any value for $TMPDIR here, instead we compute its value later in
+    // {@link #actuallyExec} and add it as a writable directory in
+    // {@link AbstractSandboxSpawnRunner#getWritableDirs}.
 
     // ~/Library/Cache and ~/Library/Logs need to be writable (cf. issue #2231).
     Path homeDir = fs.getPath(System.getProperty("user.home"));
@@ -173,11 +175,16 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
 
+    // 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> spawnEnvironment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName);
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
 
     final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
-    ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, spawnEnvironment);
+    ImmutableSet<Path> extraWritableDirs =
+        getWritableDirs(sandboxExecRoot, spawnEnvironment, tmpDir);
     writableDirs.addAll(extraWritableDirs);
 
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
@@ -187,7 +194,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     List<String> arguments =
         computeCommandLine(spawn, timeout, sandboxConfigPath, timeoutGraceSeconds);
     Map<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName);
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
 
     boolean allowNetworkForThisSpawn = allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn);
     SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn(
@@ -205,7 +212,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
             sandboxConfigPath, writableDirs, getInaccessiblePaths(), allowNetworkForThisSpawn);
       }
     };
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout);
   }
 
   private List<String> computeCommandLine(
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 fb8a21e884..4246735919 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
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.shell.CommandException;
@@ -86,22 +87,27 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   private final Path linuxSandbox;
   private final Path inaccessibleHelperFile;
   private final Path inaccessibleHelperDir;
+  private final LocalEnvProvider localEnvProvider;
   private final int timeoutGraceSeconds;
+  private final String productName;
 
   LinuxSandboxedSpawnRunner(
       CommandEnvironment cmdEnv,
       Path sandboxBase,
+      String productName,
       Path inaccessibleHelperFile,
       Path inaccessibleHelperDir,
       int timeoutGraceSeconds) {
     super(cmdEnv, sandboxBase);
     this.blazeDirs = cmdEnv.getDirectories();
     this.execRoot = cmdEnv.getExecRoot();
+    this.productName = productName;
     this.allowNetwork = SandboxHelpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.linuxSandbox = getLinuxSandbox(cmdEnv);
     this.inaccessibleHelperFile = inaccessibleHelperFile;
     this.inaccessibleHelperDir = inaccessibleHelperDir;
     this.timeoutGraceSeconds = timeoutGraceSeconds;
+    this.localEnvProvider = LocalEnvProvider.ADD_TEMP_POSIX;
   }
 
   @Override
@@ -111,7 +117,11 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
     Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
 
-    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
+    // 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");
+
+    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), tmpDir);
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
     Duration timeout = policy.getTimeout();
     List<String> arguments =
@@ -124,16 +134,19 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
             getReadOnlyBindMounts(blazeDirs, sandboxExecRoot),
             allowNetwork || SandboxHelpers.shouldAllowNetwork(spawn),
             spawn.getExecutionInfo().containsKey("requires-fakeroot"));
-
-    SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn(
-        sandboxPath,
-        sandboxExecRoot,
-        arguments,
-        spawn.getEnvironment(),
-        SandboxHelpers.getInputFiles(spawn, policy, execRoot),
-        outputs,
-        writableDirs);
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout);
+    Map<String, String> environment =
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
+
+    SandboxedSpawn sandbox =
+        new SymlinkedSandboxedSpawn(
+            sandboxPath,
+            sandboxExecRoot,
+            arguments,
+            environment,
+            SandboxHelpers.getInputFiles(spawn, policy, execRoot),
+            outputs,
+            writableDirs);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout);
   }
 
   private List<String> computeCommandLine(
@@ -214,10 +227,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
   }
 
   @Override
-  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
-      throws IOException {
+  protected ImmutableSet<Path> getWritableDirs(
+      Path sandboxExecRoot, Map<String, String> env, Path tmpDir) throws IOException {
     ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
-    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env));
+    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env, tmpDir));
 
     FileSystem fs = sandboxExecRoot.getFileSystem();
     writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks());
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 2f86fa92d3..7a844317fb 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
@@ -40,10 +40,8 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy {
   }
 
   static LinuxSandboxedSpawnRunner create(
-      CommandEnvironment cmdEnv,
-      Path sandboxBase,
-      int timeoutGraceSeconds)
-          throws IOException {
+      CommandEnvironment cmdEnv, Path sandboxBase, String productName, int timeoutGraceSeconds)
+      throws IOException {
     Path inaccessibleHelperFile = sandboxBase.getRelative("inaccessibleHelperFile");
     FileSystemUtils.touchFile(inaccessibleHelperFile);
     inaccessibleHelperFile.setReadable(false);
@@ -59,6 +57,7 @@ public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy {
     return new LinuxSandboxedSpawnRunner(
         cmdEnv,
         sandboxBase,
+        productName,
         inaccessibleHelperFile,
         inaccessibleHelperDir,
         timeoutGraceSeconds);
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 1ec4a999a8..de5fd8c95d 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
@@ -50,9 +50,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
     this.productName = productName;
     this.timeoutGraceSeconds = timeoutGraceSeconds;
     this.processWrapper = ProcessWrapperRunner.getProcessWrapper(cmdEnv);
-    this.localEnvProvider = OS.getCurrent() == OS.DARWIN
-        ? new XCodeLocalEnvProvider()
-        : LocalEnvProvider.UNMODIFIED;
+    this.localEnvProvider =
+        OS.getCurrent() == OS.DARWIN
+            ? new XCodeLocalEnvProvider()
+            : LocalEnvProvider.ADD_TEMP_POSIX;
   }
 
   @Override
@@ -62,22 +63,28 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne
     Path sandboxPath = getSandboxRoot();
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
 
+    // 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");
+
     Duration timeout = policy.getTimeout();
     List<String> arguments =
         ProcessWrapperRunner.getCommandLine(
             processWrapper, spawn.getArguments(), timeout, timeoutGraceSeconds);
     Map<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName);
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, tmpDir, productName);
+
+    SandboxedSpawn sandbox =
+        new SymlinkedSandboxedSpawn(
+            sandboxPath,
+            sandboxExecRoot,
+            arguments,
+            environment,
+            SandboxHelpers.getInputFiles(spawn, policy, execRoot),
+            SandboxHelpers.getOutputFiles(spawn),
+            getWritableDirs(sandboxExecRoot, spawn.getEnvironment(), tmpDir));
 
-    SandboxedSpawn sandbox = new SymlinkedSandboxedSpawn(
-        sandboxPath,
-        sandboxExecRoot,
-        arguments,
-        environment,
-        SandboxHelpers.getInputFiles(spawn, policy, execRoot),
-        SandboxHelpers.getOutputFiles(spawn),
-        getWritableDirs(sandboxExecRoot, spawn.getEnvironment()));
-    return runSpawn(spawn, sandbox, policy, execRoot, timeout);
+    return runSpawn(spawn, sandbox, policy, execRoot, tmpDir, timeout);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
index f92bceef64..53173e4bea 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
@@ -66,7 +66,8 @@ final class SandboxActionContextProvider extends ActionContextProvider {
     if (LinuxSandboxedSpawnRunner.isSupported(cmdEnv)) {
       SpawnRunner spawnRunner =
           withFallback(
-              cmdEnv, LinuxSandboxedStrategy.create(cmdEnv, sandboxBase, timeoutGraceSeconds));
+              cmdEnv,
+              LinuxSandboxedStrategy.create(cmdEnv, sandboxBase, productName, timeoutGraceSeconds));
       contexts.add(new LinuxSandboxedStrategy(spawnRunner));
     }
 
@@ -90,9 +91,10 @@ final class SandboxActionContextProvider extends ActionContextProvider {
   private static SpawnRunner createFallbackRunner(CommandEnvironment env) {
     LocalExecutionOptions localExecutionOptions =
         env.getOptions().getOptions(LocalExecutionOptions.class);
-    LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN
-        ? new XCodeLocalEnvProvider()
-        : LocalEnvProvider.UNMODIFIED;
+    LocalEnvProvider localEnvProvider =
+        OS.getCurrent() == OS.DARWIN
+            ? new XCodeLocalEnvProvider()
+            : LocalEnvProvider.ADD_TEMP_POSIX;
     return
         new LocalSpawnRunner(
             env.getExecRoot(),
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java
index 040f706592..15afb0c4da 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java
@@ -100,9 +100,12 @@ public class StandaloneActionContextProvider extends ActionContextProvider {
   private static SpawnRunner createLocalRunner(CommandEnvironment env) {
     LocalExecutionOptions localExecutionOptions =
         env.getOptions().getOptions(LocalExecutionOptions.class);
-    LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN
-        ? new XCodeLocalEnvProvider()
-        : LocalEnvProvider.UNMODIFIED;
+    LocalEnvProvider localEnvProvider =
+        OS.getCurrent() == OS.DARWIN
+            ? new XCodeLocalEnvProvider()
+            : (OS.getCurrent() == OS.WINDOWS
+                ? LocalEnvProvider.ADD_TEMP_WINDOWS
+                : LocalEnvProvider.ADD_TEMP_POSIX);
     return
         new LocalSpawnRunner(
             env.getExecRoot(),
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java
index 50f6e2dd8f..e6e9382fc6 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java
@@ -55,7 +55,11 @@ final class WorkerActionContextProvider extends ActionContextProvider {
     LocalExecutionOptions localExecutionOptions =
         env.getOptions().getOptions(LocalExecutionOptions.class);
     LocalEnvProvider localEnvProvider =
-        OS.getCurrent() == OS.DARWIN ? new XCodeLocalEnvProvider() : LocalEnvProvider.UNMODIFIED;
+        OS.getCurrent() == OS.DARWIN
+            ? new XCodeLocalEnvProvider()
+            : (OS.getCurrent() == OS.WINDOWS
+                ? LocalEnvProvider.ADD_TEMP_WINDOWS
+                : LocalEnvProvider.ADD_TEMP_POSIX);
     return new LocalSpawnRunner(
         env.getExecRoot(),
         localExecutionOptions,