Add the 'supports-graceful-termination' tag to request nice termination.

If an action specifies this new requirement and it spawns subprocesses via
the process-wrapper or the linux-sandbox, then interrupting Bazel while the
action is running will cause these subprocesses to be terminated gracefully
(first with a SIGTERM, then with a SIGKILL).

Tests that implement cleanup routines on interrupt can benefit from this to
ensure that those cleanup steps take place.

We aren't making this the default for all actions to keep Ctrl+C fast
(which arguably is already too slow sometimes).

Note that for this to work consistently across the local and the sandboxed
strategies, I've also had to fix AbstractSandboxSpawnRunner to properly
await for interrupted subprocesses, just like we did for the
LocalSpawnRunner back in de8d4c7.

RELNOTES: Added support for a 'supports-graceful-termination' execution
requirement and tag, which causes Bazel to send a SIGTERM to any tagged
actions before sending a delayed SIGKILL. This is to give actions, and more
specifically tests, a chance to clean up after themselves.
PiperOrigin-RevId: 329940226

src/main/java/com/google/devtools/build/lib/BUILD

@@ -276,6 +276,7 @@ java_library(
         ":runtime/command_dispatcher",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/actions:middleman_type",

src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java

@@ -241,4 +241,9 @@ public enum WorkerProtocolFormat {
   /** Use this to request eager fetching of a single remote output into local memory. */
   public static final String REMOTE_EXECUTION_INLINE_OUTPUTS = "internal-inline-outputs";
 
+  /**
+   * Request graceful termination of subprocesses on interrupt (that is, an initial {@code SIGTERM}
+   * followed by a {@code SIGKILL} after a grace period).
+   */
+  public static final String GRACEFUL_TERMINATION = "supports-graceful-termination";
 }

src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java

@@ -354,6 +354,7 @@ private SpawnResult start() throws InterruptedException, IOException {
           ProcessWrapper.CommandLineBuilder commandLineBuilder =
               processWrapper
                   .commandLineBuilder(spawn.getArguments())
+                  .addExecutionInfo(spawn.getExecutionInfo())
                   .setTimeout(context.getTimeout());
           if (localExecutionOptions.collectLocalExecutionStatistics) {
             statisticsPath = tmpDir.getRelative("stats.out");

src/main/java/com/google/devtools/build/lib/runtime/ProcessWrapper.java

@@ -15,14 +15,15 @@
 package com.google.devtools.build.lib.runtime;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.exec.local.LocalExecutionOptions;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import javax.annotation.Nullable;
 
 /** Tracks process-wrapper configuration and allows building command lines that rely on it. */
@@ -37,15 +38,15 @@ public final class ProcessWrapper {
   /** Grace delay between asking a process to stop and forcibly killing it, or null for none. */
   @Nullable private final Duration killDelay;
 
-  /** Optional list of extra flags to append to the process-wrapper invocations. */
-  private final List<String> extraFlags;
+  /** Whether to pass {@code --graceful_sigterm} or not to the process-wrapper. */
+  private final boolean gracefulSigterm;
 
   /** Creates a new process-wrapper instance from explicit values. */
   @VisibleForTesting
-  public ProcessWrapper(Path binPath, @Nullable Duration killDelay, List<String> extraFlags) {
+  public ProcessWrapper(Path binPath, @Nullable Duration killDelay, boolean gracefulSigterm) {
     this.binPath = binPath;
     this.killDelay = killDelay;
-    this.extraFlags = extraFlags;
+    this.gracefulSigterm = gracefulSigterm;
   }
 
   /**
@@ -59,24 +60,20 @@ public static ProcessWrapper fromCommandEnvironment(CommandEnvironment cmdEnv) {
     LocalExecutionOptions options = cmdEnv.getOptions().getOptions(LocalExecutionOptions.class);
     Duration killDelay = options == null ? null : options.getLocalSigkillGraceSeconds();
 
-    ImmutableList.Builder<String> extraFlags = ImmutableList.builder();
-    if (options != null) {
-      if (options.processWrapperGracefulSigterm) {
-        extraFlags.add("--graceful_sigterm");
-      }
-    }
+    boolean gracefulSigterm = options != null && options.processWrapperGracefulSigterm;
 
     Path path = cmdEnv.getBlazeWorkspace().getBinTools().getEmbeddedPath(BIN_BASENAME);
     if (OS.isPosixCompatible() && path != null && path.exists()) {
-      return new ProcessWrapper(path, killDelay, extraFlags.build());
+      return new ProcessWrapper(path, killDelay, gracefulSigterm);
     } else {
       return null;
     }
   }
 
   /** Returns a new {@link CommandLineBuilder} for the process-wrapper tool. */
   public CommandLineBuilder commandLineBuilder(List<String> commandArguments) {
-    return new CommandLineBuilder(binPath.getPathString(), commandArguments, killDelay, extraFlags);
+    return new CommandLineBuilder(
+        binPath.getPathString(), commandArguments, killDelay, gracefulSigterm);
   }
 
   /**
@@ -87,7 +84,7 @@ public static class CommandLineBuilder {
     private final String processWrapperPath;
     private final List<String> commandArguments;
     @Nullable private final Duration killDelay;
-    private final List<String> extraFlags;
+    private boolean gracefulSigterm;
 
     private Path stdoutPath;
     private Path stderrPath;
@@ -98,11 +95,11 @@ private CommandLineBuilder(
         String processWrapperPath,
         List<String> commandArguments,
         @Nullable Duration killDelay,
-        List<String> extraFlags) {
+        boolean gracefulSigterm) {
       this.processWrapperPath = processWrapperPath;
       this.commandArguments = commandArguments;
       this.killDelay = killDelay;
-      this.extraFlags = extraFlags;
+      this.gracefulSigterm = gracefulSigterm;
     }
 
     /** Sets the path to use for redirecting stdout, if any. */
@@ -129,6 +126,14 @@ public CommandLineBuilder setStatisticsPath(Path statisticsPath) {
       return this;
     }
 
+    /** Incorporates settings from a spawn's execution info. */
+    public CommandLineBuilder addExecutionInfo(Map<String, String> executionInfo) {
+      if (executionInfo.containsKey(ExecutionRequirements.GRACEFUL_TERMINATION)) {
+        gracefulSigterm = true;
+      }
+      return this;
+    }
+
     /** Build the command line to invoke a specific command using the process wrapper tool. */
     public List<String> build() {
       List<String> fullCommandLine = new ArrayList<>();
@@ -149,8 +154,9 @@ public List<String> build() {
       if (statisticsPath != null) {
         fullCommandLine.add("--stats=" + statisticsPath);
       }
-
-      fullCommandLine.addAll(extraFlags);
+      if (gracefulSigterm) {
+        fullCommandLine.add("--graceful_sigterm");
+      }
 
       fullCommandLine.addAll(commandArguments);
 

src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java

@@ -237,7 +237,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     Duration timeout = context.getTimeout();
 
     ProcessWrapper.CommandLineBuilder processWrapperCommandLineBuilder =
-        processWrapper.commandLineBuilder(spawn.getArguments()).setTimeout(timeout);
+        processWrapper
+            .commandLineBuilder(spawn.getArguments())
+            .addExecutionInfo(spawn.getExecutionInfo())
+            .setTimeout(timeout);
 
     final Path statisticsPath;
     if (getSandboxOptions().collectLocalSandboxExecutionStatistics) {

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxUtil.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.util.OsUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -70,6 +71,7 @@ public static class CommandLineBuilder {
     private boolean useFakeRoot = false;
     private boolean useFakeUsername = false;
     private boolean useDebugMode = false;
+    private boolean sigintSendsSigterm = false;
 
     private CommandLineBuilder(Path linuxSandboxPath, List<String> commandArguments) {
       this.linuxSandboxPath = linuxSandboxPath;
@@ -167,6 +169,14 @@ public CommandLineBuilder setUseDebugMode(boolean useDebugMode) {
       return this;
     }
 
+    /** Incorporates settings from a spawn's execution info. */
+    public CommandLineBuilder addExecutionInfo(Map<String, String> executionInfo) {
+      if (executionInfo.containsKey(ExecutionRequirements.GRACEFUL_TERMINATION)) {
+        sigintSendsSigterm = true;
+      }
+      return this;
+    }
+
     /**
      * Builds the command line to invoke a specific command using the {@code linux-sandbox} tool.
      */
@@ -225,6 +235,9 @@ public ImmutableList<String> build() {
       if (useDebugMode) {
         commandLineBuilder.add("-D");
       }
+      if (sigintSendsSigterm) {
+        commandLineBuilder.add("-i");
+      }
       commandLineBuilder.add("--");
       commandLineBuilder.addAll(commandArguments);
 

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java

@@ -183,6 +183,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
 
     LinuxSandboxUtil.CommandLineBuilder commandLineBuilder =
         LinuxSandboxUtil.commandLineBuilder(linuxSandbox, spawn.getArguments())
+            .addExecutionInfo(spawn.getExecutionInfo())
             .setWritableFilesAndDirectories(writableDirs)
             .setTmpfsDirectories(getTmpfsPaths())
             .setBindMounts(getReadOnlyBindMounts(blazeDirs, sandboxExecRoot))

src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java

@@ -96,7 +96,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
 
     Duration timeout = context.getTimeout();
     ProcessWrapper.CommandLineBuilder commandLineBuilder =
-        processWrapper.commandLineBuilder(spawn.getArguments()).setTimeout(timeout);
+        processWrapper
+            .commandLineBuilder(spawn.getArguments())
+            .addExecutionInfo(spawn.getExecutionInfo())
+            .setTimeout(timeout);
 
     Path statisticsPath = null;
     if (getSandboxOptions().collectLocalSandboxExecutionStatistics) {

src/test/java/com/google/devtools/build/lib/BUILD

@@ -371,6 +371,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:runtime/blaze_command_result",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+        "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",

src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java

@@ -318,7 +318,9 @@ private FileSystem setupEnvironmentForFakeExecution() {
 
   private static ProcessWrapper makeProcessWrapper(FileSystem fs, LocalExecutionOptions options) {
     return new ProcessWrapper(
-        fs.getPath("/process-wrapper"), options.getLocalSigkillGraceSeconds(), ImmutableList.of());
+        fs.getPath("/process-wrapper"),
+        options.getLocalSigkillGraceSeconds(),
+        /*gracefulSigterm=*/ false);
   }
 
   /**
@@ -915,9 +917,7 @@ public void hasExecutionStatistics_whenOptionIsEnabled() throws Exception {
             LocalSpawnRunnerTest::keepLocalEnvUnchanged,
             binTools,
             new ProcessWrapper(
-                processWrapperPath,
-                /*killDelay=*/ Duration.ZERO,
-                /*extraFlags=*/ ImmutableList.of()),
+                processWrapperPath, /*killDelay=*/ Duration.ZERO, /*gracefulSigterm=*/ false),
             Mockito.mock(RunfilesTreeUpdater.class));
 
     Spawn spawn =
@@ -982,9 +982,7 @@ public void hasNoExecutionStatistics_whenOptionIsDisabled() throws Exception {
             LocalSpawnRunnerTest::keepLocalEnvUnchanged,
             binTools,
             new ProcessWrapper(
-                processWrapperPath,
-                /*killDelay=*/ Duration.ZERO,
-                /*extraFlags=*/ ImmutableList.of()),
+                processWrapperPath, /*killDelay=*/ Duration.ZERO, /*gracefulSigterm=*/ false),
             Mockito.mock(RunfilesTreeUpdater.class));
 
     Spawn spawn =

src/test/java/com/google/devtools/build/lib/runtime/ProcessWrapperTest.java

@@ -17,6 +17,8 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
@@ -54,9 +56,9 @@ public void testProcessWrapperCommandLineBuilder_buildsWithoutOptionalArguments(
     ImmutableList<String> expectedCommandLine =
         ImmutableList.<String>builder().add("/some/path").addAll(commandArguments).build();
 
-    ImmutableList<String> extraFlags = ImmutableList.of();
     ProcessWrapper processWrapper =
-        new ProcessWrapper(makeProcessWrapperBin("/some/path"), /*killDelay=*/ null, extraFlags);
+        new ProcessWrapper(
+            makeProcessWrapperBin("/some/path"), /*killDelay=*/ null, /*gracefulSigterm=*/ false);
     List<String> commandLine = processWrapper.commandLineBuilder(commandArguments).build();
 
     assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder();
@@ -65,7 +67,6 @@ public void testProcessWrapperCommandLineBuilder_buildsWithoutOptionalArguments(
   @Test
   public void testProcessWrapperCommandLineBuilder_buildsWithOptionalArguments()
       throws IOException {
-    ImmutableList<String> extraFlags = ImmutableList.of("--debug");
     ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, world");
 
     Duration timeout = Duration.ofSeconds(10);
@@ -82,12 +83,13 @@ public void testProcessWrapperCommandLineBuilder_buildsWithOptionalArguments()
             .add("--stdout=" + stdoutPath)
             .add("--stderr=" + stderrPath)
             .add("--stats=" + statisticsPath)
-            .addAll(extraFlags)
+            .add("--graceful_sigterm")
             .addAll(commandArguments)
             .build();
 
     ProcessWrapper processWrapper =
-        new ProcessWrapper(makeProcessWrapperBin("/path/process-wrapper"), killDelay, extraFlags);
+        new ProcessWrapper(
+            makeProcessWrapperBin("/path/process-wrapper"), killDelay, /*gracefulSigterm=*/ true);
 
     List<String> commandLine =
         processWrapper
@@ -100,4 +102,27 @@ public void testProcessWrapperCommandLineBuilder_buildsWithOptionalArguments()
 
     assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder();
   }
+
+  @Test
+  public void testProcessWrapperCommandLineBuilder_withExecutionInfo() throws IOException {
+    ImmutableList<String> commandArguments = ImmutableList.of("echo", "hello, world");
+
+    ProcessWrapper processWrapper =
+        new ProcessWrapper(
+            makeProcessWrapperBin("/some/path"), /*killDelay=*/ null, /*gracefulSigterm=*/ false);
+    ProcessWrapper.CommandLineBuilder builder = processWrapper.commandLineBuilder(commandArguments);
+
+    ImmutableList<String> expectedWithoutExecutionInfo =
+        ImmutableList.<String>builder().add("/some/path").addAll(commandArguments).build();
+    assertThat(builder.build()).containsExactlyElementsIn(expectedWithoutExecutionInfo).inOrder();
+
+    ImmutableList<String> expectedWithExecutionInfo =
+        ImmutableList.<String>builder()
+            .add("/some/path")
+            .add("--graceful_sigterm")
+            .addAll(commandArguments)
+            .build();
+    builder.addExecutionInfo(ImmutableMap.of(ExecutionRequirements.GRACEFUL_TERMINATION, "1"));
+    assertThat(builder.build()).containsExactlyElementsIn(expectedWithExecutionInfo).inOrder();
+  }
 }

src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java

@@ -49,7 +49,7 @@ private ProcessWrapper getProcessWrapper() {
             .getPath(BlazeTestUtils.runfilesDir())
             .getRelative(TestConstants.PROCESS_WRAPPER_PATH),
         /*killDelay=*/ null,
-        /*extraFlags=*/ ImmutableList.of());
+        /*gracefulSigterm=*/ false);
   }
 
   private String getCpuTimeSpenderPath() {