diff --git a/init/service.cpp b/init/service.cpp index 5e900ee4d..208745257 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -194,7 +194,7 @@ void Service::NotifyStateChange(const std::string& new_state) const { } } -void Service::KillProcessGroup(int signal, bool report_oneshot) { +void Service::KillProcessGroup(int signal) { // 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. @@ -202,20 +202,11 @@ void Service::KillProcessGroup(int signal, bool report_oneshot) { if (!process_cgroup_empty_) { LOG(INFO) << "Sending signal " << signal << " to service '" << name_ << "' (pid " << pid_ << ") process group..."; - int max_processes = 0; int r; if (signal == SIGTERM) { - r = killProcessGroupOnce(uid(), pid_, signal, &max_processes); + r = killProcessGroupOnce(uid(), pid_, signal); } else { - r = killProcessGroup(uid(), pid_, signal, &max_processes); - } - - if (report_oneshot && max_processes > 0) { - LOG(WARNING) - << "Killed " << max_processes - << " additional processes from a oneshot process group for service '" << name_ - << "'. This is new behavior, previously child processes would not be killed in " - "this case."; + r = killProcessGroup(uid(), pid_, signal); } if (r == 0) process_cgroup_empty_ = true; @@ -265,7 +256,7 @@ void Service::SetProcessAttributesAndCaps(InterprocessFifo setsid_finished) { void Service::Reap(const siginfo_t& siginfo) { if (!(flags_ & SVC_ONESHOT) || (flags_ & SVC_RESTART)) { - KillProcessGroup(SIGKILL, false); + KillProcessGroup(SIGKILL); } else { // Legacy behavior from ~2007 until Android R: this else branch did not exist and we did not // kill the process group in this case. @@ -273,7 +264,7 @@ void Service::Reap(const siginfo_t& siginfo) { // The new behavior in Android R is to kill these process groups in all cases. The // 'true' parameter instructions KillProcessGroup() to report a warning message where it // detects a difference in behavior has occurred. - KillProcessGroup(SIGKILL, true); + KillProcessGroup(SIGKILL); } } diff --git a/init/service.h b/init/service.h index 9f09cefb8..35521b207 100644 --- a/init/service.h +++ b/init/service.h @@ -160,7 +160,7 @@ class Service { private: void NotifyStateChange(const std::string& new_state) const; void StopOrReset(int how); - void KillProcessGroup(int signal, bool report_oneshot = false); + void KillProcessGroup(int signal); void SetProcessAttributesAndCaps(InterprocessFifo setsid_finished); void ResetFlagsForStart(); Result CheckConsole(); diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h index dbaeb9397..5c43224ab 100644 --- a/libprocessgroup/include/processgroup/processgroup.h +++ b/libprocessgroup/include/processgroup/processgroup.h @@ -67,14 +67,11 @@ void DropTaskProfilesResourceCaching(); // Return 0 and removes the cgroup if there are no longer any processes in it. // Returns -1 in the case of an error occurring or if there are processes still running // even after retrying for up to 200ms. -// If max_processes is not nullptr, it returns the maximum number of processes seen in the cgroup -// during the killing process. Note that this can be 0 if all processes from the process group have -// already been terminated. -int killProcessGroup(uid_t uid, int initialPid, int signal, int* max_processes = nullptr); +int killProcessGroup(uid_t uid, int initialPid, int signal); // Returns the same as killProcessGroup(), however it does not retry, which means // that it only returns 0 in the case that the cgroup exists and it contains no processes. -int killProcessGroupOnce(uid_t uid, int initialPid, int signal, int* max_processes = nullptr); +int killProcessGroupOnce(uid_t uid, int initialPid, int signal); // Sends the provided signal to all members of a process group, but does not wait for processes to // exit, or for the cgroup to be removed. Callers should also ensure that killProcessGroup is called diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index cc2565fad..3fa43f8ad 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -455,8 +455,7 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid, return (!fd || feof(fd.get())) ? processes : -1; } -static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries, - int* max_processes) { +static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries) { CHECK_GE(uid, 0); CHECK_GT(initialPid, 0); @@ -468,16 +467,9 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries, std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - if (max_processes != nullptr) { - *max_processes = 0; - } - int retry = retries; int processes; while ((processes = DoKillProcessGroupOnce(cgroup, uid, initialPid, signal)) > 0) { - if (max_processes != nullptr && processes > *max_processes) { - *max_processes = processes; - } LOG(VERBOSE) << "Killed " << processes << " processes for processgroup " << initialPid; if (!CgroupsAvailable()) { // makes no sense to retry, because there are no cgroup_procs file @@ -540,12 +532,12 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries, } } -int killProcessGroup(uid_t uid, int initialPid, int signal, int* max_processes) { - return KillProcessGroup(uid, initialPid, signal, 40 /*retries*/, max_processes); +int killProcessGroup(uid_t uid, int initialPid, int signal) { + return KillProcessGroup(uid, initialPid, signal, 40 /*retries*/); } -int killProcessGroupOnce(uid_t uid, int initialPid, int signal, int* max_processes) { - return KillProcessGroup(uid, initialPid, signal, 0 /*retries*/, max_processes); +int killProcessGroupOnce(uid_t uid, int initialPid, int signal) { + return KillProcessGroup(uid, initialPid, signal, 0 /*retries*/); } int sendSignalToProcessGroup(uid_t uid, int initialPid, int signal) {