Use process groups to manage artd's subprocesses.

This change adds a new option `new_process_group` to ExecUtils, and this
option is only used by artd to manage its subprocesses. artd creates a
process group for each child process and its descendants, and it uses
`kill(-pid, SIGKILL)` to kill the whole process group upon job
cancellation.

Before this change, ExecUtils always creates a new process group for a
child process. This behavior can be dated back to
https://android.googlesource.com/platform/art/+/5643b786dd48df1f4b89a39e6024a22d032c79d4%5E%21/.
A comment on the `setpgid` call says: "change process groups, so we
don't get reaped by ProcessManager".

We cannot find out why changing the process group was needed or how
process reaping was related to process groups at that time because there
isn't such a thing called "ProcessManager" in Android anymore. However,
we don't need this behavior today because the usage of the code that
does fork and exec has completely changed.

At the time where the original CL was submitted, the code was for the
runtime to generate a persistent boot image on /data. Today, the runtime
never generates a persistent boot image because it's considered insecure.

Today, ExecUtils is mostly used in tests. Other than tests, it's only
used in four places: Zygote, odrefresh, artd, and dexopt_chroot_setup.

- Zygote uses it to call dex2oat to create an in-memory boot image. It
  doesn't make sense to keep dex2oat running when Zygote (the memfd
  owner) is killed.
- odrefresh uses it to call dex2oat to create persistent boot images. We
  do want to kill dex2oat when killing odrefresh.
- artd uses it to call dex2oat, profman, derive_classpath, and
  odrefresh. It sets `new_process_group` to true, so it's behavior
  regarding process groups is unchanged. It's a service managed by
  the service manager. The service manager uses cgroups, not process
  grups, to manage and kill processes, so creating new process groups
  for artd's children is fine.
- dexopt_chroot_setup uses it to call apexd and linkerconfig. We do want
  to kill apexd and linkerconfig when dexopt_chroot_setup is killed.

Therefore, changing this behavior is fine.

After this change, ExecUtils no longer creates a new process group for a
child process unless `new_process_group` is set to true.

Bug: 345723405
Bug: 311377497
Test: m test-art-host-gtest-art_runtime_tests
Test: m test-art-host-gtest-art_artd_tests
Test: -
  1. adb shell pm compile -m speed -f -v com.android.settings
  2. adb shell pm art cancel <job-id>
  3. No change to the existing behavior: dex2oat is still killed.
Test: -
  1. adb shell pm art pr-dexopt-job --run
  2. Press Ctrl+C.
  3. Both odrefresh and dex2oat are killed.
Change-Id: I3058fc68a6bd5f98617422c1eb0861648b144a51

1 files changed, 14 insertions, 5 deletions

diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 7e04cced92..05a7cc6bdc 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -62,7 +62,9 @@ std::string ToCommandLine(const std::vector<std::string>& args) {
 // If there is a runtime (Runtime::Current != nullptr) then the subprocess is created with the
 // same environment that existed when the runtime was started.
 // Returns the process id of the child process on success, -1 otherwise.
-pid_t ExecWithoutWait(const std::vector<std::string>& arg_vector, std::string* error_msg) {
+pid_t ExecWithoutWait(const std::vector<std::string>& arg_vector,
+                      bool new_process_group,
+                      std::string* error_msg) {
   // Convert the args to char pointers.
   const char* program = arg_vector[0].c_str();
   std::vector<char*> args;
@@ -77,8 +79,9 @@ pid_t ExecWithoutWait(const std::vector<std::string>& arg_vector, std::string* e
   if (pid == 0) {
     // no allocation allowed between fork and exec
 
-    // change process groups, so we don't get reaped by ProcessManager
-    setpgid(0, 0);
+    if (new_process_group) {
+      setpgid(0, 0);
+    }
 
     // (b/30160149): protect subprocesses from modifications to LD_LIBRARY_PATH, etc.
     // Use the snapshot of the environment from the time the runtime was created.
@@ -250,12 +253,18 @@ int ExecUtils::ExecAndReturnCode(const std::vector<std::string>& arg_vector,
 ExecResult ExecUtils::ExecAndReturnResult(const std::vector<std::string>& arg_vector,
                                           int timeout_sec,
                                           std::string* error_msg) const {
-  return ExecAndReturnResult(arg_vector, timeout_sec, ExecCallbacks(), /*stat=*/nullptr, error_msg);
+  return ExecAndReturnResult(arg_vector,
+                             timeout_sec,
+                             ExecCallbacks(),
+                             /*new_process_group=*/false,
+                             /*stat=*/nullptr,
+                             error_msg);
 }
 
 ExecResult ExecUtils::ExecAndReturnResult(const std::vector<std::string>& arg_vector,
                                           int timeout_sec,
                                           const ExecCallbacks& callbacks,
+                                          bool new_process_group,
                                           /*out*/ ProcessStat* stat,
                                           /*out*/ std::string* error_msg) const {
   if (timeout_sec > INT_MAX / 1000) {
@@ -264,7 +273,7 @@ ExecResult ExecUtils::ExecAndReturnResult(const std::vector<std::string>& arg_ve
   }
 
   // Start subprocess.
-  pid_t pid = ExecWithoutWait(arg_vector, error_msg);
+  pid_t pid = ExecWithoutWait(arg_vector, new_process_group, error_msg);
   if (pid == -1) {
     return {.status = ExecResult::kStartFailed};
   }