Send SIGKILL to subprocesses on destructing Command. PiperOrigin-RevId: 881545718
diff --git a/centipede/BUILD b/centipede/BUILD index f9dd983..b01c22c 100644 --- a/centipede/BUILD +++ b/centipede/BUILD
@@ -1675,6 +1675,7 @@ deps = [ ":runner_fork_server", "@abseil-cpp//absl/base:nullability", + "@abseil-cpp//absl/time", ], )
diff --git a/centipede/command.cc b/centipede/command.cc index 3536d93..3071428 100644 --- a/centipede/command.cc +++ b/centipede/command.cc
@@ -185,12 +185,14 @@ // PIMPL). Command::~Command() { if (is_executing()) { - FUZZTEST_LOG(WARNING) - << "Destructing Command object for " << path() << " with " - << (fork_server_ ? absl::StrCat("fork server PID ", fork_server_->pid_) - : absl::StrCat("PID ", pid_)) - << " still running. Requesting it to stop without waiting for it..."; - RequestStop(); + FUZZTEST_LOG(WARNING) << "Destructing Command object for " << path() + << " with " + << (fork_server_ ? absl::StrCat("fork server PID ", + fork_server_->pid_) + : absl::StrCat("PID ", pid_)) + << " still running. Requesting it to force-stop " + "without waiting for it..."; + RequestStop(/*force=*/true); } ResetRedirectionFiles(/*new_suffix=*/""); } @@ -520,15 +522,18 @@ return exit_code; } -void Command::RequestStop() { +void Command::RequestStop(bool force) { FUZZTEST_CHECK(is_executing()); if (fork_server_) { FUZZTEST_CHECK_NE(fork_server_->pid_, -1); - kill(fork_server_->pid_, SIGTERM); + // Cannot send SIGKILL to the fork server as it kills only the parent + // process, but not the child. The fork server would send SIGKILL to the + // child on SIGUSR1. + kill(fork_server_->pid_, force ? SIGUSR1 : SIGTERM); return; } FUZZTEST_CHECK_NE(pid_, -1); - kill(pid_, SIGTERM); + kill(pid_, force ? SIGKILL : SIGTERM); } std::string Command::ReadRedirectedStdout() const {
diff --git a/centipede/command.h b/centipede/command.h index a2fab51..d9edf8d 100644 --- a/centipede/command.h +++ b/centipede/command.h
@@ -96,10 +96,11 @@ // call `RequestEarlyStop()` (see stop.h). std::optional<int> Wait(absl::Time deadline); - // Requests the command execution to stop. Must be called only when the - // command is executing. Note that after calling this, `Wait()` is still - // needed to complete the execution. - void RequestStop(); + // Requests the command execution to stop by sending SIGTERM or SIGKILL (when + // `force` is true). Must be called only when the command is executing. Note + // that after calling this, `Wait()` is still needed to complete the + // execution. + void RequestStop(bool force = false); // Convenient method to execute synchronously. int Execute() {
diff --git a/centipede/command_test.cc b/centipede/command_test.cc index f4aaa36..dc163c9 100644 --- a/centipede/command_test.cc +++ b/centipede/command_test.cc
@@ -24,6 +24,7 @@ #include <string_view> #include <utility> +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/strings/substitute.h" #include "absl/time/clock.h" @@ -35,6 +36,8 @@ namespace fuzztest::internal { namespace { +using ::testing::Optional; + TEST(CommandTest, ToString) { EXPECT_EQ(Command{"x"}.ToString(), "env \\\nx"); { @@ -149,6 +152,24 @@ } { + const std::string input = "sleep"; + const std::string log_prefix = std::filesystem::path{test_tmpdir} / input; + Command::Options cmd_options; + cmd_options.args = {input}; + cmd_options.stdout_file_prefix = log_prefix; + cmd_options.stderr_file_prefix = log_prefix; + Command cmd{helper, std::move(cmd_options)}; + ASSERT_TRUE(cmd.StartForkServer(test_tmpdir, "ForkServer")); + ASSERT_TRUE(cmd.ExecuteAsync()); + EXPECT_EQ(cmd.Wait(absl::Now() + absl::Seconds(2)), std::nullopt); + cmd.RequestStop(/*force=*/false); + EXPECT_THAT(cmd.Wait(absl::Now() + absl::Seconds(2)), Optional(SIGTERM)); + std::string log_contents; + ReadFromLocalFile(cmd.stdout_file(), log_contents); + EXPECT_EQ(log_contents, absl::Substitute("Got input: $0", input)); + } + + { const std::string input = "hang"; const std::string log_prefix = std::filesystem::path{test_tmpdir} / input; Command::Options cmd_options; @@ -159,6 +180,10 @@ ASSERT_TRUE(cmd.StartForkServer(test_tmpdir, "ForkServer")); ASSERT_TRUE(cmd.ExecuteAsync()); EXPECT_EQ(cmd.Wait(absl::Now() + absl::Seconds(2)), std::nullopt); + cmd.RequestStop(/*force=*/false); + EXPECT_EQ(cmd.Wait(absl::Now() + absl::Seconds(2)), std::nullopt); + cmd.RequestStop(/*force=*/true); + EXPECT_THAT(cmd.Wait(absl::Now() + absl::Seconds(2)), Optional(SIGKILL)); std::string log_contents; ReadFromLocalFile(cmd.stdout_file(), log_contents); EXPECT_EQ(log_contents, absl::Substitute("Got input: $0", input));
diff --git a/centipede/command_test_helper.cc b/centipede/command_test_helper.cc index ac5b00f..d3153a8 100644 --- a/centipede/command_test_helper.cc +++ b/centipede/command_test_helper.cc
@@ -15,11 +15,14 @@ #include <unistd.h> #include <cassert> +#include <csignal> #include <cstdio> #include <cstdlib> #include <cstring> #include "absl/base/nullability.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" // A binary linked with the fork server that exits/crashes in different ways. int main(int argc, char** absl_nonnull argv) { @@ -31,6 +34,13 @@ if (!strcmp(argv[1], "ret42")) return 42; if (!strcmp(argv[1], "abort")) abort(); // Sleep longer than kTimeout in CommandDeathTest_ForkServerHangingBinary. - if (!strcmp(argv[1], "hang")) sleep(5); + if (!strcmp(argv[1], "sleep")) absl::SleepFor(absl::Seconds(5)); + if (!strcmp(argv[1], "hang")) { + struct sigaction act{}; + act.sa_handler = [](int) {}; + sigaction(SIGTERM, &act, nullptr); + absl::SleepFor(absl::Seconds(10)); + } + return 17; }
diff --git a/centipede/runner_fork_server.cc b/centipede/runner_fork_server.cc index 359128d..afd7bc2 100644 --- a/centipede/runner_fork_server.cc +++ b/centipede/runner_fork_server.cc
@@ -209,6 +209,9 @@ struct sigaction sigchld_act{}; sigchld_act.sa_handler = [](int) {}; + struct sigaction sigusr1_act{}; + sigusr1_act.sa_handler = [](int) {}; + sigset_t server_sigset; if (sigprocmask(SIG_SETMASK, nullptr, &server_sigset) != 0) { Exit("###sigprocmask() failed to get the existing sigset\n"); @@ -222,6 +225,9 @@ if (sigaddset(&server_sigset, SIGCHLD) != 0) { Exit("###sigaddset() failed to add SIGCHLD\n"); } + if (sigaddset(&server_sigset, SIGUSR1) != 0) { + Exit("###sigaddset() failed to add SIGUSR1\n"); + } sigset_t wait_sigset; if (sigemptyset(&wait_sigset) != 0) { @@ -236,10 +242,14 @@ if (sigaddset(&wait_sigset, SIGCHLD) != 0) { Exit("###sigaddset() failed to add SIGCHLD to the wait sigset\n"); } + if (sigaddset(&wait_sigset, SIGUSR1) != 0) { + Exit("###sigaddset() failed to add SIGUSR1 to the wait sigset\n"); + } struct sigaction old_sigint_act{}; struct sigaction old_sigterm_act{}; struct sigaction old_sigchld_act{}; + struct sigaction old_sigusr1_act{}; sigset_t old_sigset; bool to_restore_signal_handling = false; // Loop. @@ -258,7 +268,7 @@ Exit("###sigpending() failed\n"); } if (sigismember(&pending, SIGINT) || sigismember(&pending, SIGTERM) || - sigismember(&pending, SIGCHLD)) { + sigismember(&pending, SIGCHLD) || sigismember(&pending, SIGUSR1)) { int unused_sig; if (sigwait(&wait_sigset, &unused_sig) != 0) { Exit("###sigwait() failed\n"); @@ -276,6 +286,9 @@ if (sigaction(SIGCHLD, &old_sigchld_act, nullptr) != 0) { Exit("###sigaction failed on SIGCHLD for the child"); } + if (sigaction(SIGUSR1, &old_sigusr1_act, nullptr) != 0) { + Exit("###sigaction failed on SIGUSR1 for the child"); + } if (sigprocmask(SIG_SETMASK, &old_sigset, nullptr) != 0) { Exit("###sigprocmask() failed to restore the previous sigset\n"); } @@ -303,6 +316,9 @@ if (sigaction(SIGCHLD, &sigchld_act, &old_sigchld_act) != 0) { Exit("###sigaction failed on SIGCHLD for the fork server"); } + if (sigaction(SIGUSR1, &sigusr1_act, &old_sigusr1_act) != 0) { + Exit("###sigaction failed on SIGUSR1 for the fork server"); + } if (sigprocmask(SIG_SETMASK, &server_sigset, &old_sigset) != 0) { Exit("###sigprocmask() failed to set the fork server sigset\n"); } @@ -335,6 +351,9 @@ } else if (sig == SIGTERM) { Log("###Got SIGTERM - forwarding to the child\n"); kill(pid, SIGTERM); + } else if (sig == SIGUSR1) { + Log("###Got SIGUSR1 - sending SIGKILL to the child\n"); + kill(pid, SIGKILL); } else if (sig == SIGCHLD) { Log("###Got SIGCHLD - reaping the child\n"); const pid_t ret = waitpid(pid, &status, WNOHANG);