Fix ExecUtils.WaitChildWithTimeoutFallback by adding a fallback.

`WaitChildWithTimeout` didn't work with old Linux kernels because it
uses a syscall `pidfd_open`, which was added in Linux 5.4.

This change adds a fallback implementation of `WaitChildWithTimeout`
that creates a thread to wait instead of relying on `pidfd_open`.

Also:
- Use android::base::unique_fd to hold pidfd so that the fd is
  guaranteed to be closed.
- Allow timeout_sec to be negative, in which case it blocks until the
  subprocess exits.

Bug: 229268202
Test: m test-art-host-gtest-art_runtime_tests
Change-Id: Iec3f21db135d9a8a3a915372253f1ff3e285e5cf
Merged-In: Iec3f21db135d9a8a3a915372253f1ff3e285e5cf
(cherry picked from commit 99bd8dd2bf8eac1a60bf65e442462753ed7f2147)

1 files changed, 121 insertions, 47 deletions

diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 6e2a5b40f0..722f70ac06 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -21,19 +21,24 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-#include "base/macros.h"
-
 #ifdef __BIONIC__
 #include <sys/pidfd.h>
 #endif
 
+#include <chrono>
+#include <climits>
+#include <condition_variable>
 #include <cstdint>
+#include <mutex>
 #include <string>
+#include <thread>
 #include <vector>
 
 #include "android-base/scopeguard.h"
 #include "android-base/stringprintf.h"
 #include "android-base/strings.h"
