Kill services even when cgroups is disabled
process_cgroup_empty_ is used to indicate that a service is already
killed or not. If cgroup support lacks, services cannot be killed
because process_cgroup_empty_ is always true.
This change fixes it by not assigning process_cgroup_empty_ as true.
Instead, make KillProcessGroup send signals even when cgroup is
disabled. Also DoKillProcessGroupOnce() is updated so it returns a number of killed processes, excluding already dead processes. This behavior agrees with its name (DoKillProcessOnce), and it prevents regression upon missing cgroups, because kill(-pgid) will always
"succeed" so KillProcessGroup will loop even when all processes are
already dead.
Bug: 257264124
Test: boot microdroid, see services are terminated
Change-Id: I19abf19ff1b70c666cd6f12d0a12956765174aaa
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 3fac373..45ac99c 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -367,59 +367,70 @@
// Returns 0 if there are no processes in the process cgroup left to kill
// Returns -1 on error
static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, int signal) {
- auto path = ConvertUidPidToPath(cgroup, uid, initialPid) + PROCESSGROUP_CGROUP_PROCS_FILE;
- std::unique_ptr<FILE, decltype(&fclose)> fd(fopen(path.c_str(), "re"), fclose);
- if (!fd) {
- if (errno == ENOENT) {
- // This happens when process is already dead
- return 0;
- }
- PLOG(WARNING) << __func__ << " failed to open process cgroup uid " << uid << " pid "
- << initialPid;
- return -1;
- }
-
// We separate all of the pids in the cgroup into those pids that are also the leaders of
// process groups (stored in the pgids set) and those that are not (stored in the pids set).
std::set<pid_t> pgids;
pgids.emplace(initialPid);
std::set<pid_t> pids;
- pid_t pid;
+ std::unique_ptr<FILE, decltype(&fclose)> fd(nullptr, fclose);
+
+ if (CgroupsAvailable()) {
+ auto path = ConvertUidPidToPath(cgroup, uid, initialPid) + PROCESSGROUP_CGROUP_PROCS_FILE;
+ fd.reset(fopen(path.c_str(), "re"));
+ if (!fd) {
+ if (errno == ENOENT) {
+ // This happens when process is already dead
+ return 0;
+ }
+ PLOG(WARNING) << __func__ << " failed to open process cgroup uid " << uid << " pid "
+ << initialPid;
+ return -1;
+ }
+ pid_t pid;
+ bool file_is_empty = true;
+ while (fscanf(fd.get(), "%d\n", &pid) == 1 && pid >= 0) {
+ file_is_empty = false;
+ if (pid == 0) {
+ // Should never happen... but if it does, trying to kill this
+ // will boomerang right back and kill us! Let's not let that happen.
+ LOG(WARNING)
+ << "Yikes, we've been told to kill pid 0! How about we don't do that?";
+ continue;
+ }
+ pid_t pgid = getpgid(pid);
+ if (pgid == -1) PLOG(ERROR) << "getpgid(" << pid << ") failed";
+ if (pgid == pid) {
+ pgids.emplace(pid);
+ } else {
+ pids.emplace(pid);
+ }
+ }
+ if (file_is_empty) {
+ // This happens when process is already dead
+ return 0;
+ }
+
+ // Erase all pids that will be killed when we kill the process groups.
+ for (auto it = pids.begin(); it != pids.end();) {
+ pid_t pgid = getpgid(*it);
+ if (pgids.count(pgid) == 1) {
+ it = pids.erase(it);
+ } else {
+ ++it;
+ }
+ }
+ }
+
int processes = 0;
- while (fscanf(fd.get(), "%d\n", &pid) == 1 && pid >= 0) {
- processes++;
- if (pid == 0) {
- // Should never happen... but if it does, trying to kill this
- // will boomerang right back and kill us! Let's not let that happen.
- LOG(WARNING) << "Yikes, we've been told to kill pid 0! How about we don't do that?";
- continue;
- }
- pid_t pgid = getpgid(pid);
- if (pgid == -1) PLOG(ERROR) << "getpgid(" << pid << ") failed";
- if (pgid == pid) {
- pgids.emplace(pid);
- } else {
- pids.emplace(pid);
- }
- }
-
- // Erase all pids that will be killed when we kill the process groups.
- for (auto it = pids.begin(); it != pids.end();) {
- pid_t pgid = getpgid(*it);
- if (pgids.count(pgid) == 1) {
- it = pids.erase(it);
- } else {
- ++it;
- }
- }
-
// Kill all process groups.
for (const auto pgid : pgids) {
LOG(VERBOSE) << "Killing process group " << -pgid << " in uid " << uid
<< " as part of process cgroup " << initialPid;
- if (kill(-pgid, signal) == -1 && errno != ESRCH) {
+ if (kill(-pgid, signal) == 0) {
+ processes++;
+ } else if (errno != ESRCH) {
PLOG(WARNING) << "kill(" << -pgid << ", " << signal << ") failed";
}
}
@@ -429,18 +440,22 @@
LOG(VERBOSE) << "Killing pid " << pid << " in uid " << uid << " as part of process cgroup "
<< initialPid;
- if (kill(pid, signal) == -1 && errno != ESRCH) {
+ if (kill(pid, signal) == 0) {
+ processes++;
+ } else if (errno != ESRCH) {
PLOG(WARNING) << "kill(" << pid << ", " << signal << ") failed";
}
}
- return feof(fd.get()) ? processes : -1;
+ return (!fd || feof(fd.get())) ? processes : -1;
}
static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries,
int* max_processes) {
std::string hierarchy_root_path;
- CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, &hierarchy_root_path);
+ if (CgroupsAvailable()) {
+ CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, &hierarchy_root_path);
+ }
const char* cgroup = hierarchy_root_path.c_str();
std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
@@ -485,6 +500,11 @@
<< " in " << static_cast<int>(ms) << "ms";
}
+ if (!CgroupsAvailable()) {
+ // nothing to do here, if cgroups isn't available
+ return 0;
+ }
+
// 400 retries correspond to 2 secs max timeout
int err = RemoveProcessGroup(cgroup, uid, initialPid, 400);