Revert^2 "Update ExecUtils to support artd use cases."

This reverts commit ff95ad517297384d4a2ea589490019f90d637e17.

Reason for revert: Fix available (aosp/2148047)

Test: Presubmit
Change-Id: I952960949267a7b6e63cd7966a347cb1f3374b85
Merged-In: I375034162cded6fdf09ee4d4cd783a0d3987af49
diff --git a/odrefresh/odrefresh_test.cc b/odrefresh/odrefresh_test.cc
index 2457da5..d28c0d8 100644
--- a/odrefresh/odrefresh_test.cc
+++ b/odrefresh/odrefresh_test.cc
@@ -71,14 +71,14 @@
  public:
   // A workaround to avoid MOCK_METHOD on a method with an `std::string*` parameter, which will lead
   // to a conflict between gmock and android-base/logging.h (b/132668253).
-  int ExecAndReturnCode(std::vector<std::string>& arg_vector,
-                        time_t,
+  int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                        int,
                         bool*,
                         std::string*) const override {
     return DoExecAndReturnCode(arg_vector);
   }
 
-  MOCK_METHOD(int, DoExecAndReturnCode, (std::vector<std::string> & arg_vector), (const));
+  MOCK_METHOD(int, DoExecAndReturnCode, (const std::vector<std::string>& arg_vector), (const));
 };
 
 // Matches a flag that starts with `flag` and is a colon-separated list that contains an element
diff --git a/runtime/Android.bp b/runtime/Android.bp
index c5cd7c5..809445b 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -849,6 +849,9 @@
         "libunwindstack",
         "libziparchive",
     ],
+    static_libs: [
+        "libgmock",
+    ],
     header_libs: [
         "art_cmdlineparser_headers", // For parsed_options_test.
     ],
diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 463d458..6e2a5b4 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -16,22 +16,32 @@
 
 #include "exec_utils.h"
 
+#include <poll.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <unistd.h>
+
+#include "base/macros.h"
+
+#ifdef __BIONIC__
+#include <sys/pidfd.h>
+#endif
+
+#include <cstdint>
 #include <string>
 #include <vector>
 
+#include "android-base/scopeguard.h"
 #include "android-base/stringprintf.h"
 #include "android-base/strings.h"
-
 #include "runtime.h"
 
 namespace art {
 
-using android::base::StringPrintf;
-
 namespace {
 
+using ::android::base::StringPrintf;
+
 std::string ToCommandLine(const std::vector<std::string>& args) {
   return android::base::Join(args, ' ');
 }
@@ -40,7 +50,7 @@
 // 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(std::vector<std::string>& arg_vector) {
+pid_t ExecWithoutWait(const std::vector<std::string>& arg_vector, std::string* error_msg) {
   // Convert the args to char pointers.
   const char* program = arg_vector[0].c_str();
   std::vector<char*> args;
@@ -65,110 +75,124 @@
     } else {
       execve(program, &args[0], envp);
     }
-    PLOG(ERROR) << "Failed to execve(" << ToCommandLine(arg_vector) << ")";
-    // _exit to avoid atexit handlers in child.
-    _exit(1);
+    // This should be regarded as a crash rather than a normal return.
+    PLOG(FATAL) << "Failed to execute (" << ToCommandLine(arg_vector) << ")";
+    UNREACHABLE();
+  } else if (pid == -1) {
+    *error_msg = StringPrintf("Failed to execute (%s) because fork failed: %s",
+                              ToCommandLine(arg_vector).c_str(),
+                              strerror(errno));
+    return -1;
   } else {
     return pid;
   }
 }
 
-}  // namespace
-
-int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg) {
-  pid_t pid = ExecWithoutWait(arg_vector);
-  if (pid == -1) {
-    *error_msg = StringPrintf("Failed to execv(%s) because fork failed: %s",
-                              ToCommandLine(arg_vector).c_str(), strerror(errno));
-    return -1;
-  }
-
-  // wait for subprocess to finish
+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 after fork for execv(%s) because waitpid failed: "
-                              "wanted %d, got %d: %s",
-                              ToCommandLine(arg_vector).c_str(), pid, got_pid, strerror(errno));
+    *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));
     return -1;
   }
-  if (WIFEXITED(status)) {
-    return WEXITSTATUS(status);
+  if (!WIFEXITED(status)) {
+    *error_msg =
+        StringPrintf("Failed to execute (%s) because the child process is terminated by signal %d",
+                     ToCommandLine(arg_vector).c_str(),
+                     WTERMSIG(status));
+    return -1;
   }
-  return -1;
+  return WEXITSTATUS(status);
 }
 
