Skip to content

Commit

Permalink
Rewrite AbstractSandboxSpawnRunner to use Subprocess
Browse files Browse the repository at this point in the history
It was previously using Command. This change is in preparation for
merging the local sandboxed and non-sandboxed execution paths, in order
to simplify supporting async execution for both.

I copied over the equivalent code (with minor changes) from
LocalSpawnRunner with the intent of unifying the LocalSpawnRunner and
the AbstractSandboxSpawnRunner. It may be possible to extract this into
a common class rather than having two copies. TBD.

PiperOrigin-RevId: 262545190
  • Loading branch information
ulfjack authored and copybara-github committed Aug 9, 2019
1 parent dcc80fb commit 71288e6
Showing 1 changed file with 27 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.shell.AbnormalTerminationException;
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.ExecutionStatistics;
import com.google.devtools.build.lib.shell.Subprocess;
import com.google.devtools.build.lib.shell.SubprocessBuilder;
import com.google.devtools.build.lib.shell.TerminationStatus;
import com.google.devtools.build.lib.util.CommandFailureUtils;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.OutErr;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
Expand Down Expand Up @@ -104,7 +103,7 @@ protected SpawnResult runSpawn(
throws IOException, InterruptedException {
try {
sandbox.createFileSystem();
OutErr outErr = context.getFileOutErr();
FileOutErr outErr = context.getFileOutErr();
context.prefetchInputs();

SpawnResult result = run(originalSpawn, sandbox, outErr, timeout, statisticsPath);
Expand All @@ -127,14 +126,10 @@ protected SpawnResult runSpawn(
private final SpawnResult run(
Spawn originalSpawn,
SandboxedSpawn sandbox,
OutErr outErr,
FileOutErr outErr,
Duration timeout,
Path statisticsPath)
throws IOException, InterruptedException {
Command cmd = new Command(
sandbox.getArguments().toArray(new String[0]),
sandbox.getEnvironment(),
sandbox.getSandboxExecRoot().getPathFile());
String failureMessage;
if (sandboxOptions.sandboxDebug) {
failureMessage =
Expand All @@ -155,23 +150,29 @@ private final SpawnResult run(
+ SANDBOX_DEBUG_SUGGESTION;
}

SubprocessBuilder subprocessBuilder = new SubprocessBuilder();
subprocessBuilder.setWorkingDirectory(sandbox.getSandboxExecRoot().getPathFile());
subprocessBuilder.setStdout(outErr.getOutputPath().getPathFile());
subprocessBuilder.setStderr(outErr.getErrorPath().getPathFile());
subprocessBuilder.setEnv(sandbox.getEnvironment());
subprocessBuilder.setArgv(sandbox.getArguments());
long startTime = System.currentTimeMillis();
CommandResult commandResult;
TerminationStatus terminationStatus;
try {
commandResult = cmd.execute(outErr.getOutputStream(), outErr.getErrorStream());
if (Thread.currentThread().isInterrupted()) {
throw new InterruptedException();
}
} catch (AbnormalTerminationException e) {
if (Thread.currentThread().isInterrupted()) {
throw new InterruptedException();
Subprocess subprocess = subprocessBuilder.start();
subprocess.getOutputStream().close();
try {
subprocess.waitFor();
terminationStatus = new TerminationStatus(subprocess.exitValue(), subprocess.timedout());
} catch (InterruptedException e) {
subprocess.destroy();
throw e;
}
commandResult = e.getResult();
} catch (CommandException e) {
// At the time this comment was written, this must be a ExecFailedException encapsulating an
// IOException from the underlying Subprocess.Factory.
} catch (IOException e) {
String msg = e.getMessage() == null ? e.getClass().getName() : e.getMessage();
outErr.getErrorStream().write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8));
outErr
.getErrorStream()
.write(("Action failed to execute: java.io.IOException: " + msg + "\n").getBytes(UTF_8));
outErr.getErrorStream().flush();
return new SpawnResult.Builder()
.setRunnerName(getName())
Expand All @@ -183,11 +184,8 @@ private final SpawnResult run(

// TODO(b/62588075): Calculate wall time inside commands instead?
Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime);
boolean wasTimeout = wasTimeout(timeout, wallTime);
int exitCode =
wasTimeout
? POSIX_TIMEOUT_EXIT_CODE
: commandResult.getTerminationStatus().getRawExitCode();
boolean wasTimeout = terminationStatus.timedOut() || wasTimeout(timeout, wallTime);
int exitCode = wasTimeout ? POSIX_TIMEOUT_EXIT_CODE : terminationStatus.getRawExitCode();
Status status =
wasTimeout
? Status.TIMEOUT
Expand Down

0 comments on commit 71288e6

Please sign in to comment.