diff options
Diffstat (limited to 'src/main/java')
17 files changed, 82 insertions, 66 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java index d4827be875..e1156e9745 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.util.CommandDescriptionForm; import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.vfs.Path; +import java.time.Duration; import java.util.Collection; import java.util.Map; @@ -34,13 +35,13 @@ public final class Spawns { /** * Parse the timeout key in the spawn execution info, if it exists. Otherwise, return -1. */ - public static int getTimeoutSeconds(Spawn spawn) throws ExecException { + public static Duration getTimeout(Spawn spawn) throws ExecException { String timeoutStr = spawn.getExecutionInfo().get("timeout"); if (timeoutStr == null) { - return -1; + return Duration.ZERO; } try { - return Integer.parseInt(timeoutStr); + return Duration.ofSeconds(Integer.parseInt(timeoutStr)); } catch (NumberFormatException e) { throw new UserExecException("could not parse timeout: ", e); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 4641df0dc2..ffef030501 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -34,8 +34,8 @@ import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.time.Duration; import java.util.SortedMap; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -69,7 +69,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte if (actionExecutionContext.reportsSubcommands()) { actionExecutionContext.reportSubcommand(spawn); } - final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); + final Duration timeout = Spawns.getTimeout(spawn); SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { private final int id = execCount.incrementAndGet(); @@ -104,8 +104,8 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte } @Override - public long getTimeoutMillis() { - return TimeUnit.SECONDS.toMillis(timeoutSeconds); + public Duration getTimeout() { + return timeout; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index d8c6d774ee..ecea21a1da 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -24,6 +24,7 @@ import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsBase; +import java.time.Duration; import java.util.Map; /** @@ -203,7 +204,7 @@ public class ExecutionOptions extends OptionsBase { + "moderate, long and eternal (in that order). In either form, a value of -1 tells " + "blaze to use its default timeouts for that category." ) - public Map<TestTimeout, Integer> testTimeout; + public Map<TestTimeout, Duration> testTimeout; @Option( name = "resource_autosense", diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index c2819766cc..b98ca2ee59 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.time.Duration; import java.util.SortedMap; /** @@ -169,7 +170,7 @@ public interface SpawnRunner { void lockOutputFiles() throws InterruptedException; /** Returns the timeout that should be applied for the given {@link Spawn} instance. */ - long getTimeoutMillis(); + Duration getTimeout(); /** The files to which to write stdout and stderr. */ FileOutErr getFileOutErr(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index acbf5c4963..8edc5217fb 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -112,7 +112,7 @@ public class StandaloneTestStrategy extends TestStrategy { executionInfo.put(ExecutionRequirements.NO_CACHE, ""); } // This key is only understood by StandaloneSpawnStrategy. - executionInfo.put("timeout", "" + getTimeout(action)); + executionInfo.put("timeout", "" + getTimeout(action).getSeconds()); executionInfo.putAll(action.getTestProperties().getExecutionInfo()); ResourceSet localResourceUsage = diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java index 09bf0e04b3..b6db013173 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.rules.test.TestRunnerAction; import com.google.devtools.build.lib.util.UserUtils; import com.google.devtools.build.lib.vfs.PathFragment; +import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -58,7 +59,7 @@ public class TestPolicy { public Map<String, String> computeTestEnvironment( TestRunnerAction testAction, Map<String, String> clientEnv, - int timeoutInSeconds, + Duration timeout, PathFragment relativeRunfilesDir, PathFragment tmpDir) { Map<String, String> env = new HashMap<>(); @@ -95,7 +96,7 @@ public class TestPolicy { // Setup any test-specific env variables; note that this does not overwrite existing values for // TEST_RANDOM_SEED or TEST_SIZE if they're already set. - testAction.setupEnvVariables(env, timeoutInSeconds); + testAction.setupEnvVariables(env, timeout); return env; } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java index d12b700576..d37b56621a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java @@ -48,6 +48,7 @@ import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -235,7 +236,7 @@ public abstract class TestStrategy implements TestActionContext { * the "categorical timeouts" which are based on the --test_timeout flag. A rule picks its timeout * but ends up with the same effective value as all other rules in that bucket. */ - protected final int getTimeout(TestRunnerAction testAction) { + protected final Duration getTimeout(TestRunnerAction testAction) { return executionOptions.testTimeout.get(testAction.getTestProperties().getTimeout()); } 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 2619ef5249..b2c9d3c965 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 @@ -42,6 +42,7 @@ import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.io.OutputStream; +import java.time.Duration; import java.util.ArrayList; import java.util.EnumMap; import java.util.List; @@ -236,14 +237,13 @@ public final class LocalSpawnRunner implements SpawnRunner { stepLog(INFO, "running locally"); setState(State.LOCAL_ACTION_RUNNING); - int timeoutSeconds = (int) (policy.getTimeoutMillis() / 1000); Command cmd; OutputStream stdOut = ByteStreams.nullOutputStream(); OutputStream stdErr = ByteStreams.nullOutputStream(); if (useProcessWrapper) { List<String> cmdLine = new ArrayList<>(); cmdLine.add(processWrapper); - cmdLine.add("--timeout=" + timeoutSeconds); + cmdLine.add("--timeout=" + policy.getTimeout().getSeconds()); cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds); cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath())); cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath())); @@ -259,7 +259,10 @@ public final class LocalSpawnRunner implements SpawnRunner { spawn.getArguments().toArray(new String[0]), localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName), execRoot.getPathFile(), - policy.getTimeoutMillis()); + // 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()); } long startTime = System.currentTimeMillis(); @@ -289,9 +292,9 @@ public final class LocalSpawnRunner implements SpawnRunner { } setState(State.SUCCESS); - long wallTime = System.currentTimeMillis() - startTime; + long wallTimeMillis = System.currentTimeMillis() - startTime; boolean wasTimeout = result.getTerminationStatus().timedout() - || (useProcessWrapper && wasTimeout(timeoutSeconds, wallTime)); + || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis)); Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; int exitCode = status == Status.TIMEOUT ? POSIX_TIMEOUT_EXIT_CODE @@ -300,7 +303,7 @@ public final class LocalSpawnRunner implements SpawnRunner { .setStatus(status) .setExitCode(exitCode) .setExecutorHostname(hostName) - .setWallTimeMillis(wallTime) + .setWallTimeMillis(wallTimeMillis) .build(); } @@ -308,8 +311,8 @@ public final class LocalSpawnRunner implements SpawnRunner { return path == null ? "/dev/null" : path.getPathString(); } - private boolean wasTimeout(int timeoutSeconds, long wallTimeMillis) { - return timeoutSeconds > 0 && wallTimeMillis / 1000.0 > timeoutSeconds; + private boolean wasTimeout(Duration timeout, long wallTimeMillis) { + return !timeout.isZero() && wallTimeMillis > timeout.toMillis(); } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java b/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java index a14f68ef65..6e834a70d7 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java +++ b/src/main/java/com/google/devtools/build/lib/packages/TestTimeout.java @@ -23,6 +23,7 @@ import com.google.common.collect.RangeMap; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.EnumMap; @@ -140,10 +141,15 @@ public enum TestTimeout { return super.toString().toUpperCase(); } - public int getTimeout() { + @Deprecated // use getTimeout instead + public int getTimeoutSeconds() { return timeout; } + public Duration getTimeout() { + return Duration.ofSeconds(timeout); + } + /** * Returns true iff the given time is not close to the upper bound timeout and is so short that it * should be assigned a different timeout. @@ -184,25 +190,25 @@ public enum TestTimeout { /** * Converter for the --test_timeout option. */ - public static class TestTimeoutConverter implements Converter<Map<TestTimeout, Integer>> { + public static class TestTimeoutConverter implements Converter<Map<TestTimeout, Duration>> { public TestTimeoutConverter() {} @Override - public Map<TestTimeout, Integer> convert(String input) throws OptionsParsingException { - List<Integer> values = new ArrayList<>(); + public Map<TestTimeout, Duration> convert(String input) throws OptionsParsingException { + List<Duration> values = new ArrayList<>(); for (String token : Splitter.on(',').limit(6).split(input)) { // Handle the case of "2," which is accepted as legal... Because Splitter.split is lazy, // there's no way of knowing if an empty string is a trailing or an intermediate one, // so we can't fully emulate String.split(String, 0). if (!token.isEmpty() || values.size() > 1) { try { - values.add(Integer.valueOf(token)); + values.add(Duration.ofSeconds(Integer.valueOf(token))); } catch (NumberFormatException e) { throw new OptionsParsingException("'" + input + "' is not an int"); } } } - EnumMap<TestTimeout, Integer> timeouts = Maps.newEnumMap(TestTimeout.class); + EnumMap<TestTimeout, Duration> timeouts = Maps.newEnumMap(TestTimeout.class); if (values.size() == 1) { timeouts.put(SHORT, values.get(0)); timeouts.put(MODERATE, values.get(0)); @@ -217,7 +223,7 @@ public enum TestTimeout { throw new OptionsParsingException("Invalid number of comma-separated entries"); } for (TestTimeout label : values()) { - if (!timeouts.containsKey(label) || timeouts.get(label) <= 0) { + if (!timeouts.containsKey(label) || timeouts.get(label).compareTo(Duration.ZERO) <= 0) { timeouts.put(label, label.getTimeout()); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java index 8b0376a4b7..0eda635972 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunner.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Spawn; -import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.exec.SpawnResult; @@ -33,11 +32,11 @@ import com.google.devtools.remoteexecution.v1test.ActionResult; import com.google.devtools.remoteexecution.v1test.Command; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.Platform; -import com.google.protobuf.Duration; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import io.grpc.StatusRuntimeException; import java.io.IOException; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -97,7 +96,7 @@ final class CachedLocalSpawnRunner implements SpawnRunner { spawn.getOutputFiles(), Digests.computeDigest(command), repository.getMerkleDigest(inputRoot), - Spawns.getTimeoutSeconds(spawn)); + policy.getTimeout()); // Look up action cache, and reuse the action output if it is found. actionKey = Digests.computeActionKey(action); @@ -135,7 +134,7 @@ final class CachedLocalSpawnRunner implements SpawnRunner { Collection<? extends ActionInput> outputs, Digest command, Digest inputRoot, - long timeoutSeconds) { + Duration timeout) { Action.Builder action = Action.newBuilder(); action.setCommandDigest(command); action.setInputRootDigest(inputRoot); @@ -149,8 +148,8 @@ final class CachedLocalSpawnRunner implements SpawnRunner { if (platform != null) { action.setPlatform(platform); } - if (timeoutSeconds > 0) { - action.setTimeout(Duration.newBuilder().setSeconds(timeoutSeconds)); + if (!timeout.isZero()) { + action.setTimeout(com.google.protobuf.Duration.newBuilder().setSeconds(timeout.getSeconds())); } return action.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index ff24657f93..45ed382569 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -40,9 +40,9 @@ import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.ExecuteRequest; import com.google.devtools.remoteexecution.v1test.ExecuteResponse; import com.google.devtools.remoteexecution.v1test.Platform; -import com.google.protobuf.Duration; import io.grpc.Status.Code; import java.io.IOException; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -102,7 +102,7 @@ class RemoteSpawnRunner implements SpawnRunner { spawn.getOutputFiles(), Digests.computeDigest(command), repository.getMerkleDigest(inputRoot), - Spawns.getTimeoutSeconds(spawn)); + policy.getTimeout()); // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = Digests.computeActionKey(action); @@ -179,7 +179,7 @@ class RemoteSpawnRunner implements SpawnRunner { Collection<? extends ActionInput> outputs, Digest command, Digest inputRoot, - long timeoutSeconds) { + Duration timeout) { Action.Builder action = Action.newBuilder(); action.setCommandDigest(command); action.setInputRootDigest(inputRoot); @@ -193,8 +193,8 @@ class RemoteSpawnRunner implements SpawnRunner { if (platform != null) { action.setPlatform(platform); } - if (timeoutSeconds > 0) { - action.setTimeout(Duration.newBuilder().setSeconds(timeoutSeconds)); + if (!timeout.isZero()) { + action.setTimeout(com.google.protobuf.Duration.newBuilder().setSeconds(timeout.getSeconds())); } return action.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java index 38d61a1ae8..8a58468ac7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java @@ -46,6 +46,7 @@ import com.google.devtools.common.options.TriState; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -401,9 +402,9 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa } } - public void setupEnvVariables(Map<String, String> env, int timeoutInSeconds) { + public void setupEnvVariables(Map<String, String> env, Duration timeout) { env.put("TEST_SIZE", getTestProperties().getSize().toString()); - env.put("TEST_TIMEOUT", Integer.toString(timeoutInSeconds)); + env.put("TEST_TIMEOUT", Long.toString(timeout.getSeconds())); env.put("TEST_WORKSPACE", getRunfilesPrefix()); env.put( "TEST_BINARY", 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 f4fbeebbde..7046a7bf7b 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 @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.time.Duration; import java.util.Map; /** Abstract common ancestor for sandbox spawn runners implementing the common parts. */ @@ -81,12 +82,12 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { SandboxedSpawn sandbox, SpawnExecutionPolicy policy, Path execRoot, - int timeoutSeconds) + Duration timeout) throws ExecException, IOException, InterruptedException { try { sandbox.createFileSystem(); OutErr outErr = policy.getFileOutErr(); - SpawnResult result = run(sandbox, outErr, timeoutSeconds); + SpawnResult result = run(sandbox, outErr, timeout); policy.lockOutputFiles(); @@ -120,7 +121,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } } - private final SpawnResult run(SandboxedSpawn sandbox, OutErr outErr, int timeoutSeconds) + private final SpawnResult run(SandboxedSpawn sandbox, OutErr outErr, Duration timeout) throws IOException, InterruptedException { Command cmd = new Command( sandbox.getArguments().toArray(new String[0]), @@ -157,7 +158,7 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { } long wallTime = System.currentTimeMillis() - startTime; - boolean wasTimeout = wasTimeout(timeoutSeconds, wallTime); + boolean wasTimeout = wasTimeout(timeout, wallTime); Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; int exitCode = status == Status.TIMEOUT ? POSIX_TIMEOUT_EXIT_CODE @@ -169,8 +170,8 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { .build(); } - private boolean wasTimeout(int timeoutSeconds, long wallTimeMillis) { - return timeoutSeconds > 0 && wallTimeMillis / 1000.0 > timeoutSeconds; + private boolean wasTimeout(Duration timeout, long wallTimeMillis) { + return !timeout.isZero() && wallTimeMillis > timeout.toMillis(); } /** 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 2344516ac7..e34b04ccd3 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 @@ -39,12 +39,12 @@ import java.io.File; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; +import java.time.Duration; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; /** Spawn runner that uses Darwin (macOS) sandboxing to execute a process. */ @ExecutionStrategy( @@ -188,9 +188,9 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb"); - int timeoutSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(policy.getTimeoutMillis()); + Duration timeout = policy.getTimeout(); List<String> arguments = - computeCommandLine(spawn, timeoutSeconds, sandboxConfigPath, timeoutGraceSeconds); + computeCommandLine(spawn, timeout, sandboxConfigPath, timeoutGraceSeconds); Map<String, String> environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName); @@ -210,18 +210,18 @@ final class DarwinSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { sandboxConfigPath, writableDirs, getInaccessiblePaths(), allowNetworkForThisSpawn); } }; - return runSpawn(spawn, sandbox, policy, execRoot, timeoutSeconds); + return runSpawn(spawn, sandbox, policy, execRoot, timeout); } private List<String> computeCommandLine( - Spawn spawn, int timeoutSeconds, Path sandboxConfigPath, int timeoutGraceSeconds) { + Spawn spawn, Duration timeout, Path sandboxConfigPath, int timeoutGraceSeconds) { List<String> commandLineArgs = new ArrayList<>(); commandLineArgs.add(SANDBOX_EXEC); commandLineArgs.add("-f"); commandLineArgs.add(sandboxConfigPath.getPathString()); commandLineArgs.addAll( ProcessWrapperRunner.getCommandLine( - processWrapper, spawn.getArguments(), timeoutSeconds, timeoutGraceSeconds)); + processWrapper, spawn.getArguments(), timeout, timeoutGraceSeconds)); return commandLineArgs; } 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 7ea2120441..ecaf2bb842 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 @@ -33,12 +33,12 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import java.io.File; import java.io.IOException; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; import java.util.SortedMap; -import java.util.concurrent.TimeUnit; /** Spawn runner that uses linux sandboxing APIs to execute a local subprocess. */ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { @@ -118,10 +118,10 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment()); ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn); - int timeoutSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(policy.getTimeoutMillis()); + Duration timeout = policy.getTimeout(); List<String> arguments = computeCommandLine( spawn, - timeoutSeconds, + timeout, linuxSandbox, writableDirs, getTmpfsPaths(), @@ -136,12 +136,12 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { SandboxHelpers.getInputFiles(spawn, policy, execRoot), outputs, writableDirs); - return runSpawn(spawn, sandbox, policy, execRoot, timeoutSeconds); + return runSpawn(spawn, sandbox, policy, execRoot, timeout); } private List<String> computeCommandLine( Spawn spawn, - int timeoutSeconds, + Duration timeout, Path linuxSandbox, Set<Path> writableDirs, Set<Path> tmpfsPaths, @@ -155,9 +155,9 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { } // Kill the process after a timeout. - if (timeoutSeconds != -1) { + if (!timeout.isZero()) { commandLineArgs.add("-T"); - commandLineArgs.add(Integer.toString(timeoutSeconds)); + commandLineArgs.add(Long.toString(timeout.getSeconds())); } if (timeoutGraceSeconds != -1) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java index 481044638a..cff4678e28 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperRunner.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -41,10 +42,10 @@ final class ProcessWrapperRunner { } static List<String> getCommandLine( - Path processWrapper, List<String> spawnArguments, int timeout, int timeoutGraceSeconds) { + Path processWrapper, List<String> spawnArguments, Duration timeout, int timeoutGraceSeconds) { List<String> commandLineArgs = new ArrayList<>(5 + spawnArguments.size()); commandLineArgs.add(processWrapper.getPathString()); - commandLineArgs.add("--timeout=" + timeout); + commandLineArgs.add("--timeout=" + timeout.getSeconds()); commandLineArgs.add("--kill_delay=" + timeoutGraceSeconds); commandLineArgs.addAll(spawnArguments); return commandLineArgs; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 5e98898030..f77e90523e 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -23,9 +23,9 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; +import java.time.Duration; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; /** Strategy that uses sandboxing to execute a process. */ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { @@ -62,10 +62,10 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne Path sandboxPath = getSandboxRoot(); Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName()); - int timeoutSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(policy.getTimeoutMillis()); + Duration timeout = policy.getTimeout(); List<String> arguments = ProcessWrapperRunner.getCommandLine( - processWrapper, spawn.getArguments(), timeoutSeconds, timeoutGraceSeconds); + processWrapper, spawn.getArguments(), timeout, timeoutGraceSeconds); Map<String, String> environment = localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName); @@ -77,7 +77,7 @@ final class ProcessWrapperSandboxedSpawnRunner extends AbstractSandboxSpawnRunne SandboxHelpers.getInputFiles(spawn, policy, execRoot), SandboxHelpers.getOutputFiles(spawn), getWritableDirs(sandboxExecRoot, spawn.getEnvironment())); - return runSpawn(spawn, sandbox, policy, execRoot, timeoutSeconds); + return runSpawn(spawn, sandbox, policy, execRoot, timeout); } @Override |