Rewrite the Command API

Important: the simplified API now defaults to forwarding interrupts to
subprocesses. I did audit all the call sites, and I think this is a safe change
to make.

- Properly support timeouts with all implementations
- Simplify the API
  - only provide two flavours of blocking calls, which require no input and
    forward interrupts; this is the most common usage
  - provide a number of async calls, which optionally takes input, and a flag
    whether to forward interrupts
  - only support input streams, no byte arrays or other 'convenience features'
    that are rarely needed and unnecessarily increase the surface area
  - use java.time.Duration to specify timeout; for consistency, interpret a
    timeout of <= 0 as no timeout (i.e., including rather than excluding 0)
  - KillableObserver and subclasses are no longer part of the public API, but
    still used to implement timeouts if the Subprocess.Factory does not support
    them
- Update the documentation for Command
- Update all callers; most callers now use the simplified API

PiperOrigin-RevId: 164716782

21 files changed, 339 insertions, 835 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkExecutionResult.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkExecutionResult.java
index 87102ad8a8..d7aff9d625 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkExecutionResult.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkExecutionResult.java
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.shell.BadExitStatusException;
 import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.shell.CommandException;
 import com.google.devtools.build.lib.shell.CommandResult;
-import com.google.devtools.build.lib.shell.TimeoutKillableObserver;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -30,6 +29,7 @@ import com.google.devtools.build.lib.util.io.DelegatingOutErr;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.util.io.RecordingOutErr;
 import java.io.File;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -191,10 +191,9 @@ final class SkylarkExecutionResult {
         for (int i = 0; i < args.size(); i++) {
           argsArray[i] = args.get(i);
         }
-        Command command = new Command(argsArray, envBuilder, directory);
-        CommandResult result = command.execute(
-            new byte[]{}, new TimeoutKillableObserver(timeout),
-            delegator.getOutputStream(), delegator.getErrorStream());
+        Command command = new Command(argsArray, envBuilder, directory, Duration.ofMillis(timeout));
+        CommandResult result =
+            command.execute(delegator.getOutputStream(), delegator.getErrorStream());
         return new SkylarkExecutionResult(
             result.getTerminationStatus().getExitCode(),
             recorder.outAsLatin1(),
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 8744ef0186..57f1082978 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
@@ -115,9 +115,10 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider {
       } else {
         Map<String, String> env = Strings.isNullOrEmpty(developerDir)
             ? ImmutableMap.<String, String>of() : ImmutableMap.of("DEVELOPER_DIR", developerDir);
-        CommandResult xcrunResult = new Command(
-            new String[] {"/usr/bin/xcrun", "--sdk", sdkString, "--show-sdk-path"},
-            env, null).execute();
+        CommandResult xcrunResult =
+            new Command(
+                new String[] {"/usr/bin/xcrun", "--sdk", sdkString, "--show-sdk-path"}, env, null)
+            .execute();
 
         // calling xcrun via Command returns a value with a newline on the end.
         String sdkRoot = new String(xcrunResult.getStdout(), StandardCharsets.UTF_8).trim();
@@ -178,8 +179,9 @@ public final class XCodeLocalEnvProvider implements LocalEnvProvider {
       if (cacheResult != null) {
         return cacheResult;
       } else {
-        CommandResult xcodeLocatorResult = new Command(new String[] {
-            execRoot.getRelative("_bin/xcode-locator").getPathString(), version.toString()})
+        CommandResult xcodeLocatorResult = new Command(
+            new String[] {
+                execRoot.getRelative("_bin/xcode-locator").getPathString(), version.toString()})
             .execute();
 
         String developerDir =
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 b2c9d3c965..548b3b4091 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
@@ -241,6 +241,11 @@ public final class LocalSpawnRunner implements SpawnRunner {
       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());
@@ -259,16 +264,13 @@ public final class LocalSpawnRunner implements SpawnRunner {
             spawn.getArguments().toArray(new String[0]),
             localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName),
             execRoot.getPathFile(),
-            // TODO(ulfjack): Command throws if timeouts are unsupported and timeout >= 0. For
-            // consistency, we should change it to not throw (and not enforce a timeout) if
-            // timeout <= 0 instead.
-            policy.getTimeout().isZero() ? -1 : policy.getTimeout().toMillis());
+            policy.getTimeout());
       }
 
       long startTime = System.currentTimeMillis();
       CommandResult result;
       try {
-        result = cmd.execute(Command.NO_INPUT, Command.NO_OBSERVER, stdOut, stdErr, true);
+        result = cmd.execute(stdOut, stdErr);
         if (Thread.currentThread().isInterrupted()) {
           throw new InterruptedException();
         }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 5651aa987c..57b8f1dc20 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -322,14 +322,13 @@ public class RunCommand implements BlazeCommand  {
       // actual output of the command being run even if --color=no is specified.
       env.getReporter().switchToAnsiAllowingHandler();
 
-      // The command API is a little strange in that the following statement
-      // will return normally only if the program exits with exit code 0.
-      // If it ends with any other code, we have to catch BadExitStatusException.
-      command.execute(com.google.devtools.build.lib.shell.Command.NO_INPUT,
-          com.google.devtools.build.lib.shell.Command.NO_OBSERVER,
-          outErr.getOutputStream(),
-          outErr.getErrorStream(),
-          true /* interruptible */).getTerminationStatus().getExitCode();
+      // The command API is a little strange in that the following statement will return normally
+      // only if the program exits with exit code 0. If it ends with any other code, we have to
+      // catch BadExitStatusException.
+      command
+          .execute(outErr.getOutputStream(), outErr.getErrorStream())
+          .getTerminationStatus()
+          .getExitCode();
       return ExitCode.SUCCESS;
     } catch (BadExitStatusException e) {
       String message = "Non-zero return code '"
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
index ab909d025f..8a83400c63 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java
@@ -264,16 +264,11 @@ public class MobileInstallCommand implements BlazeCommand {
       // actual output of the command being run even if --color=no is specified.
       env.getReporter().switchToAnsiAllowingHandler();
 
-      // The command API is a little strange in that the following statement
-      // will return normally only if the program exits with exit code 0.
-      // If it ends with any other code, we have to catch BadExitStatusException.
+      // The command API is a little strange in that the following statement will return normally
+      // only if the program exits with exit code 0. If it ends with any other code, we have to
+      // catch BadExitStatusException.
       command
-          .execute(
-              com.google.devtools.build.lib.shell.Command.NO_INPUT,
-              com.google.devtools.build.lib.shell.Command.NO_OBSERVER,
-              outErr.getOutputStream(),
-              outErr.getErrorStream(),
-              true /* interruptible */)
+          .execute(outErr.getOutputStream(), outErr.getErrorStream())
           .getTerminationStatus()
           .getExitCode();
       return ExitCode.SUCCESS;
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 7046a7bf7b..5e2c42d7f3 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
@@ -131,12 +131,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner {
     long startTime = System.currentTimeMillis();
     CommandResult result;
     try {
-      result = cmd.execute(
-          /* stdin */ new byte[] {},
-          Command.NO_OBSERVER,
-          outErr.getOutputStream(),
-          outErr.getErrorStream(),
-          /* killSubprocessOnInterrupt */ true);
+      result = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException();
       }
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 e34b04ccd3..c021356d54 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
@@ -73,12 +73,7 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
     Command cmd = new Command(args.toArray(new String[0]), env, cwd);
     try {
-      cmd.execute(
-          /* stdin */ new byte[] {},
-          Command.NO_OBSERVER,
-          ByteStreams.nullOutputStream(),
-          ByteStreams.nullOutputStream(),
-          /* killSubprocessOnInterrupt */ true);
+      cmd.execute(ByteStreams.nullOutputStream(), ByteStreams.nullOutputStream());
     } catch (CommandException e) {
       return false;
     }
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 ecaf2bb842..9e80c56d90 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
@@ -67,12 +67,7 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
     Command cmd = new Command(args.toArray(new String[0]), env, cwd);
     try {
-      cmd.execute(
-          /* stdin */ new byte[] {},
-          Command.NO_OBSERVER,
-          ByteStreams.nullOutputStream(),
-          ByteStreams.nullOutputStream(),
-          /* killSubprocessOnInterrupt */ true);
+      cmd.execute(ByteStreams.nullOutputStream(), ByteStreams.nullOutputStream());
     } catch (CommandException e) {
       return false;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Command.java b/src/main/java/com/google/devtools/build/lib/shell/Command.java
index 2e13636283..f33f53d14a 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Command.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Command.java
@@ -14,27 +14,34 @@
 
 package com.google.devtools.build.lib.shell;
 
-
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
+import com.google.common.io.ByteStreams;
+import com.google.devtools.build.lib.shell.Consumers.OutErrConsumers;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.time.Duration;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 
 /**
- * <p>Represents an executable command, including its arguments and
- * runtime environment (environment variables, working directory). This class
- * lets a caller execute a command, get its results, and optionally try to kill
- * the task during execution.</p>
+ * An executable command, including its arguments and runtime environment (environment variables,
+ * working directory). It lets a caller execute a command, get its results, and optionally forward
+ * interrupts to the subprocess. This class creates threads to ensure timely reading of subprocess
+ * outputs.
+ *
+ * <p>This class is immutable and thread-safe.
+ *
+ * <p>The use of "shell" in the package name of this class is a misnomer.  In terms of the way its
+ * arguments are interpreted, this class is closer to {@code execve(2)} than to {@code system(3)}.
+ * No shell is executed.
  *
- * <p>The use of "shell" in the full name of this class is a misnomer.  In
- * terms of the way its arguments are interpreted, this class is closer to
- * {@code execve(2)} than to {@code system(3)}.  No Bourne shell is executed.
+ * <h4>Examples</h4>
  *
  * <p>The most basic use-case for this class is as follows:
  * <pre>
@@ -42,18 +49,30 @@ import java.util.logging.Logger;
  *   CommandResult result = new Command(args).execute();
  *   String output = new String(result.getStdout());
  * </pre>
- * which writes the output of the {@code du(1)} command into {@code output}.
- * More complex cases might inspect the stderr stream, kill the subprocess
- * asynchronously, feed input to its standard input, handle the exceptions
- * thrown if the command fails, or print the termination status (exit code or
- * signal name).
+ * which writes the output of the {@code du(1)} command into {@code output}. More complex cases
+ * might inspect the stderr stream, kill the subprocess asynchronously, feed input to its standard
+ * input, handle the exceptions thrown if the command fails, or print the termination status (exit
+ * code or signal name).
+ *
+ * <h4>Other Features</h4>
+ *
+ * <p>A caller can optionally specify bytes to be written to the process's "stdin". The returned
+ * {@link CommandResult} object gives the caller access to the exit status, as well as output from
+ * "stdout" and "stderr". To use this class with processes that generate very large amounts of
+ * input/output, consider {@link #execute(OutputStream, OutputStream)},
+ * {@link #executeAsync(OutputStream, OutputStream)}, or
+ * {@link #executeAsync(InputStream, OutputStream, OutputStream, boolean)}.
  *
- * <h4>Invoking the Bourne shell</h4>
+ * <p>This class ensures that stdout and stderr streams are read promptly, avoiding potential
+ * deadlock if the output is large. See
+ * <a href="http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html"> when
+ * <code>Runtime.exec()</code> won't</a>.
  *
- * <p>Perhaps the most common command invoked programmatically is the UNIX
- * shell, {@code /bin/sh}.  Because the shell is a general-purpose programming
- * language, care must be taken to ensure that variable parts of the shell
- * command (e.g. strings entered by the user) do not contain shell
+ * <h4>Caution: Invoking Shell Commands</h4>
+ *
+ * <p>Perhaps the most common command invoked programmatically is the UNIX shell, {@code /bin/sh}.
+ * Because the shell is a general-purpose programming language, care must be taken to ensure that
+ * variable parts of the shell command (e.g. strings entered by the user) do not contain shell
  * metacharacters, as this poses a correctness and/or security risk.
  *
  * <p>To execute a shell command directly, use the following pattern:
@@ -61,76 +80,49 @@ import java.util.logging.Logger;
  *   String[] args = { "/bin/sh", "-c", shellCommand };
  *   CommandResult result = new Command(args).execute();
  * </pre>
- * {@code shellCommand} is a complete Bourne shell program, possibly containing
- * all kinds of unescaped metacharacters.  For example, here's a shell command
- * that enumerates the working directories of all processes named "foo":
+ * {@code shellCommand} is a complete Bourne shell program, possibly containing all kinds of
+ * unescaped metacharacters.  For example, here's a shell command that enumerates the working
+ * directories of all processes named "foo":
  * <pre>ps auxx | grep foo | awk '{print $1}' |
  *      while read pid; do readlink /proc/$pid/cwd; done</pre>
- * It is the responsibility of the caller to ensure that this string means what
- * they intend.
+ * It is the responsibility of the caller to ensure that this string means what they intend.
  *
- * <p>Consider the risk posed by allowing the "foo" part of the previous
- * command to be some arbitrary (untrusted) string called {@code processName}:
+ * <p>Consider the risk posed by allowing the "foo" part of the previous command to be some
+ * arbitrary (untrusted) string called {@code processName}:
  * <pre>
  *  // WARNING: unsafe!
  *  String shellCommand = "ps auxx | grep " + processName + " | awk '{print $1}' | "
  *  + "while read pid; do readlink /proc/$pid/cwd; done";</pre>
  * </pre>
- * Passing this string to {@link Command} is unsafe because if the string
- * {@processName} contains shell metacharacters, the meaning of the command can
- * be arbitrarily changed;  consider:
+ * Passing this string to {@link Command} is unsafe because if the string {@processName} contains
+ * shell metacharacters, the meaning of the command can be arbitrarily changed; consider:
  * <pre>String processName = ". ; rm -fr $HOME & ";</pre>
  *
- * <p>To defend against this possibility, it is essential to properly quote the
- * variable portions of the shell command so that shell metacharacters are
- * escaped.  Use {@link ShellUtils#shellEscape} for this purpose:
+ * <p>To defend against this possibility, it is essential to properly quote the variable portions of
+ * the shell command so that shell metacharacters are escaped.  Use {@link ShellUtils#shellEscape}
+ * for this purpose:
  * <pre>
  *  // Safe.
  *  String shellCommand = "ps auxx | grep " + ShellUtils.shellEscape(processName)
  *      + " | awk '{print $1}' | while read pid; do readlink /proc/$pid/cwd; done";
  * </pre>
  *
- * <p>Tip: if you are only invoking a single known command, and no shell
- * features (e.g. $PATH lookup, output redirection, pipelines, etc) are needed,
- * call it directly without using a shell, as in the {@code du(1)} example
- * above.
- *
- * <h4>Other features</h4>
- *
- * <p>A caller can optionally specify bytes to be written to the process's
- * "stdin". The returned {@link CommandResult} object gives the caller access to
- * the exit status, as well as output from "stdout" and "stderr". To use
- * this class with processes that generate very large amounts of input/output,
- * consider
- * {@link #execute(InputStream, KillableObserver, OutputStream, OutputStream)}
- * and
- * {@link #execute(byte[], KillableObserver, OutputStream, OutputStream)}.
- * </p>
- *
- * <p>This class ensures that stdout and stderr streams are read promptly,
- * avoiding potential deadlock if the output is large. See <a
- * href="http://www.javaworld.com/javaworld/jw-12-2000/jw-1229-traps.html"> When
- * <code>Runtime.exec()</code> won't</a>.</p>
- *
- * <p>This class is immutable and therefore thread-safe.</p>
+ * <p>Tip: if you are only invoking a single known command, and no shell features (e.g. $PATH
+ * lookup, output redirection, pipelines, etc) are needed, call it directly without using a shell,
+ * as in the {@code du(1)} example above.
  */
 public final class Command {
 
   private static final Logger log =
     Logger.getLogger("com.google.devtools.build.lib.shell.Command");
 
-  /**
-   * Pass this value to {@link #execute(byte[])} to indicate that no input
-   * should be written to stdin.
-   */
-  public static final byte[] NO_INPUT = new byte[0];
+  /** Pass this value to {@link #execute} to indicate that no input should be written to stdin. */
+  public static final InputStream NO_INPUT = new NullInputStream();
 
-  /**
-   * Pass this to {@link #execute(byte[], KillableObserver, boolean)} to
-   * indicate that you do not wish to observe / kill the underlying
-   * process.
-   */
-  public static final KillableObserver NO_OBSERVER = new KillableObserver() {
+  public static final boolean KILL_SUBPROCESS_ON_INTERRUPT = true;
+  public static final boolean CONTINUE_SUBPROCESS_ON_INTERRUPT = false;
+
+  private static final KillableObserver NO_OBSERVER = new KillableObserver() {
     @Override
     public void startObserving(final Killable killable) {
       // do nothing
@@ -143,43 +135,38 @@ public final class Command {
 
   private final SubprocessBuilder subprocessBuilder;
 
-  // Start of public API -----------------------------------------------------
-
-  /**
   /**
-   * Creates a new {@link Command} for the given command line elements. The
-   * command line is executed exactly as given, without a shell.
-   * Subsequent calls to {@link #execute()} will use the JVM's working
-   * directory and environment.
+   * Creates a new {@link Command} for the given command line. The environment is inherited from the
+   * current process, as is the working directory. No timeout is enforced. The command line is
+   * executed exactly as given, without a shell. Subsequent calls to {@link #execute()} will use the
+   * JVM's working directory and environment.
    *
    * @param commandLineElements elements of raw command line to execute
    * @throws IllegalArgumentException if commandLine is null or empty
    */
-  /* TODO(bazel-team): Use varargs here
-   */
-  public Command(final String[] commandLineElements) {
-    this(commandLineElements, null, null);
+  public Command(String[] commandLineElements) {
+    this(commandLineElements, null, null, Duration.ZERO);
   }
 
   /**
-   * Just like {@link Command(String, Map<String, String>, File, long), but without a timeout.
+   * Just like {@link #Command(String[], Map, File, Duration)}, but without a timeout.
    */
   public Command(
       String[] commandLineElements,
-      Map<String, String> environmentVariables,
-      File workingDirectory) {
-    this(commandLineElements, environmentVariables, workingDirectory, -1);
+      @Nullable Map<String, String> environmentVariables,
+      @Nullable File workingDirectory) {
+    this(commandLineElements, environmentVariables, workingDirectory, Duration.ZERO);
   }
 
   /**
-   * Creates a new {@link Command} for the given command line elements. The
-   * command line is executed without a shell.
+   * Creates a new {@link Command} for the given command line elements. The command line is executed
+   * without a shell.
    *
-   * The given environment variables and working directory are used in subsequent
-   * calls to {@link #execute()}.
+   * <p>The given environment variables and working directory are used in subsequent calls to
+   * {@link #execute()}.
    *
-   * This command treats the  0-th element of {@code commandLineElement}
-   * (the name of an executable to run) specially.
+   * <p>This command treats the  0-th element of {@code commandLineElement} (the name of an
+   * executable to run) specially.
    * <ul>
    *  <li>If it is an absolute path, it is used as it</li>
    *  <li>If it is a single file name, the PATH lookup is performed</li>
@@ -188,21 +175,22 @@ public final class Command {
    * </ul>
    *
    * @param commandLineElements elements of raw command line to execute
-   * @param environmentVariables environment variables to replace JVM's
-   *    environment variables; may be null
-   * @param workingDirectory working directory for execution; if null, current
-   *    working directory is used
-   * @param timeoutMillis timeout in milliseconds. Only supported on Windows.
+   * @param environmentVariables environment variables to replace JVM's environment variables; may
+   *    be null
+   * @param workingDirectory working directory for execution; if null, the VM's current working
+   *    directory is used
+   * @param timeout timeout; a value less than or equal to 0 is treated as no timeout
    * @throws IllegalArgumentException if commandLine is null or empty
    */
+  // TODO(ulfjack): Throw a special exception if there was a timeout.
   public Command(
       String[] commandLineElements,
-      Map<String, String> environmentVariables,
-      File workingDirectory,
-      long timeoutMillis) {
-    if (commandLineElements == null || commandLineElements.length == 0) {
-      throw new IllegalArgumentException("command line is null or empty");
-    }
+      @Nullable Map<String, String> environmentVariables,
+      @Nullable File workingDirectory,
+      Duration timeout) {
+    Preconditions.checkNotNull(commandLineElements);
+    Preconditions.checkArgument(
+        commandLineElements.length != 0, "cannot run an empty command line");
 
     File executable = new File(commandLineElements[0]);
     if (!executable.isAbsolute() && executable.getParent() != null) {
@@ -214,512 +202,199 @@ public final class Command {
     subprocessBuilder.setArgv(ImmutableList.copyOf(commandLineElements));
     subprocessBuilder.setEnv(environmentVariables);
     subprocessBuilder.setWorkingDirectory(workingDirectory);
-    subprocessBuilder.setTimeoutMillis(timeoutMillis);
+    subprocessBuilder.setTimeoutMillis(timeout.toMillis());
   }
 
-  /**
-   * @return raw command line elements to be executed
-   */
+  /** Returns the raw command line elements to be executed */
   public String[] getCommandLineElements() {
     final List<String> elements = subprocessBuilder.getArgv();
     return elements.toArray(new String[elements.size()]);
   }
 
-  /**
-   * @return (unmodifiable) {@link Map} view of command's environment variables
-   */
-  public Map<String, String> getEnvironmentVariables() {
+  /** Returns an (unmodifiable) {@link Map} view of command's environment variables or null. */
+  @Nullable public Map<String, String> getEnvironmentVariables() {
     return subprocessBuilder.getEnv();
   }
 
-  /**
-   * @return working directory used for execution, or null if the current
-   *         working directory is used
-   */
-  public File getWorkingDirectory() {
+  /** Returns the working directory to be used for execution, or null. */
+  @Nullable public File getWorkingDirectory() {
     return subprocessBuilder.getWorkingDirectory();
   }
 
   /**
-   * Execute this command with no input to stdin. This call will block until the
-   * process completes or an error occurs.
+   * Execute this command with no input to stdin, and with the output captured in memory. If the
+   * current process is interrupted, then the subprocess is also interrupted. This call blocks until
+   * the subprocess completes or an error occurs.
+   *
+   * <p>This method is a convenience wrapper for <code>executeAsync().get()</code>.
    *
    * @return {@link CommandResult} representing result of the execution
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if an {@link IOException} is
-   *  encountered while reading from the process, or the process was terminated
-   *  due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
+   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
+   * @throws BadExitStatusException if the process exits with a non-zero status
    */
   public CommandResult execute() throws CommandException {
-    return execute(NO_INPUT);
+    return executeAsync().get();
   }
 
   /**
-   * Execute this command with given input to stdin. This call will block until
-   * the process completes or an error occurs.
+   * Execute this command with no input to stdin, and with the output streamed to the given output
+   * streams, which must be thread-safe. If the current process is interrupted, then the subprocess
+   * is also interrupted. This call blocks until the subprocess completes or an error occurs.
    *
-   * @param stdinInput bytes to be written to process's stdin
-   * @return {@link CommandResult} representing result of the execution
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if an {@link IOException} is
-   *  encountered while reading from the process, or the process was terminated
-   *  due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
-   * @throws NullPointerException if stdin is null
-   */
-  public CommandResult execute(final byte[] stdinInput)
-    throws CommandException {
-    nullCheck(stdinInput, "stdinInput");
-    return doExecute(new ByteArrayInputSource(stdinInput),
-                     NO_OBSERVER,
-                     Consumers.createAccumulatingConsumers(),
-                     /*killSubprocess=*/false, /*closeOutput=*/false).get();
-  }
-
-  /**
-   * <p>Execute this command with given input to stdin. This call will block
-   * until the process completes or an error occurs. Caller may specify
-   * whether the method should ignore stdout/stderr output. If the
-   * given number of milliseconds elapses before the command has
-   * completed, this method will attempt to kill the command.</p>
+   * <p>Note that the given output streams are never closed by this class.
    *
-   * @param stdinInput bytes to be written to process's stdin, or
-   * {@link #NO_INPUT} if no bytes should be written
-   * @param timeout number of milliseconds to wait for command completion
-   *  before attempting to kill the command
-   * @param ignoreOutput if true, method will ignore stdout/stderr output
-   *  and return value will not contain this data
-   * @return {@link CommandResult} representing result of the execution
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if an {@link IOException} is
-   *  encountered while reading from the process, or the process was terminated
-   *  due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
-   * @throws NullPointerException if stdin is null
-   */
-  public CommandResult execute(final byte[] stdinInput,
-                               final long timeout,
-                               final boolean ignoreOutput)
-    throws CommandException {
-    return execute(stdinInput,
-                   new TimeoutKillableObserver(timeout),
-                   ignoreOutput);
-  }
-
-  /**
-   * <p>Execute this command with given input to stdin. This call will block
-   * until the process completes or an error occurs. Caller may specify
-   * whether the method should ignore stdout/stderr output. The given {@link
-   * KillableObserver} may also terminate the process early while running.</p>
+   * <p>This method is a convenience wrapper for <code>executeAsync(stdOut, stdErr).get()</code>.
    *
-   * @param stdinInput bytes to be written to process's stdin, or
-   *  {@link #NO_INPUT} if no bytes should be written
-   * @param observer {@link KillableObserver} that should observe the running
-   *  process, or {@link #NO_OBSERVER} if caller does not wish to kill
-   *  the process
-   * @param ignoreOutput if true, method will ignore stdout/stderr output
-   *  and return value will not contain this data
    * @return {@link CommandResult} representing result of the execution
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if the process is interrupted (or
-   *  killed) before completion, if an {@link IOException} is encountered while
-   *  reading from the process, or the process was terminated due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
-   * @throws NullPointerException if stdin is null
+   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
+   * @throws BadExitStatusException if the process exits with a non-zero status
    */
-  public CommandResult execute(final byte[] stdinInput,
-                               final KillableObserver observer,
-                               final boolean ignoreOutput)
-    throws CommandException {
-    // supporting "null" here for backwards compatibility
-    final KillableObserver theObserver =
-      observer == null ? NO_OBSERVER : observer;
-    return doExecute(new ByteArrayInputSource(stdinInput),
-                     theObserver,
-                     ignoreOutput ? Consumers.createDiscardingConsumers()
-                                  : Consumers.createAccumulatingConsumers(),
-                     /*killSubprocess=*/false, /*closeOutput=*/false).get();
+  public CommandResult execute(OutputStream stdOut, OutputStream stdErr) throws CommandException {
+    return doExecute(
+        NO_INPUT, Consumers.createStreamingConsumers(stdOut, stdErr), KILL_SUBPROCESS_ON_INTERRUPT)
+            .get();
   }
 
   /**
-   * <p>Execute this command with given input to stdin. This call blocks
-   * until the process completes or an error occurs. The caller provides
-   * {@link OutputStream} instances into which the process writes its
-   * stdout/stderr output; these streams are <em>not</em> closed when the
-   * process terminates. The given {@link KillableObserver} may also
-   * terminate the process early while running.</p>
-   *
-   * <p>Note that stdout and stderr are written concurrently. If these are
-   * aliased to each other, it is the caller's duty to ensure thread safety.
-   * </p>
+   * Execute this command with no input to stdin, and with the output captured in memory. If the
+   * current process is interrupted, then the subprocess is also interrupted. This call blocks until
+   * the subprocess is started or throws an error if that fails, but does not wait for the
+   * subprocess to exit.
    *
-   * @param stdinInput bytes to be written to process's stdin, or
-   * {@link #NO_INPUT} if no bytes should be written
-   * @param observer {@link KillableObserver} that should observe the running
-   *  process, or {@link #NO_OBSERVER} if caller does not wish to kill the
-   *  process
-   * @param stdOut the process will write its standard output into this stream.
-   *  E.g., you could pass {@link System#out} as <code>stdOut</code>.
-   * @param stdErr the process will write its standard error into this stream.
-   *  E.g., you could pass {@link System#err} as <code>stdErr</code>.
-   * @return {@link CommandResult} representing result of the execution. Note
-   *  that {@link CommandResult#getStdout()} and
-   *  {@link CommandResult#getStderr()} will yield {@link IllegalStateException}
-   *  in this case, as the output is written to <code>stdOut/stdErr</code>
-   *  instead.
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if the process is interrupted (or
-   *  killed) before completion, if an {@link IOException} is encountered while
-   *  reading from the process, or the process was terminated due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
-   * @throws NullPointerException if any argument is null.
+   * @return {@link CommandResult} representing result of the execution
+   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
+   * @throws BadExitStatusException if the process exits with a non-zero status
    */
-  public CommandResult execute(final byte[] stdinInput,
-                               final KillableObserver observer,
-                               final OutputStream stdOut,
-                               final OutputStream stdErr)
-    throws CommandException {
-    return execute(stdinInput, observer, stdOut, stdErr, false);
+  public FutureCommandResult executeAsync() throws CommandException {
+    return doExecute(
+        NO_INPUT, Consumers.createAccumulatingConsumers(), KILL_SUBPROCESS_ON_INTERRUPT);
   }
 
   /**
-   * Like {@link #execute(byte[], KillableObserver, OutputStream, OutputStream)}
-   * but enables setting of the killSubprocessOnInterrupt attribute.
+   * Execute this command with no input to stdin, and with the output streamed to the given output
+   * streams, which must be thread-safe. If the current process is interrupted, then the subprocess
+   * is also interrupted. This call blocks until the subprocess is started or throws an error if
+   * that fails, but does not wait for the subprocess to exit.
    *
-   * @param killSubprocessOnInterrupt if set to true, the execution of
-   * this command is <i>interruptible</i>: in other words, if this thread is
-   * interrupted during a call to execute, the subprocess will be terminated
-   * and the call will return in a timely manner.  If false, the subprocess
-   * will run to completion; this is the default value use by all other
-   * constructors.  The thread's interrupted status is preserved in all cases,
-   * however.
-   */
-  public CommandResult execute(final byte[] stdinInput,
-                               final KillableObserver observer,
-                               final OutputStream stdOut,
-                               final OutputStream stdErr,
-                               final boolean killSubprocessOnInterrupt)
-    throws CommandException {
-    nullCheck(stdinInput, "stdinInput");
-    nullCheck(observer, "observer");
-    nullCheck(stdOut, "stdOut");
-    nullCheck(stdErr, "stdErr");
-    return doExecute(new ByteArrayInputSource(stdinInput),
-                     observer,
-                     Consumers.createStreamingConsumers(stdOut, stdErr),
-                     killSubprocessOnInterrupt, false).get();
-  }
-
-  /**
-   * Execute this command with given input to stdin; this stream is closed when the process
-   * terminates, and exceptions raised when closing this stream are ignored. This call blocks until
-   * the process completes or an error occurs. The caller provides {@link OutputStream} instances
-   * into which the process writes its stdout/stderr output; these streams are <em>not</em> closed
-   * when the process terminates. The given {@link KillableObserver} may also terminate the process
-   * early while running.
+   * <p>Note that the given output streams are never closed by this class.
    *
-   * <p>If stdOut or stdErr is {@code null}, it will be redirected to /dev/null.
+   * @return {@link CommandResult} representing result of the execution
+   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
+   * @throws BadExitStatusException if the process exits with a non-zero status
    */
-  public CommandResult execute(
-      final byte[] stdinInput,
-      final KillableObserver observer,
-      final File stdOut,
-      final File stdErr,
-      final boolean killSubprocessOnInterrupt)
+  public FutureCommandResult executeAsync(OutputStream stdOut, OutputStream stdErr)
       throws CommandException {
-    nullCheck(stdinInput, "stdinInput");
-    nullCheck(observer, "observer");
-    if (stdOut == null) {
-      subprocessBuilder.setStdout(StreamAction.DISCARD);
-    } else {
-      subprocessBuilder.setStdout(stdOut);
-    }
-
-    if (stdErr == null) {
-      subprocessBuilder.setStderr(StreamAction.DISCARD);
-    } else {
-      subprocessBuilder.setStderr(stdErr);
-    }
     return doExecute(
-            new ByteArrayInputSource(stdinInput), observer, null, killSubprocessOnInterrupt, false)
-        .get();
+        NO_INPUT, Consumers.createStreamingConsumers(stdOut, stdErr), KILL_SUBPROCESS_ON_INTERRUPT);
   }
 
   /**
-   * Execute this command with given input to stdin; this stream is closed when the process
-   * terminates, and exceptions raised when closing this stream are ignored. This call blocks until
-   * the process completes or an error occurs. The caller provides {@link OutputStream} instances
-   * into which the process writes its stdout/stderr output; these streams are <em>not</em> closed
-   * when the process terminates. The given {@link KillableObserver} may also terminate the process
-   * early while running.
+   * Execute this command with no input to stdin, and with the output captured in memory. This call
+   * blocks until the subprocess is started or throws an error if that fails, but does not wait for
+   * the subprocess to exit.
    *
-   * @param stdinInput The input to this process's stdin
-   * @param observer {@link KillableObserver} that should observe the running process, or {@link
-   *     #NO_OBSERVER} if caller does not wish to kill the process
-   * @param stdOut the process will write its standard output into this stream. E.g., you could pass
-   *     {@link System#out} as <code>stdOut</code>.
-   * @param stdErr the process will write its standard error into this stream. E.g., you could pass
-   *     {@link System#err} as <code>stdErr</code>.
-   * @return {@link CommandResult} representing result of the execution. Note that {@link
-   *     CommandResult#getStdout()} and {@link CommandResult#getStderr()} will yield {@link
-   *     IllegalStateException} in this case, as the output is written to <code>stdOut/stdErr</code>
-   *     instead.
+   * @param killSubprocessOnInterrupt whether the subprocess should be killed if the current process
+   *     is interrupted
+   * @return {@link CommandResult} representing result of the execution
    * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
-   * @throws AbnormalTerminationException if the process is interrupted (or killed) before
-   *     completion, if an {@link IOException} is encountered while reading from the process, or the
-   *     process was terminated due to a signal.
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
    * @throws BadExitStatusException if the process exits with a non-zero status
-   * @throws NullPointerException if any argument is null.
    */
-  public CommandResult execute(
-      final InputStream stdinInput,
-      final KillableObserver observer,
-      final OutputStream stdOut,
-      final OutputStream stdErr)
-      throws CommandException {
-    nullCheck(stdinInput, "stdinInput");
-    nullCheck(observer, "observer");
-    nullCheck(stdOut, "stdOut");
-    nullCheck(stdErr, "stdErr");
-    return doExecute(new InputStreamInputSource(stdinInput),
-                     observer,
-                     Consumers.createStreamingConsumers(stdOut, stdErr),
-                     /*killSubprocess=*/false, /*closeOutput=*/false).get();
+  public FutureCommandResult executeAsync(
+      InputStream stdinInput, boolean killSubprocessOnInterrupt) throws CommandException {
+    return doExecute(
+        stdinInput, Consumers.createAccumulatingConsumers(), killSubprocessOnInterrupt);
   }
 
   /**
-   * <p>Execute this command with given input to stdin; this stream is closed
-   * when the process terminates, and exceptions raised when closing this
-   * stream are ignored. This call blocks
-   * until the process completes or an error occurs. The caller provides
-   * {@link OutputStream} instances into which the process writes its
-   * stdout/stderr output; these streams are closed when the process terminates
-   * if closeOut is set. The given {@link KillableObserver} may also
-   * terminate the process early while running.</p>
+   * Execute this command with no input to stdin, and with the output streamed to the given output
+   * streams, which must be thread-safe. This call blocks until the subprocess is started or throws
+   * an error if that fails, but does not wait for the subprocess to exit.
    *
-   * @param stdinInput The input to this process's stdin
-   * @param observer {@link KillableObserver} that should observe the running
-   *  process, or {@link #NO_OBSERVER} if caller does not wish to kill the
-   *  process
-   * @param stdOut the process will write its standard output into this stream.
-   *  E.g., you could pass {@link System#out} as <code>stdOut</code>.
-   * @param stdErr the process will write its standard error into this stream.
-   *  E.g., you could pass {@link System#err} as <code>stdErr</code>.
-   * @param closeOut whether to close the output streams when the subprocess
-   *  terminates.
-   * @return {@link CommandResult} representing result of the execution. Note
-   *  that {@link CommandResult#getStdout()} and
-   *  {@link CommandResult#getStderr()} will yield {@link IllegalStateException}
-   *  in this case, as the output is written to <code>stdOut/stdErr</code>
-   *  instead.
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws AbnormalTerminationException if the process is interrupted (or
-   *  killed) before completion, if an {@link IOException} is encountered while
-   *  reading from the process, or the process was terminated due to a signal.
-   * @throws BadExitStatusException if the process exits with a
-   *  non-zero status
-   * @throws NullPointerException if any argument is null.
-   */
-  public CommandResult execute(final InputStream stdinInput,
-      final KillableObserver observer,
-      final OutputStream stdOut,
-      final OutputStream stdErr,
-      boolean closeOut)
-      throws CommandException {
-    nullCheck(stdinInput, "stdinInput");
-    nullCheck(observer, "observer");
-    nullCheck(stdOut, "stdOut");
-    nullCheck(stdErr, "stdErr");
-    return doExecute(new InputStreamInputSource(stdinInput),
-        observer,
-        Consumers.createStreamingConsumers(stdOut, stdErr),
-        false, closeOut).get();
-  }
-
-  /**
-   * Executes this command with the given stdinInput, but does not wait for it to complete. The
-   * caller may choose to observe the status of the launched process by calling methods on the
-   * returned object.
+   * <p>Note that the given output streams are never closed by this class.
    *
-   * @param stdinInput bytes to be written to process's stdin, or {@link #NO_INPUT} if no bytes
-   *     should be written
-   * @return An object that can be used to check if the process terminated and obtain the process
-   *     results.
+   * @param killSubprocessOnInterrupt whether the subprocess should be killed if the current process
+   *     is interrupted
+   * @return {@link CommandResult} representing result of the execution
    * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any reason
-   * @throws NullPointerException if stdin is null
+   * @throws AbnormalTerminationException if an {@link IOException} is encountered while reading
+   *    from the process, or the process was terminated due to a signal
+   * @throws BadExitStatusException if the process exits with a non-zero status
    */
-  public FutureCommandResult executeAsynchronously(final byte[] stdinInput)
-      throws CommandException {
-    return executeAsynchronously(stdinInput, NO_OBSERVER);
+  public FutureCommandResult executeAsync(
+      InputStream stdinInput,
+      OutputStream stdOut,
+      OutputStream stdErr,
+      boolean killSubprocessOnInterrupt)
+          throws CommandException {
+    return doExecute(
+        stdinInput, Consumers.createStreamingConsumers(stdOut, stdErr), killSubprocessOnInterrupt);
   }
 
   /**
-   * <p>Executes this command with the given input to stdin, but does
-   * not wait for it to complete. The caller may choose to observe the
-   * status of the launched process by calling methods on the returned
-   * object.  This method performs the minimum cleanup after the
-   * process terminates: It closes the input stream, and it ignores
-   * exceptions that result from closing it. The given {@link
-   * KillableObserver} may also terminate the process early while
-   * running.</p>
-   *
-   * <p>Note that in this case the {@link KillableObserver} will be assigned
-   * to start observing the process via
-   * {@link KillableObserver#startObserving(Killable)} but will only be
-   * unassigned via {@link KillableObserver#stopObserving(Killable)}, if
-   * {@link FutureCommandResult#get()} is called. If the
-   * {@link KillableObserver} implementation used with this method will
-   * not work correctly without calls to
-   * {@link KillableObserver#stopObserving(Killable)} then a new instance
-   * should be used for each call to this method.</p>
-   *
-   * @param stdinInput bytes to be written to process's stdin, or
-   * {@link #NO_INPUT} if no bytes should be written
-   * @param observer {@link KillableObserver} that should observe the running
-   *  process, or {@link #NO_OBSERVER} if caller does not wish to kill
-   *  the process
-   * @return An object that can be used to check if the process terminated and
-   *  obtain the process results.
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws NullPointerException if stdin is null
+   * A string representation of this command object which includes the arguments, the environment,
+   * and the working directory. Avoid relying on the specifics of this format. Note that the size of
+   * the result string will reflect the size of the command.
    */
-  public FutureCommandResult executeAsynchronously(final byte[] stdinInput,
-                                    final KillableObserver observer)
-    throws CommandException {
-    // supporting "null" here for backwards compatibility
-    final KillableObserver theObserver =
-      observer == null ? NO_OBSERVER : observer;
-    nullCheck(stdinInput, "stdinInput");
-    return doExecute(new ByteArrayInputSource(stdinInput),
-        theObserver,
-        Consumers.createDiscardingConsumers(),
-        /*killSubprocess=*/false, /*closeOutput=*/false);
-  }
-
-  /**
-   * <p>Executes this command with the given input to stdin, but does
-   * not wait for it to complete. The caller may choose to observe the
-   * status of the launched process by calling methods on the returned
-   * object.  This method performs the minimum cleanup after the
-   * process terminates: It closes the input stream, and it ignores
-   * exceptions that result from closing it. The caller provides
-   * {@link OutputStream} instances into which the process writes its
-   * stdout/stderr output; these streams are <em>not</em> closed when
-   * the process terminates. The given {@link KillableObserver} may
-   * also terminate the process early while running.</p>
-   *
-   * <p>Note that stdout and stderr are written concurrently. If these are
-   * aliased to each other, or if the caller continues to write to these
-   * streams, it is the caller's duty to ensure thread safety.
-   * </p>
-   *
-   * <p>Note that in this case the {@link KillableObserver} will be assigned
-   * to start observing the process via
-   * {@link KillableObserver#startObserving(Killable)} but will only be
-   * unassigned via {@link KillableObserver#stopObserving(Killable)}, if
-   * {@link FutureCommandResult#get()} is called. If the
-   * {@link KillableObserver} implementation used with this method will
-   * not work correctly without calls to
-   * {@link KillableObserver#stopObserving(Killable)} then a new instance
-   * should be used for each call to this method.</p>
-   *
-   * @param stdinInput The input to this process's stdin
-   * @param observer {@link KillableObserver} that should observe the running
-   *  process, or {@link #NO_OBSERVER} if caller does not wish to kill
-   *  the process
-   * @param stdOut the process will write its standard output into this stream.
-   *  E.g., you could pass {@link System#out} as <code>stdOut</code>.
-   * @param stdErr the process will write its standard error into this stream.
-   *  E.g., you could pass {@link System#err} as <code>stdErr</code>.
-   * @param killSubprocessOnInterrupt whether or not to kill the created process on interrupt
-   * @param closeOutput whether to close stdout / stderr when the process closes its output streams.
-   * @return An object that can be used to check if the process terminated and
-   *  obtain the process results.
-   * @throws ExecFailedException if {@link Runtime#exec(String[])} fails for any
-   *  reason
-   * @throws NullPointerException if stdin is null
-   */
-  public FutureCommandResult executeAsynchronously(final InputStream stdinInput,
-                                    final KillableObserver observer,
-                                    final OutputStream stdOut,
-                                    final OutputStream stdErr,
-                                    final boolean killSubprocessOnInterrupt,
-                                    final boolean closeOutput)
-      throws CommandException {
-    // supporting "null" here for backwards compatibility
-    final KillableObserver theObserver =
-        observer == null ? NO_OBSERVER : observer;
-    nullCheck(stdinInput, "stdinInput");
-    return doExecute(new InputStreamInputSource(stdinInput),
-        theObserver,
-        Consumers.createStreamingConsumers(stdOut, stdErr),
-        killSubprocessOnInterrupt,
-        closeOutput);
-  }
-
-  public FutureCommandResult executeAsynchronously(final InputStream stdinInput,
-      final KillableObserver observer,
-      final OutputStream stdOut,
-      final OutputStream stdErr)
-      throws CommandException {
-    return executeAsynchronously(
-        stdinInput,
-        observer,
-        stdOut,
-        stdErr,
-        /*killSubprocess=*/ false,
-        /*closeOutput=*/ false);
-  }
-
-  // End of public API -------------------------------------------------------
-
-  private void nullCheck(Object argument, String argumentName) {
-    if (argument == null) {
-      String message = argumentName + " argument must not be null.";
-      throw new NullPointerException(message);
+  public String toDebugString() {
+    StringBuilder message = new StringBuilder(128);
+    message.append("Executing (without brackets):");
+    for (String arg : subprocessBuilder.getArgv()) {
+      message.append(" [");
+      message.append(arg);
+      message.append(']');
     }
+    message.append("; environment: ");
+    message.append(subprocessBuilder.getEnv());
+    message.append("; working dir: ");
+    File workingDirectory = subprocessBuilder.getWorkingDirectory();
+    message.append(workingDirectory == null ?
+                   "(current)" :
+                   workingDirectory.toString());
+    return message.toString();
   }
 
-  private FutureCommandResult doExecute(final InputSource stdinInput,
-      final KillableObserver observer,
-      final Consumers.OutErrConsumers outErrConsumers,
-      final boolean killSubprocessOnInterrupt,
-      final boolean closeOutputStreams)
-      throws ExecFailedException {
-
+  private FutureCommandResult doExecute(
+      InputStream stdinInput, OutErrConsumers outErrConsumers, boolean killSubprocessOnInterrupt) 
+          throws ExecFailedException {
+    Preconditions.checkNotNull(stdinInput, "stdinInput");
     logCommand();
 
     final Subprocess process = startProcess();
 
     outErrConsumers.logConsumptionStrategy();
-
     outErrConsumers.registerInputs(
-        process.getInputStream(), process.getErrorStream(), closeOutputStreams);
+        process.getInputStream(), process.getErrorStream(), /*closeOutputStreams=*/false);
 
+    // TODO(ulfjack): This call blocks until all input is written. If stdinInput is large (or
+    // unbounded), then the async calls can block for a long time, and the timeout is not properly
+    // enforced.
     processInput(stdinInput, process);
 
-    // TODO(bazel-team): if the input stream is unbounded, observers will not get start
-    // notification in a timely manner!
-    final Killable processKillable = observeProcess(process, observer);
+    final KillableObserver theObserver;
+    if ((subprocessBuilder.getTimeoutMillis() > 0)
+        && !SubprocessBuilder.factory.supportsTimeout()) {
+      theObserver = new TimeoutKillableObserver(subprocessBuilder.getTimeoutMillis());
+    } else {
+      theObserver = NO_OBSERVER;
+    }
+    Killable processKillable = observeProcess(process, theObserver);
 
     return new FutureCommandResult() {
       @Override
       public CommandResult get() throws AbnormalTerminationException {
-        return waitForProcessToComplete(process,
-            observer,
+        return waitForProcessToComplete(
+            process,
+            theObserver,
             processKillable,
             outErrConsumers,
             killSubprocessOnInterrupt);
@@ -729,11 +404,15 @@ public final class Command {
       public boolean isDone() {
         return process.finished();
       }
+
+      @Override
+      public void cancel() {
+        process.destroy();
+      }
     };
   }
 
-  private Subprocess startProcess()
-    throws ExecFailedException {
+  private Subprocess startProcess() throws ExecFailedException {
     try {
       return subprocessBuilder.start();
     } catch (IOException ioe) {
@@ -741,102 +420,49 @@ public final class Command {
     }
   }
 
-  private static interface InputSource {
-    void copyTo(OutputStream out) throws IOException;
-    boolean isEmpty();
-    String toLogString(String sourceName);
-  }
-
-  private static class ByteArrayInputSource implements InputSource {
-    private byte[] bytes;
-    ByteArrayInputSource(byte[] bytes){
-      this.bytes = bytes;
-    }
-    @Override
-    public void copyTo(OutputStream out) throws IOException {
-      out.write(bytes);
-      out.flush();
-    }
+  private static class NullInputStream extends InputStream {
     @Override
-    public boolean isEmpty() {
-      return bytes.length == 0;
-    }
-    @Override
-    public String toLogString(String sourceName) {
-      if (isEmpty()) {
-        return "No input to " + sourceName;
-      } else {
-        return "Input to " + sourceName + ": " +
-            LogUtil.toTruncatedString(bytes);
-      }
+    public int read() {
+      return -1;
     }
-  }
 
-  private static class InputStreamInputSource implements InputSource {
-    private InputStream inputStream;
-    InputStreamInputSource(InputStream inputStream){
-      this.inputStream = inputStream;
-    }
-    @Override
-    public void copyTo(OutputStream out) throws IOException {
-      byte[] buf = new byte[4096];
-      int r;
-      while ((r = inputStream.read(buf)) != -1) {
-        out.write(buf, 0, r);
-        out.flush();
-      }
-    }
     @Override
-    public boolean isEmpty() {
-      return false;
-    }
-    @Override
-    public String toLogString(String sourceName) {
-      return "Input to " + sourceName + " is a stream.";
+    public int available() {
+      return 0;
     }
   }
 
-  private static void processInput(InputSource stdinInput, Subprocess process) {
+  private static void processInput(InputStream stdinInput, Subprocess process) {
     if (log.isLoggable(Level.FINER)) {
-      log.finer(stdinInput.toLogString("stdin"));
+      log.finer(stdinInput.toString());
     }
-    try {
-      if (stdinInput.isEmpty()) {
-        return;
-      }
-      stdinInput.copyTo(process.getOutputStream());
+    try (OutputStream out = process.getOutputStream()) {
+      ByteStreams.copy(stdinInput, out);
     } catch (IOException ioe) {
-      // Note: this is not an error!  Perhaps the command just isn't hungry for
-      // our input and exited with success.  Process.waitFor (later) will tell
-      // us.
+      // Note: this is not an error!  Perhaps the command just isn't hungry for our input and exited
+      // with success. Process.waitFor (later) will tell us.
       //
       // (Unlike out/err streams, which are read asynchronously, the input stream is written
-      // synchronously, in its entirety, before processInput returns.  If the input is
-      // infinite, and is passed through e.g. "cat" subprocess and back into the
-      // ByteArrayOutputStream, that will eventually run out of memory, causing the output stream
-      // to be closed, "cat" to terminate with SIGPIPE, and processInput to receive an IOException.
-    } finally {
-      // if this statement is ever deleted, the process's outputStream
-      // must be closed elsewhere -- it is not closed automatically
-      Command.silentClose(process.getOutputStream());
+      // synchronously, in its entirety, before processInput returns.  If the input is infinite, and
+      // is passed through e.g. "cat" subprocess and back into the ByteArrayOutputStream, that will
+      // eventually run out of memory, causing the output stream to be closed, "cat" to terminate
+      // with SIGPIPE, and processInput to receive an IOException.
     }
   }
 
-  private static Killable observeProcess(Subprocess process,
-      final KillableObserver observer) {
-    final Killable processKillable = new ProcessKillable(process);
+  private static Killable observeProcess(Subprocess process, KillableObserver observer) {
+    Killable processKillable = new ProcessKillable(process);
     observer.startObserving(processKillable);
     return processKillable;
   }
 
   private CommandResult waitForProcessToComplete(
-    final Subprocess process,
-    final KillableObserver observer,
-    final Killable processKillable,
-    final Consumers.OutErrConsumers outErr,
-    final boolean killSubprocessOnInterrupt)
-    throws AbnormalTerminationException {
-
+      Subprocess process,
+      KillableObserver observer,
+      Killable processKillable,
+      Consumers.OutErrConsumers outErr,
+      boolean killSubprocessOnInterrupt)
+          throws AbnormalTerminationException {
     log.finer("Waiting for process...");
 
     TerminationStatus status = waitForProcess(process, killSubprocessOnInterrupt);
@@ -887,8 +513,8 @@ public final class Command {
     }
   }
 
-  private static TerminationStatus waitForProcess(Subprocess process,
-                                       boolean killSubprocessOnInterrupt) {
+  private static TerminationStatus waitForProcess(
+      Subprocess process, boolean killSubprocessOnInterrupt) {
     boolean wasInterrupted = false;
     try {
       while (true) {
@@ -916,40 +542,4 @@ public final class Command {
     }
     log.fine(toDebugString());
   }
-
-  /**
-   * A string representation of this command object which includes
-   * the arguments, the environment, and the working directory. Avoid
-   * relying on the specifics of this format. Note that the size
-   * of the result string will reflect the size of the command.
-   */
-  public String toDebugString() {
-    StringBuilder message = new StringBuilder(128);
-    message.append("Executing (without brackets):");
-    for (String arg : subprocessBuilder.getArgv()) {
-      message.append(" [");
-      message.append(arg);
-      message.append(']');
-    }
-    message.append("; environment: ");
-    message.append(subprocessBuilder.getEnv());
-    message.append("; working dir: ");
-    File workingDirectory = subprocessBuilder.getWorkingDirectory();
-    message.append(workingDirectory == null ?
-                   "(current)" :
-                   workingDirectory.toString());
-    return message.toString();
-  }
-
-  /**
-   * Close the <code>out</code> stream and log a warning if anything happens.
-   */
-  private static void silentClose(final OutputStream out) {
-    try {
-      out.close();
-    } catch (IOException ioe) {
-      String message = "Unexpected exception while closing output stream";
-      log.log(Level.WARNING, message, ioe);
-    }
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Consumers.java b/src/main/java/com/google/devtools/build/lib/shell/Consumers.java
index 47515d7d1f..b3864d36da 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Consumers.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Consumers.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.shell;
 
+import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.Uninterruptibles;
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
@@ -31,11 +32,10 @@ import java.util.logging.Logger;
 /**
  * This class provides convenience methods for consuming (actively reading)
  * output and error streams with different consumption policies:
- * discarding ({@link #createDiscardingConsumers()},
  * accumulating ({@link #createAccumulatingConsumers()},
  * and streaming ({@link #createStreamingConsumers(OutputStream, OutputStream)}).
  */
-class Consumers {
+final class Consumers {
 
   private static final Logger log =
     Logger.getLogger("com.google.devtools.build.lib.shell.Command");
@@ -45,33 +45,26 @@ class Consumers {
   private static final ExecutorService pool =
     Executors.newCachedThreadPool(new AccumulatorThreadFactory());
 
-  static OutErrConsumers createDiscardingConsumers() {
-    return new OutErrConsumers(new DiscardingConsumer(),
-                               new DiscardingConsumer());
-  }
-
   static OutErrConsumers createAccumulatingConsumers() {
-    return new OutErrConsumers(new AccumulatingConsumer(),
-                               new AccumulatingConsumer());
+    return new OutErrConsumers(new AccumulatingConsumer(), new AccumulatingConsumer());
   }
 
-  static OutErrConsumers createStreamingConsumers(OutputStream out,
-                                                  OutputStream err) {
-    return new OutErrConsumers(new StreamingConsumer(out),
-                               new StreamingConsumer(err));
+  static OutErrConsumers createStreamingConsumers(OutputStream out, OutputStream err) {
+    Preconditions.checkNotNull(out);
+    Preconditions.checkNotNull(err);
+    return new OutErrConsumers(new StreamingConsumer(out), new StreamingConsumer(err));
   }
 
   static class OutErrConsumers {
-
     private final OutputConsumer out;
     private final OutputConsumer err;
 
-    private OutErrConsumers(final OutputConsumer out, final OutputConsumer err){
+    private OutErrConsumers(final OutputConsumer out, final OutputConsumer err) {
       this.out = out;
       this.err = err;
     }
 
-    void registerInputs(InputStream outInput, InputStream errInput, boolean closeStreams){
+    void registerInputs(InputStream outInput, InputStream errInput, boolean closeStreams) {
       out.registerInput(outInput, closeStreams);
       err.registerInput(errInput, closeStreams);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/ExecFailedException.java b/src/main/java/com/google/devtools/build/lib/shell/ExecFailedException.java
index bc9760d7d4..8d5c627c0f 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/ExecFailedException.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/ExecFailedException.java
@@ -15,8 +15,8 @@
 package com.google.devtools.build.lib.shell;
 
 /**
- * Thrown when a command could not even be executed by the JVM --
- * in particular, when {@link Runtime#exec(String[])} fails.
+ * Thrown when a command could not even be executed by the JVM; in particular, when
+ * {@link Runtime#exec(String[])} fails.
  */
 public final class ExecFailedException extends CommandException {
 
diff --git a/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResult.java b/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResult.java
index 8d23e01ae6..a0fd99a38f 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResult.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/FutureCommandResult.java
@@ -14,16 +14,13 @@
 package com.google.devtools.build.lib.shell;
 
 /**
- * Supplier of the command result which additionally allows to check if
- * the command already terminated. Implementing full fledged Future would
- * be a much harder undertaking, so a bare minimum that makes this class still
- * useful for asynchronous command execution is implemented.
+ * Supplier of the command result which additionally allows to check if the command already
+ * terminated. Implementations of this interface may not be thread-safe.
  */
 public interface FutureCommandResult {
   /**
-   * Returns the result of command execution. If the process is not finished
-   * yet (as reported by {@link #isDone()}, the call will block until that
-   * process terminates.
+   * Returns the result of command execution. If the process is not finished yet (as reported by
+   * {@link #isDone()}, the call will block until that process terminates.
    *
    * @return non-null result of command execution
    * @throws AbnormalTerminationException if command execution failed
@@ -31,8 +28,14 @@ public interface FutureCommandResult {
   CommandResult get() throws AbnormalTerminationException;
 
   /**
-   * Returns true if the process terminated, the command result is available
-   * and the call to {@link #get()} will not block.
+   * Aborts the subprocess if it is still running. Note that it does not immediately terminate the
+   * process, so {@link #isDone} may still return true if called immediately afterwards.
+   */
+  void cancel();
+
+  /**
+   * Returns true if the process terminated, the command result is available and the call to
+   * {@link #get()} will not block.
    *
    * @return true if the process terminated
    */
diff --git a/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
index dac751669d..e266cc9b5f 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.shell;
 
 import com.google.devtools.build.lib.shell.SubprocessBuilder.StreamAction;
-
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -98,10 +97,12 @@ public class JavaSubprocessFactory implements Subprocess.Factory {
   }
 
   @Override
+  public boolean supportsTimeout() {
+    return false;
+  }
+
+  @Override
   public Subprocess create(SubprocessBuilder params) throws IOException {
-    if (params.getTimeoutMillis() >= 0) {
-      throw new UnsupportedOperationException("Timeouts are not supported");
-    }
     ProcessBuilder builder = new ProcessBuilder();
     builder.command(params.getArgv());
     if (params.getEnv() != null) {
@@ -117,8 +118,8 @@ public class JavaSubprocessFactory implements Subprocess.Factory {
   }
 
   /**
-   * Returns a {@link ProcessBuilder.Redirect} appropriate for the parameters. If a file redirected
-   * to exists, deletes the file before redirecting to it.
+   * Returns a {@link java.lang.ProcessBuilder.Redirect} appropriate for the parameters. If a file
+   * redirected to exists, deletes the file before redirecting to it.
    */
   private Redirect getRedirect(StreamAction action, File file) {
     switch (action) {
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Killable.java b/src/main/java/com/google/devtools/build/lib/shell/Killable.java
index c8f324cd00..82eec849d5 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Killable.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Killable.java
@@ -15,17 +15,15 @@
 package com.google.devtools.build.lib.shell;
 
 /**
- * Implementations encapsulate a running process that can be killed.
- * In particular, here, it is used to wrap up a {@link Process} object
- * and expose it to a {@link KillableObserver}. It is wrapped in this way
- * so that the actual {@link Process} object can't be altered by
- * a {@link KillableObserver}.
+ * Implementations encapsulate a running process that can be killed. In particular, here, it is used
+ * to wrap up a {@link Process} object and expose it to a {@link KillableObserver}. It is wrapped in
+ * this way so that the actual {@link Process} object can't be altered by a
+ * {@link KillableObserver}.
  */
-public interface Killable {
+interface Killable {
 
   /**
    * Kill this killable instance.
    */
   void kill();
-
 }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/KillableObserver.java b/src/main/java/com/google/devtools/build/lib/shell/KillableObserver.java
index fc2382fbc5..6afa57c816 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/KillableObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/KillableObserver.java
@@ -15,35 +15,24 @@
 package com.google.devtools.build.lib.shell;
 
 /**
- * Implementations of this interface observe, and potentially kill,
- * a {@link Killable} object. This is the mechanism by which "kill"
- * functionality is exposed to callers in the
- * {@link Command#execute(byte[], KillableObserver, boolean)} method.
- * 
+ * Implementations of this interface observe, and potentially kill, a {@link Killable} object.
  */
-public interface KillableObserver {
-
+interface KillableObserver {
   /**
-   * <p>Begin observing the given {@link Killable}. This method must return
-   * promptly; until it returns, {@link Command#execute()} cannot complete.
-   * Implementations may wish to start a new {@link Thread} here to handle
-   * kill logic, and to interrupt or otherwise ask the thread to stop in the
-   * {@link #stopObserving(Killable)} method. See
-   * <a href="http://builder.com.com/5100-6370-5144546.html">
-   * Interrupting Java threads</a> for notes on how to implement this
-   * correctly.</p>
+   * Begin observing the given {@link Killable}. This method must return promptly; until it returns,
+   * {@link Command#execute()} cannot complete. Implementations may wish to start a new
+   * {@link Thread} here to handle kill logic, and to interrupt or otherwise ask the thread to stop
+   * in the {@link #stopObserving(Killable)} method. See
+   * <a href="http://builder.com.com/5100-6370-5144546.html">Interrupting Java threads</a> for notes
+   * on how to implement this correctly.
    *
-   * <p>Implementations may or may not be able to observe more than
-   * one {@link Killable} at a time; see javadoc for details.</p>
+   * <p>Implementations may or may not be able to observe more than one {@link Killable} at a time;
+   * see javadoc for details.
    *
    * @param killable killable to observer
    */
   void startObserving(Killable killable);
 
-  /**
-   * Stop observing the given {@link Killable}, since it is
-   * no longer active.
-   */
+  /** Stop observing the given {@link Killable}, since it is no longer active. */
   void stopObserving(Killable killable);
-
 }
diff --git a/src/main/java/com/google/devtools/build/lib/shell/SimpleKillableObserver.java b/src/main/java/com/google/devtools/build/lib/shell/SimpleKillableObserver.java
deleted file mode 100644
index 4b6588311e..0000000000
--- a/src/main/java/com/google/devtools/build/lib/shell/SimpleKillableObserver.java
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.shell;
-
-/**
- * <p>A simple implementation of {@link KillableObserver} which can be told
- * explicitly to kill its {@link Killable} by calling {@link #kill()}. This
- * is the sort of functionality that callers might expect to find available
- * on the {@link Command} class.</p>
- *
- * <p>Note that this class can only observe one {@link Killable} at a time;
- * multiple instances should be used for concurrent calls to
- * {@link Command#execute(byte[], KillableObserver, boolean)}.</p>
- */
-public final class SimpleKillableObserver implements KillableObserver {
-
-  private Killable killable;
-
-  /**
-   * Does nothing except store a reference to the given {@link Killable}.
-   *
-   * @param killable {@link Killable} to kill
-   */
-  @Override
-  public synchronized void startObserving(final Killable killable) {
-    this.killable = killable;
-  }
-
-  /**
-   * Forgets reference to {@link Killable} provided to
-   * {@link #startObserving(Killable)}
-   */
-  @Override
-  public synchronized void stopObserving(final Killable killable) {
-    if (!this.killable.equals(killable)) {
-      throw new IllegalStateException("start/stopObservering called with " +
-                                      "different Killables");
-    }
-    this.killable = null;
-  }
-
-  /**
-   * Calls {@link Killable#kill()} on the saved {@link Killable}.
-   */
-  public synchronized void kill() {
-    if (killable != null) {
-      killable.kill();
-    }
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java b/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java
index 769b6b50bc..13ad59321a 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/Subprocess.java
@@ -28,6 +28,11 @@ public interface Subprocess extends Closeable {
    * Something that can create subprocesses.
    */
   interface Factory {
+    /**
+     * Whether the factory supports timeouts natively; if it returns false, Command implements
+     * timeouts outside of the factory.
+     */
+    boolean supportsTimeout();
 
     /**
      * Create a subprocess according to the specified parameters.
diff --git a/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java b/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java
index 5de1e8a4e9..ac21d64c8d 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/SubprocessBuilder.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap;
 import java.io.File;
 import java.io.IOException;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 /**
  * A builder class that starts a subprocess.
@@ -36,7 +37,8 @@ public class SubprocessBuilder {
     DISCARD,
 
     /** Stream back to the parent process using an output stream. */
-    STREAM };
+    STREAM
+  }
 
   private ImmutableList<String> argv;
   private ImmutableMap<String, String> env;
@@ -45,9 +47,9 @@ public class SubprocessBuilder {
   private StreamAction stderrAction;
   private File stderrFile;
   private File workingDirectory;
-  private long timeoutMillis = -1;
+  private long timeoutMillis;
 
-  private static Subprocess.Factory factory = JavaSubprocessFactory.INSTANCE;
+  static Subprocess.Factory factory = JavaSubprocessFactory.INSTANCE;
 
   public static void setSubprocessFactory(Subprocess.Factory factory) {
     SubprocessBuilder.factory = factory;
@@ -99,9 +101,8 @@ public class SubprocessBuilder {
    * Sets the environment passed to the child process. If null, inherit the environment of the
    * server.
    */
-  public SubprocessBuilder setEnv(Map<String, String> env) {
-    this.env = env == null
-        ? null : ImmutableMap.copyOf(env);
+  public SubprocessBuilder setEnv(@Nullable Map<String, String> env) {
+    this.env = env == null ? null : ImmutableMap.copyOf(env);
     return this;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/shell/TimeoutKillableObserver.java b/src/main/java/com/google/devtools/build/lib/shell/TimeoutKillableObserver.java
index ca6da0eadb..44a3299dd6 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/TimeoutKillableObserver.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/TimeoutKillableObserver.java
@@ -18,15 +18,13 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * <p>{@link KillableObserver} implementation which will kill its observed
- * {@link Killable} if it is still being observed after a given amount
- * of time has elapsed.</p>
+ * {@link KillableObserver} implementation which will kill its observed {@link Killable} if it is
+ * still being observed after a given amount of time has elapsed.
  *
- * <p>Note that this class can only observe one {@link Killable} at a time;
- * multiple instances should be used for concurrent calls to
- * {@link Command#execute(byte[], KillableObserver, boolean)}.</p>
+ * <p>Note that this class can only observe one {@link Killable} at a time; multiple instances
+ * should be used for concurrent calls to {@link Command#execute}.
  */
-public final class TimeoutKillableObserver implements KillableObserver {
+final class TimeoutKillableObserver implements KillableObserver {
 
   private static final Logger log =
       Logger.getLogger(TimeoutKillableObserver.class.getCanonicalName());
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
index 59d9792696..d99cafe6a7 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java
@@ -133,7 +133,8 @@ public class WindowsSubprocess implements Subprocess {
       boolean stderrRedirected, long timeoutMillis) {
     this.commandLine = commandLine;
     this.nativeProcess = nativeProcess;
-    this.timeoutMillis = timeoutMillis;
+    // As per the spec of Command, we should only apply timeouts that are > 0.
+    this.timeoutMillis = timeoutMillis <= 0 ? -1 : timeoutMillis;
     stdoutStream =
         stdoutRedirected ? null : new ProcessInputStream(WindowsProcesses.getStdout(nativeProcess));
     stderrStream =
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
index e398a3b195..d033bacf13 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
@@ -37,6 +37,11 @@ public class WindowsSubprocessFactory implements Subprocess.Factory {
   }
 
   @Override
+  public boolean supportsTimeout() {
+    return true;
+  }
+
+  @Override
   public Subprocess create(SubprocessBuilder builder) throws IOException {
     List<String> argv = builder.getArgv();