-int ExecAndReturnCode(std::vector<std::string>& arg_vector,
-                      time_t timeout_secs,
+int WaitChildWithTimeout(pid_t pid,
+                         const std::vector<std::string>& arg_vector,
+                         int timeout_sec,
+                         bool* timed_out,
+                         std::string* error_msg) {
+  auto cleanup = android::base::make_scope_guard([&]() {
+    kill(pid, SIGKILL);
+    std::string ignored_error_msg;
+    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.events = POLLIN;
+  int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_sec * 1000));
+
+  close(pidfd);
+
+  if (poll_ret < 0) {
+    *error_msg = StringPrintf("poll failed for pid %d: %s", pid, strerror(errno));
+    return -1;
+  }
+  if (poll_ret == 0) {
+    *timed_out = true;
+    *error_msg = StringPrintf("Child process %d timed out after %ds. Killing it", pid, timeout_sec);
+    return -1;
+  }
+
+  cleanup.Disable();
+  return WaitChild(pid, arg_vector, error_msg);
+}
+
+}  // 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 ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                      int timeout_sec,
                       bool* timed_out,
                       std::string* error_msg) {
   *timed_out = false;
 
   // Start subprocess.
-  pid_t pid = ExecWithoutWait(arg_vector);
+  pid_t pid = ExecWithoutWait(arg_vector, error_msg);
   if (pid == -1) {
-    *error_msg = StringPrintf("Failed to execv(%s) because fork failed: %s",
-                              ToCommandLine(arg_vector).c_str(), strerror(errno));
     return -1;
   }
 
-  // Add SIGCHLD to the signal set.
-  sigset_t child_mask, original_mask;
-  sigemptyset(&child_mask);
-  sigaddset(&child_mask, SIGCHLD);
-  if (sigprocmask(SIG_BLOCK, &child_mask, &original_mask) == -1) {
-    *error_msg = StringPrintf("Failed to set sigprocmask(): %s", strerror(errno));
-    return -1;
-  }
-
-  // Wait for a SIGCHLD notification.
-  errno = 0;
-  timespec ts = {timeout_secs, 0};
-  int wait_result = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, nullptr, &ts));
-  int wait_errno = errno;
-
-  // Restore the original signal set.
-  if (sigprocmask(SIG_SETMASK, &original_mask, nullptr) == -1) {
-    *error_msg = StringPrintf("Fail to restore sigprocmask(): %s", strerror(errno));
-    if (wait_result == 0) {
-      return -1;
-    }
-  }
-
-  // Having restored the signal set, see if we need to terminate the subprocess.
-  if (wait_result == -1) {
-    if (wait_errno == EAGAIN) {
-      *error_msg = "Timed out.";
-      *timed_out = true;
-    } else {
-      *error_msg = StringPrintf("Failed to sigtimedwait(): %s", strerror(errno));
-    }
-    if (kill(pid, SIGKILL) != 0) {
-      PLOG(ERROR) << "Failed to kill() subprocess: ";
-    }
-  }
-
   // Wait for subprocess to finish.
-  int status = -1;
-  pid_t got_pid = TEMP_FAILURE_RETRY(waitpid(pid, &status, 0));
-  if (got_pid != pid) {
-    *error_msg = StringPrintf("Failed after fork for execv(%s) because waitpid failed: "
-                              "wanted %d, got %d: %s",
-                              ToCommandLine(arg_vector).c_str(), pid, got_pid, strerror(errno));
-    return -1;
-  }
-  if (WIFEXITED(status)) {
-    return WEXITSTATUS(status);
-  }
-  return -1;
+  return WaitChildWithTimeout(pid, arg_vector, timeout_sec, timed_out, error_msg);
 }
 
