Skip to content

Commit 866d470

Browse files
committed
Kill the entire child process group when cancelling postinstall
Postinstall scripts might spawn child processes. To ensure that all recursive child processes exited, mark the postinstall script as its own process group leader using setpgid(), and kill the entire process group when klling a subprocess. This ensures that no processes would be using postinstall mount dir after cancelling postinstall, allowing us to unmount safely. Test: th Bug: 306296608 Change-Id: I571b96c89e1bcc8eeafae0491754c1f167c4ef2f
1 parent 85a6d99 commit 866d470

File tree

1 file changed

+36
-5
lines changed

1 file changed

+36
-5
lines changed

common/subprocess.cc

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <string.h>
2222
#include <unistd.h>
2323

24+
#include <chrono>
2425
#include <memory>
2526
#include <string>
2627
#include <utility>
@@ -48,6 +49,10 @@ namespace {
4849
bool SetupChild(const std::map<string, string>& env, uint32_t flags) {
4950
// Setup the environment variables.
5051
clearenv();
52+
if (setpgid(0, 0) != 0) {
53+
PLOG(ERROR) << "Failed to setpgid on subprocess " << getpid();
54+
return false;
55+
}
5156
for (const auto& key_value : env) {
5257
setenv(key_value.first.c_str(), key_value.second.c_str(), 0);
5358
}
@@ -147,9 +152,11 @@ void Subprocess::ChildExitedCallback(const siginfo_t& info) {
147152

148153
// Don't print any log if the subprocess exited with exit code 0.
149154
if (info.si_code != CLD_EXITED) {
150-
LOG(INFO) << "Subprocess terminated with si_code " << info.si_code;
155+
LOG(INFO) << "Subprocess " << info.si_pid << " terminated with si_code "
156+
<< info.si_code;
151157
} else if (info.si_status != 0) {
152-
LOG(INFO) << "Subprocess exited with si_status: " << info.si_status;
158+
LOG(INFO) << "Subprocess " << info.si_pid
159+
<< " exited with si_status: " << info.si_status;
153160
}
154161

155162
if (!record->stdout_str.empty()) {
@@ -206,18 +213,42 @@ pid_t Subprocess::ExecFlags(const vector<string>& cmd,
206213
return pid;
207214
}
208215

216+
bool WaitForProcessGroup(pid_t pid, std::chrono::milliseconds timeout) {
217+
using std::chrono::system_clock;
218+
auto start = system_clock::now();
219+
do {
220+
pid_t w = waitpid(-pid, nullptr, WNOHANG);
221+
if (w < 0) {
222+
// When all of the child process with this process group ID exits, waitpid
223+
// will return ECHILD. Until that point, keep callilng waitpid() as there
224+
// might be multiple child processes with the same process group id.
225+
if (errno == ECHILD) {
226+
LOG(INFO) << "All processes with process group id " << pid << " exited";
227+
return true;
228+
}
229+
PLOG(ERROR) << "Waitpid returned " << w;
230+
return false;
231+
}
232+
usleep(100);
233+
} while ((system_clock::now() - start) <= timeout);
234+
LOG(INFO) << "process group " << pid << " did not exit in " << timeout.count()
235+
<< " milliseconds";
236+
return false;
237+
}
238+
209239
void Subprocess::KillExec(pid_t pid) {
240+
using namespace std::chrono_literals;
210241
auto pid_record = subprocess_records_.find(pid);
211242
if (pid_record == subprocess_records_.end())
212243
return;
213244
pid_record->second->callback.Reset();
214245
// We don't care about output/return code, so we use SIGKILL here to ensure it
215246
// will be killed, SIGTERM might lead to leaked subprocess.
216247
CHECK_EQ(pid_record->second->proc.pid(), pid);
217-
if (!pid_record->second->proc.Kill(SIGKILL, 5)) {
218-
PLOG(WARNING) << "Error sending SIGKILL to "
219-
<< pid_record->second->proc.pid();
248+
if (kill(-pid, SIGKILL) != 0) {
249+
PLOG(WARNING) << "Failed to kill subprocess group " << pid;
220250
}
251+
WaitForProcessGroup(pid, 5000ms);
221252
// Release the pid now so we don't try to kill it if Subprocess is destroyed
222253
// before the corresponding ChildExitedCallback() is called.
223254
pid_record->second->proc.Release();

0 commit comments

Comments
 (0)