diff options
Diffstat (limited to 'src/main')
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(); |