+#include "android-base/unique_fd.h"
+#include "base/macros.h"
 #include "runtime.h"
 
 namespace art {
@@ -41,6 +46,7 @@ namespace art {
 namespace {
 
 using ::android::base::StringPrintf;
+using ::android::base::unique_fd;
 
 std::string ToCommandLine(const std::vector<std::string>& args) {
   return android::base::Join(args, ' ');
@@ -88,31 +94,86 @@ pid_t ExecWithoutWait(const std::vector<std::string>& arg_vector, std::string* e
   }
 }
 
-int WaitChild(pid_t pid, const std::vector<std::string>& arg_vector, std::string* error_msg) {
-  int status = -1;
-  pid_t got_pid = TEMP_FAILURE_RETRY(waitpid(pid, &status, 0));
-  if (got_pid != pid) {
-    *error_msg =
-        StringPrintf("Failed to execute (%s) because waitpid failed: wanted %d, got %d: %s",
-                     ToCommandLine(arg_vector).c_str(),
-                     pid,
-                     got_pid,
-                     strerror(errno));
+int WaitChild(pid_t pid,
+              const std::vector<std::string>& arg_vector,
+              bool no_wait,
+              std::string* error_msg) {
+  siginfo_t info;
+  // WNOWAIT leaves the child in a waitable state. The call is still blocking.
+  int options = WEXITED | (no_wait ? WNOWAIT : 0);
+  if (TEMP_FAILURE_RETRY(waitid(P_PID, pid, &info, options)) != 0) {
+    *error_msg = StringPrintf("Failed to execute (%s) because waitid failed for pid %d: %s",
+                              ToCommandLine(arg_vector).c_str(),
+                              pid,
+                              strerror(errno));
     return -1;
   }
-  if (!WIFEXITED(status)) {
+  if (info.si_pid != pid) {
+    *error_msg = StringPrintf("Failed to execute (%s) because waitid failed: wanted %d, got %d: %s",
+                              ToCommandLine(arg_vector).c_str(),
+                              pid,
+                              info.si_pid,
+                              strerror(errno));
+    return -1;
+  }
+  if (info.si_code != CLD_EXITED) {
     *error_msg =
         StringPrintf("Failed to execute (%s) because the child process is terminated by signal %d",
                      ToCommandLine(arg_vector).c_str(),
-                     WTERMSIG(status));
+                     info.si_status);
+    return -1;
+  }
+  return info.si_status;
+}
+
+int WaitChild(pid_t pid, const std::vector<std::string>& arg_vector, std::string* error_msg) {
+  return WaitChild(pid, arg_vector, /*no_wait=*/false, error_msg);
+}
+
+// A fallback implementation of `WaitChildWithTimeout` that creates a thread to wait instead of
+// relying on `pidfd_open`.
+int WaitChildWithTimeoutFallback(pid_t pid,
+                                 const std::vector<std::string>& arg_vector,
+                                 int timeout_ms,
+                                 bool* timed_out,
+                                 std::string* error_msg) {
+  bool child_exited = false;
+  std::condition_variable cv;
+  std::mutex m;
+
+  std::thread wait_thread([&]() {
+    std::unique_lock<std::mutex> lock(m);
+    if (!cv.wait_for(lock, std::chrono::milliseconds(timeout_ms), [&] { return child_exited; })) {
+      *timed_out = true;
+      *error_msg =
+          StringPrintf("Child process %d timed out after %dms. Killing it", pid, timeout_ms);
+      kill(pid, SIGKILL);
+    }
+  });
+
+  // Leave the child in a waitable state just in case `wait_thread` sends a `SIGKILL` after the
+  // child exits.
+  std::string ignored_error_msg;
+  WaitChild(pid, arg_vector, /*no_wait=*/true, &ignored_error_msg);
+
+  {
+    std::unique_lock<std::mutex> lock(m);
+    child_exited = true;
+  }
+  cv.notify_all();
+  wait_thread.join();
+
+  if (*timed_out) {
+    WaitChild(pid, arg_vector, &ignored_error_msg);
     return -1;
   }
-  return WEXITSTATUS(status);
+  return WaitChild(pid, arg_vector, error_msg);
 }
 
 int WaitChildWithTimeout(pid_t pid,
+                         unique_fd pidfd,
                          const std::vector<std::string>& arg_vector,
-                         int timeout_sec,
+                         int timeout_ms,
                          bool* timed_out,
                          std::string* error_msg) {
   auto cleanup = android::base::make_scope_guard([&]() {
@@ -121,24 +182,12 @@ int WaitChildWithTimeout(pid_t pid,
     WaitChild(pid, arg_vector, &ignored_error_msg);
   });
 
-#ifdef __BIONIC__
-  int pidfd = pidfd_open(pid, /*flags=*/0);
-#else
-  // There is no glibc wrapper for pidfd_open.
-  constexpr int SYS_pidfd_open = 434;
-  int pidfd = syscall(SYS_pidfd_open, pid, /*flags=*/0);
-#endif
-  if (pidfd < 0) {
-    *error_msg = StringPrintf("pidfd_open failed for pid %d: %s", pid, strerror(errno));
-    return -1;
-  }
-
   struct pollfd pfd;
-  pfd.fd = pidfd;
+  pfd.fd = pidfd.get();
   pfd.events = POLLIN;
-  int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_sec * 1000));
+  int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_ms));
 
-  close(pidfd);
+  pidfd.reset();
 
   if (poll_ret < 0) {
     *error_msg = StringPrintf("poll failed for pid %d: %s", pid, strerror(errno));
@@ -146,7 +195,7 @@ int WaitChildWithTimeout(pid_t pid,
   }
   if (poll_ret == 0) {
     *timed_out = true;
-    *error_msg = StringPrintf("Child process %d timed out after %ds. Killing it", pid, timeout_sec);
+    *error_msg = StringPrintf("Child process %d timed out after %dms. Killing it", pid, timeout_ms);
     return -1;
   }
 
@@ -156,23 +205,23 @@ int WaitChildWithTimeout(pid_t pid,
 
 }  // namespace
 
-int ExecAndReturnCode(const std::vector<std::string>& arg_vector, std::string* error_msg) {
-  // Start subprocess.
-  pid_t pid = ExecWithoutWait(arg_vector, error_msg);
-  if (pid == -1) {
-    return -1;
-  }
-
-  // Wait for subprocess to finish.
-  return WaitChild(pid, arg_vector, error_msg);
+int ExecUtils::ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                                 std::string* error_msg) const {
+  bool ignored_timed_out;
+  return ExecAndReturnCode(arg_vector, /*timeout_sec=*/-1, &ignored_timed_out, error_msg);
 }
 
-int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
-                      int timeout_sec,
-                      bool* timed_out,
-                      std::string* error_msg) {
+int ExecUtils::ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                                 int timeout_sec,
+                                 bool* timed_out,
+                                 std::string* error_msg) const {
   *timed_out = false;
 
+  if (timeout_sec > INT_MAX / 1000) {
+    *error_msg = "Timeout too large";
+    return -1;
+  }
+
   // Start subprocess.
   pid_t pid = ExecWithoutWait(arg_vector, error_msg);
   if (pid == -1) {
@@ -180,10 +229,23 @@ int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
   }
 
   // Wait for subprocess to finish.
-  return WaitChildWithTimeout(pid, arg_vector, timeout_sec, timed_out, error_msg);
+  if (timeout_sec >= 0) {
+    unique_fd pidfd = PidfdOpen(pid);
+    if (pidfd.get() >= 0) {
+      return WaitChildWithTimeout(
+          pid, std::move(pidfd), arg_vector, timeout_sec * 1000, timed_out, error_msg);
+    } else {
+      LOG(DEBUG) << StringPrintf(
+          "pidfd_open failed for pid %d: %s, falling back", pid, strerror(errno));
+      return WaitChildWithTimeoutFallback(
+          pid, arg_vector, timeout_sec * 1000, timed_out, error_msg);
+    }
+  } else {
+    return WaitChild(pid, arg_vector, error_msg);
+  }
 }
 
-bool Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) {
+bool ExecUtils::Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) const {
   int status = ExecAndReturnCode(arg_vector, error_msg);
   if (status < 0) {
     // Internal error. The error message is already set.
@@ -198,4 +260,16 @@ bool Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) {
   return true;
 }
 
+unique_fd ExecUtils::PidfdOpen(pid_t pid) const {
+#ifdef __BIONIC__
+  return unique_fd(pidfd_open(pid, /*flags=*/0));
+#else
+  // There is no glibc wrapper for pidfd_open.
+#ifndef SYS_pidfd_open
+  constexpr int SYS_pidfd_open = 434;
+#endif
+  return unique_fd(syscall(SYS_pidfd_open, pid, /*flags=*/0));
+#endif
+}
+
 }  // namespace art