diff --git a/init/service.cpp b/init/service.cpp index 7c931daf0..1a6474bc4 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -209,17 +209,6 @@ void Service::NotifyStateChange(const std::string& new_state) const { } void Service::KillProcessGroup(int signal) { - // We ignore reporting errors of ESRCH as this commonly happens in the below case, - // 1) Terminate() is called, which sends SIGTERM to the process - // 2) The process successfully exits - // 3) ReapOneProcess() is called, which calls waitpid(-1, ...) which removes the pid entry. - // 4) Reap() is called, which sends SIGKILL, but the pid no longer exists. - // TODO: sigaction for SIGCHLD reports the pid of the exiting process, - // we should do this kill with that pid first before calling waitpid(). - if (kill(-pid_, signal) == -1 && errno != ESRCH) { - PLOG(ERROR) << "kill(" << pid_ << ", " << signal << ") failed"; - } - // If we've already seen a successful result from killProcessGroup*(), then we have removed // the cgroup already and calling these functions a second time will simply result in an error. // This is true regardless of which signal was sent. diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index 27b4065da..f5d4e1c9e 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -27,10 +27,12 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -258,6 +260,12 @@ static int doKillProcessGroupOnce(uid_t uid, int initialPid, int signal) { return -errno; } + // 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; + int ret; pid_t pid; int processes = 0; @@ -269,8 +277,40 @@ static int doKillProcessGroupOnce(uid_t uid, int initialPid, int signal) { 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(pid); + 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) { + PLOG(WARNING) << "kill(" << -pgid << ", " << signal << ") failed"; + } + } + + // Kill remaining pids. + for (const auto pid : pids) { LOG(VERBOSE) << "Killing pid " << pid << " in uid " << uid << " as part of process cgroup " << initialPid; + if (kill(pid, signal) == -1) { PLOG(WARNING) << "kill(" << pid << ", " << signal << ") failed"; }