diff options
author | 2017-08-09 15:27:49 +0200 | |
---|---|---|
committer | 2017-08-10 13:39:00 +0200 | |
commit | f2d459502f5fb422d6000db782795cffc6efa3e4 (patch) | |
tree | 35fc4d0cfb5cd22a179107fd90068366ffa85768 /src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java | |
parent | 6b2dce6710ed7d2429e0e3dc113c9bad622b8c4b (diff) |
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
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java | 12 |
1 files changed, 7 insertions, 5 deletions
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(); } |