summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--odrefresh/odrefresh_test.cc6
-rw-r--r--runtime/Android.bp3
-rw-r--r--runtime/exec_utils.cc184
-rw-r--r--runtime/exec_utils.h19
-rw-r--r--runtime/exec_utils_test.cc40
5 files changed, 157 insertions, 95 deletions
diff --git a/odrefresh/odrefresh_test.cc b/odrefresh/odrefresh_test.cc
index ae7cc78594..1a8f4e9e37 100644
--- a/odrefresh/odrefresh_test.cc
+++ b/odrefresh/odrefresh_test.cc
@@ -71,14 +71,14 @@ class MockExecUtils : public ExecUtils {
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 c5cd7c57da..809445bc0f 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -849,6 +849,9 @@ art_cc_defaults {
"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 463d4580cf..6e2a5b40f0 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 @@ 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(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 @@ pid_t ExecWithoutWait(std::vector<std::string>& arg_vector) {
} 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,
- bool* timed_out,
- std::string* error_msg) {
- *timed_out = false;
-
- // Start subprocess.
- 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));
+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;
}
- // 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));
+ 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;
}
-
- // 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;
- }
+ if (poll_ret == 0) {
+ *timed_out = true;
+ *error_msg = StringPrintf("Child process %d timed out after %ds. Killing it", pid, timeout_sec);
+ 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: ";
- }
+ 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.
- 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 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, error_msg);
+ if (pid == -1) {
return -1;
}
- if (WIFEXITED(status)) {
- return WEXITSTATUS(status);
- }
- return -1;
-}
+ // Wait for subprocess to finish.
+ 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 7ce0a9c20a..ff90ebdfb3 100644
--- a/runtime/exec_utils.h
+++ b/runtime/exec_utils.h
@@ -29,13 +29,13 @@ namespace art {
// 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 @@ class ExecUtils {
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 dc789aa292..aa53739cfa 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, ExecSuccess) {
}
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 @@ static std::vector<std::string> SleepCommand(int sleep_seconds) {
}
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, ExecTimeout) {
}
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 @@ TEST_F(ExecUtilsTest, ExecNoTimeout) {
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
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-20 03:52:30 +0000