init: always kill oneshot services' process groups.
~2007 a change was added that would allow oneshot services to
daemonize by not killing their process group. This was a hack at the
time, and should certainly not be needed now. I've resisted removing
the behavior however, as it hadn't caused any issues.
Recently, it was detected that the cgroups that these processes belong
to, would exist forever and therefore leak memory. Instead of simply
removing the cgroups when empty, this provides a good opportunity to
do the right thing and fix this behavior once and for all.
The new (correct) behavior only happens for devices with vendor images
built for Android R or later. Init will log a warning to dmesg when
it detects this difference in behavior has occurred.
Bug: 144545923
Test: boot CF/Coral and see no difference in behavior.
Test: boot CF with a service that daemonizes and see the warning.
Change-Id: I333a2e25a541ec0114ac50ab8ae7f1ea3f055447
diff --git a/init/service.cpp b/init/service.cpp
index 574ff52..ad42df7 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -42,9 +42,11 @@
#if defined(__ANDROID__)
#include <ApexProperties.sysprop.h>
+#include <android/api-level.h>
#include "mount_namespace.h"
#include "property_service.h"
+#include "selinux.h"
#else
#include "host_init_stubs.h"
#endif
@@ -182,7 +184,7 @@
}
}
-void Service::KillProcessGroup(int signal) {
+void Service::KillProcessGroup(int signal, bool report_oneshot) {
// 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.
@@ -190,11 +192,20 @@
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);
+ r = killProcessGroupOnce(proc_attr_.uid, pid_, signal, &max_processes);
} else {
- r = killProcessGroup(proc_attr_.uid, pid_, signal);
+ 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.";
}
if (r == 0) process_cgroup_empty_ = true;
@@ -244,7 +255,16 @@
void Service::Reap(const siginfo_t& siginfo) {
if (!(flags_ & SVC_ONESHOT) || (flags_ & SVC_RESTART)) {
- KillProcessGroup(SIGKILL);
+ 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);
+ }
}
// Remove any socket resources we may have created.