-
-bool Exec(std::vector<std::string>& arg_vector, std::string* error_msg) {
+bool Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) {
   int status = ExecAndReturnCode(arg_vector, error_msg);
-  if (status != 0) {
-    *error_msg = StringPrintf("Failed execv(%s) because non-0 exit status",
-                              ToCommandLine(arg_vector).c_str());
+  if (status < 0) {
+    // Internal error. The error message is already set.
+    return false;
+  }
+  if (status > 0) {
+    *error_msg =
+        StringPrintf("Failed to execute (%s) because the child process returns non-zero exit code",
+                     ToCommandLine(arg_vector).c_str());
     return false;
   }
   return true;
diff --git a/runtime/exec_utils.h b/runtime/exec_utils.h
index 7ce0a9c..ff90ebd 100644
--- a/runtime/exec_utils.h
+++ b/runtime/exec_utils.h
@@ -29,13 +29,13 @@
 // of the runtime (Runtime::Current()) was started.  If no instance of the runtime was started, it
 // will use the current environment settings.
 
-bool Exec(std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg);
-int ExecAndReturnCode(std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg);
+bool Exec(const std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg);
+int ExecAndReturnCode(const std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg);
 
 // Execute the command specified in `argv_vector` in a subprocess with a timeout.
 // Returns the process exit code on success, -1 otherwise.
-int ExecAndReturnCode(std::vector<std::string>& arg_vector,
-                      time_t timeout_secs,
+int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                      int timeout_sec,
                       /*out*/ bool* timed_out,
                       /*out*/ std::string* error_msg);
 
@@ -44,20 +44,21 @@
  public:
   virtual ~ExecUtils() = default;
 
-  virtual bool Exec(std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg) const {
+  virtual bool Exec(const std::vector<std::string>& arg_vector,
+                    /*out*/ std::string* error_msg) const {
     return art::Exec(arg_vector, error_msg);
   }
 
-  virtual int ExecAndReturnCode(std::vector<std::string>& arg_vector,
+  virtual int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
                                 /*out*/ std::string* error_msg) const {
     return art::ExecAndReturnCode(arg_vector, error_msg);
   }
 
-  virtual int ExecAndReturnCode(std::vector<std::string>& arg_vector,
-                                time_t timeout_secs,
+  virtual int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
+                                int timeout_sec,
                                 /*out*/ bool* timed_out,
                                 /*out*/ std::string* error_msg) const {
-    return art::ExecAndReturnCode(arg_vector, timeout_secs, timed_out, error_msg);
+    return art::ExecAndReturnCode(arg_vector, timeout_sec, timed_out, error_msg);
   }
 };
 
diff --git a/runtime/exec_utils_test.cc b/runtime/exec_utils_test.cc
index dc789aa..aa53739 100644
--- a/runtime/exec_utils_test.cc
+++ b/runtime/exec_utils_test.cc
@@ -16,16 +16,34 @@
 
 #include "exec_utils.h"
 
+#include <unistd.h>
+
 #include "android-base/stringprintf.h"
 #include "base/file_utils.h"
 #include "base/memory_tool.h"
 #include "common_runtime_test.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 namespace art {
 
 std::string PrettyArguments(const char* signature);
 std::string PrettyReturnType(const char* signature);
 
+bool IsPidfdSupported() {
+#ifdef __BIONIC__
+  return true;
+#else
+  constexpr int SYS_pidfd_open = 434;
+  int pidfd = syscall(SYS_pidfd_open, getpid(), /*flags=*/0);
+  if (pidfd < 0) {
+    return false;
+  }
+  close(pidfd);
+  return true;
+#endif
+}
+
 class ExecUtilsTest : public CommonRuntimeTest {};
 
 TEST_F(ExecUtilsTest, ExecSuccess) {
@@ -44,9 +62,6 @@
 }
 
 TEST_F(ExecUtilsTest, ExecError) {
-  // This will lead to error messages in the log.
-  ScopedLogSeverity sls(LogSeverity::FATAL);
-
   std::vector<std::string> command;
   command.push_back("bogus");
   std::string error_msg;
@@ -115,6 +130,10 @@
 }
 
 TEST_F(ExecUtilsTest, ExecTimeout) {
+  if (!IsPidfdSupported()) {
+    GTEST_SKIP() << "pidfd not supported";
+  }
+
   static constexpr int kSleepSeconds = 5;
   static constexpr int kWaitSeconds = 1;
   std::vector<std::string> command = SleepCommand(kSleepSeconds);
@@ -125,6 +144,10 @@
 }
 
 TEST_F(ExecUtilsTest, ExecNoTimeout) {
+  if (!IsPidfdSupported()) {
+    GTEST_SKIP() << "pidfd not supported";
+  }
+
   static constexpr int kSleepSeconds = 1;
   static constexpr int kWaitSeconds = 5;
   std::vector<std::string> command = SleepCommand(kSleepSeconds);
@@ -134,4 +157,15 @@
   EXPECT_FALSE(timed_out);
 }
 
+TEST_F(ExecUtilsTest, ExecTimeoutNotSupported) {
+  if (IsPidfdSupported()) {
+    GTEST_SKIP() << "pidfd supported";
+  }
+
+  std::string error_msg;
+  bool timed_out;
+  ASSERT_EQ(ExecAndReturnCode({"command"}, /*timeout_sec=*/0, &timed_out, &error_msg), -1);
+  EXPECT_THAT(error_msg, testing::HasSubstr("pidfd_open failed for pid"));
+}
+
 }  // namespace art