Skip to content

Commit

Permalink
Revert "init: always kill oneshot services' process groups."
Browse files Browse the repository at this point in the history
This reverts commit d89ed13.

Change-Id: Ife6873c3d3ffac572241e682adc873986885fa15
  • Loading branch information
rINanDO committed May 3, 2021
1 parent 8ca6344 commit 43d2ee2
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 43 deletions.
29 changes: 5 additions & 24 deletions init/service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@

#ifdef INIT_FULL_SOURCES
#include <ApexProperties.sysprop.h>
#include <android/api-level.h>

#include "mount_namespace.h"
#include "selinux.h"
#include "property_service.h"
#else
#include "host_init_stubs.h"
#endif
Expand Down Expand Up @@ -190,28 +189,19 @@ 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.
// These functions handle their own logging, so no additional logging is needed.
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(proc_attr_.uid, pid_, signal, &max_processes);
r = killProcessGroupOnce(proc_attr_.uid, pid_, signal);
} else {
r = killProcessGroup(proc_attr_.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(proc_attr_.uid, pid_, signal);
}

if (r == 0) process_cgroup_empty_ = true;
Expand Down Expand Up @@ -261,16 +251,7 @@ void Service::SetProcessAttributesAndCaps() {

void Service::Reap(const siginfo_t& siginfo) {
if (!(flags_ & SVC_ONESHOT) || (flags_ & SVC_RESTART)) {
KillProcessGroup(SIGKILL, false);
} 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.
if (SelinuxGetVendorAndroidVersion() >= __ANDROID_API_R__) {
// 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);
}

// Remove any socket resources we may have created.
Expand Down
2 changes: 1 addition & 1 deletion init/service.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,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();

static unsigned long next_start_order_;
Expand Down
7 changes: 2 additions & 5 deletions libprocessgroup/include/processgroup/processgroup.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,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);

int createProcessGroup(uid_t uid, int initialPid, bool memControl = false);

Expand Down
18 changes: 5 additions & 13 deletions libprocessgroup/processgroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid,
return 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) {
std::string cpuacct_path;
std::string memory_path;

Expand All @@ -316,16 +315,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 (retry > 0) {
std::this_thread::sleep_for(5ms);
Expand Down Expand Up @@ -366,12 +358,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 createProcessGroup(uid_t uid, int initialPid, bool memControl) {
Expand Down

0 comments on commit 43d2ee2

Please sign in to comment.