update_engine: resume suspended PostInstall action to handle termination
PostinstallRunnerActionTest.RunAsRootCancelPostinstallActionTest exposes
an issue when PostinstallRunnerAction tries to terminate an action that
has been suspended. PostinstallRunnerAction::TerminateProcessing() uses
Subprocess::KillExec() to terminate the action by sending SIGTERM to the
child process associated with the action. However, if the action has
been suspended by PostinstallRunnerAction::SuspendAction(), the child
process won't receive the SIGTERM until it's resumed. This CL changes
PostinstallRunnerAction::TerminateProcessing() to resume the child
process after issuing SIGTERM.
BUG=chromium:678643
TEST=Verified that no orphaned 'sleep' process is left after running
PostinstallRunnerActionTest.RunAsRootCancelPostinstallActionTest.
Change-Id: I6852e560550ce2ce49e28733da2b67a7f2e11ca5
Reviewed-on: https://chromium-review.googlesource.com/426878
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index c24590e..dd40485 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -345,6 +345,8 @@
return;
if (kill(current_command_, SIGSTOP) != 0) {
PLOG(ERROR) << "Couldn't pause child process " << current_command_;
+ } else {
+ is_current_command_suspended_ = true;
}
}
@@ -353,6 +355,8 @@
return;
if (kill(current_command_, SIGCONT) != 0) {
PLOG(ERROR) << "Couldn't resume child process " << current_command_;
+ } else {
+ is_current_command_suspended_ = false;
}
}
@@ -362,6 +366,13 @@
// Calling KillExec() will discard the callback we registered and therefore
// the unretained reference to this object.
Subprocess::Get().KillExec(current_command_);
+
+ // If the command has been suspended, resume it after KillExec() so that the
+ // process can process the SIGTERM sent by KillExec().
+ if (is_current_command_suspended_) {
+ ResumeAction();
+ }
+
current_command_ = 0;
Cleanup();
}
diff --git a/payload_consumer/postinstall_runner_action.h b/payload_consumer/postinstall_runner_action.h
index 4cdc47e..26e18a1 100644
--- a/payload_consumer/postinstall_runner_action.h
+++ b/payload_consumer/postinstall_runner_action.h
@@ -138,6 +138,9 @@
// Postinstall command currently running, or 0 if no program running.
pid_t current_command_{0};
+ // True if |current_command_| has been suspended by SuspendAction().
+ bool is_current_command_suspended_{false};
+
// The parent progress file descriptor used to watch for progress reports from
// the postinstall program and the task watching for them.
int progress_fd_{-1};