diff --git a/init/service.cpp b/init/service.cpp index 2ce81a0ad..78bf42ffa 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -712,8 +712,6 @@ Result Service::Start() { if (use_memcg) { ConfigureMemcg(); } - } else { - process_cgroup_empty_ = true; } if (oom_score_adjust_ != DEFAULT_OOM_SCORE_ADJUST) { diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index 3fac373cb..45ac99cfe 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -367,59 +367,70 @@ err: // 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 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 pgids; pgids.emplace(initialPid); std::set pids; - pid_t pid; + std::unique_ptr 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 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, 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 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries, << " in " << static_cast(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);