Merge "init: move killing of process groups to libprocessgroup"
am: fbbb3bd49a

Change-Id: I44ce77f50905b209d24dee592eef28325c2a409d
diff --git a/init/service.cpp b/init/service.cpp
index 7c931da..1a6474b 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -209,17 +209,6 @@
 }
 
 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 27b4065..f5d4e1c 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -27,10 +27,12 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <unistd.h>
 
 #include <chrono>
 #include <memory>
 #include <mutex>
+#include <set>
 #include <thread>
 
 #include <android-base/logging.h>
@@ -258,6 +260,12 @@
         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<pid_t> pgids;
+    pgids.emplace(initialPid);
+    std::set<pid_t> pids;
+
     int ret;
     pid_t pid;
     int processes = 0;
@@ -269,8 +277,40 @@
             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";
         }