Temporarily disable art-tests for userfaultfd GC am: a8760e2839 am: 76018fd633
Original change: https://android-review.googlesource.com/c/platform/art/+/2191948
Change-Id: I194ab393292adc93349d8553a8a0959da6b968e4
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 722f70a..6e2a5b4 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -21,24 +21,19 @@
#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 {
@@ -46,7 +41,6 @@
namespace {
using ::android::base::StringPrintf;
-using ::android::base::unique_fd;
std::string ToCommandLine(const std::vector<std::string>& args) {
return android::base::Join(args, ' ');
@@ -94,86 +88,31 @@
}
}
-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));
+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));
return -1;
}
- 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) {
+ if (!WIFEXITED(status)) {
*error_msg =
StringPrintf("Failed to execute (%s) because the child process is terminated by signal %d",
ToCommandLine(arg_vector).c_str(),
- info.si_status);
+ WTERMSIG(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 WaitChild(pid, arg_vector, error_msg);
+ return WEXITSTATUS(status);
}
int WaitChildWithTimeout(pid_t pid,
- unique_fd pidfd,
const std::vector<std::string>& arg_vector,
- int timeout_ms,
+ int timeout_sec,
bool* timed_out,
std::string* error_msg) {
auto cleanup = android::base::make_scope_guard([&]() {
@@ -182,12 +121,24 @@
WaitChild(pid, arg_vector, &ignored_error_msg);
});
- struct pollfd pfd;
- pfd.fd = pidfd.get();
- pfd.events = POLLIN;
- int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_ms));
+#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;
+ }
- pidfd.reset();
+ 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));
@@ -195,7 +146,7 @@
}
if (poll_ret == 0) {
*timed_out = true;
- *error_msg = StringPrintf("Child process %d timed out after %dms. Killing it", pid, timeout_ms);
+ *error_msg = StringPrintf("Child process %d timed out after %ds. Killing it", pid, timeout_sec);
return -1;
}
@@ -205,23 +156,23 @@
} // namespace
-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 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";
+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, error_msg);
if (pid == -1) {
@@ -229,23 +180,10 @@
}
// Wait for subprocess to finish.
- 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);
- }
+ return WaitChildWithTimeout(pid, arg_vector, timeout_sec, timed_out, error_msg);
}
-bool ExecUtils::Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) const {
+bool Exec(const std::vector<std::string>& arg_vector, std::string* error_msg) {
int status = ExecAndReturnCode(arg_vector, error_msg);
if (status < 0) {
// Internal error. The error message is already set.
@@ -260,16 +198,4 @@
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
diff --git a/runtime/exec_utils.h b/runtime/exec_utils.h
index 79a12d7..ff90ebd 100644
--- a/runtime/exec_utils.h
+++ b/runtime/exec_utils.h
@@ -22,46 +22,46 @@
#include <string>
#include <vector>
-#include "android-base/unique_fd.h"
-
namespace art {
// Wrapper on fork/execv to run a command in a subprocess.
// These spawn child processes using the environment as it was set when the single instance
// of the runtime (Runtime::Current()) was started. If no instance of the runtime was started, it
// will use the current environment settings.
+
+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(const std::vector<std::string>& arg_vector,
+ int timeout_sec,
+ /*out*/ bool* timed_out,
+ /*out*/ std::string* error_msg);
+
+// A wrapper class to make the functions above mockable.
class ExecUtils {
public:
virtual ~ExecUtils() = default;
virtual bool Exec(const std::vector<std::string>& arg_vector,
- /*out*/ std::string* error_msg) const;
+ /*out*/ std::string* error_msg) const {
+ return art::Exec(arg_vector, error_msg);
+ }
virtual int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
- /*out*/ std::string* error_msg) const;
+ /*out*/ std::string* error_msg) const {
+ return art::ExecAndReturnCode(arg_vector, error_msg);
+ }
- // Executes the command specified in `arg_vector` in a subprocess with a timeout.
- // If `timeout_sec` is negative, blocks until the subprocess exits.
- // Returns the process exit code on success, -1 otherwise.
- // Sets `timed_out` to true if the process times out, or false otherwise.
virtual int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
int timeout_sec,
/*out*/ bool* timed_out,
- /*out*/ std::string* error_msg) const;
-
- protected:
- virtual android::base::unique_fd PidfdOpen(pid_t pid) const;
+ /*out*/ std::string* error_msg) const {
+ return art::ExecAndReturnCode(arg_vector, timeout_sec, timed_out, error_msg);
+ }
};
-inline bool Exec(const std::vector<std::string>& arg_vector, /*out*/ std::string* error_msg) {
- return ExecUtils().Exec(arg_vector, error_msg);
-}
-
-inline int ExecAndReturnCode(const std::vector<std::string>& arg_vector,
- /*out*/ std::string* error_msg) {
- return ExecUtils().ExecAndReturnCode(arg_vector, error_msg);
-}
-
} // namespace art
#endif // ART_RUNTIME_EXEC_UTILS_H_
diff --git a/runtime/exec_utils_test.cc b/runtime/exec_utils_test.cc
index c10b84e..aa53739 100644
--- a/runtime/exec_utils_test.cc
+++ b/runtime/exec_utils_test.cc
@@ -16,18 +16,13 @@
#include "exec_utils.h"
-#include <sys/utsname.h>
+#include <unistd.h>
-#include <cstring>
-#include <filesystem>
-#include <memory>
-#include <tuple>
-
-#include "android-base/logging.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 {
@@ -35,95 +30,69 @@
std::string PrettyArguments(const char* signature);
std::string PrettyReturnType(const char* signature);
-std::string GetBin(const std::string& name) {
+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) {
+ std::vector<std::string> command;
if (kIsTargetBuild) {
std::string android_root(GetAndroidRoot());
- return android_root + "/bin/" + name;
- } else if (std::filesystem::exists("/usr/bin/" + name)) {
- return "/usr/bin/" + name;
+ command.push_back(android_root + "/bin/id");
} else {
- return "/bin/" + name;
+ command.push_back("/usr/bin/id");
}
-}
-
-std::tuple<int, int> GetKernelVersion() {
- std::tuple<int, int> version;
- utsname uts;
- CHECK_EQ(uname(&uts), 0);
- CHECK_EQ(sscanf(uts.release, "%d.%d", &std::get<0>(version), &std::get<1>(version)), 2);
- return version;
-}
-
-class AlwaysFallbackExecUtils : public ExecUtils {
- protected:
- android::base::unique_fd PidfdOpen(pid_t) const override { return android::base::unique_fd(-1); }
-};
-
-class NeverFallbackExecUtils : public ExecUtils {
- protected:
- android::base::unique_fd PidfdOpen(pid_t pid) const override {
- android::base::unique_fd pidfd = ExecUtils::PidfdOpen(pid);
- CHECK_GE(pidfd.get(), 0) << strerror(errno);
- return pidfd;
- }
-};
-
-class ExecUtilsTest : public CommonRuntimeTest, public testing::WithParamInterface<bool> {
- protected:
- void SetUp() override {
- CommonRuntimeTest::SetUp();
- bool always_fallback = GetParam();
- if (always_fallback) {
- exec_utils_ = std::make_unique<AlwaysFallbackExecUtils>();
- } else {
- if (GetKernelVersion() >= std::make_tuple(5, 4)) {
- exec_utils_ = std::make_unique<NeverFallbackExecUtils>();
- } else {
- GTEST_SKIP() << "Kernel version older than 5.4";
- }
- }
- }
-
- std::unique_ptr<ExecUtils> exec_utils_;
-};
-
-TEST_P(ExecUtilsTest, ExecSuccess) {
- std::vector<std::string> command;
- command.push_back(GetBin("id"));
std::string error_msg;
// Historical note: Running on Valgrind failed due to some memory
// that leaks in thread alternate signal stacks.
- EXPECT_TRUE(exec_utils_->Exec(command, &error_msg));
+ EXPECT_TRUE(Exec(command, &error_msg));
EXPECT_EQ(0U, error_msg.size()) << error_msg;
}
-TEST_P(ExecUtilsTest, ExecError) {
+TEST_F(ExecUtilsTest, ExecError) {
std::vector<std::string> command;
command.push_back("bogus");
std::string error_msg;
// Historical note: Running on Valgrind failed due to some memory
// that leaks in thread alternate signal stacks.
- EXPECT_FALSE(exec_utils_->Exec(command, &error_msg));
+ EXPECT_FALSE(Exec(command, &error_msg));
EXPECT_FALSE(error_msg.empty());
}
-TEST_P(ExecUtilsTest, EnvSnapshotAdditionsAreNotVisible) {
+TEST_F(ExecUtilsTest, EnvSnapshotAdditionsAreNotVisible) {
static constexpr const char* kModifiedVariable = "EXEC_SHOULD_NOT_EXPORT_THIS";
static constexpr int kOverwrite = 1;
// Set an variable in the current environment.
EXPECT_EQ(setenv(kModifiedVariable, "NEVER", kOverwrite), 0);
// Test that it is not exported.
std::vector<std::string> command;
- command.push_back(GetBin("printenv"));
+ if (kIsTargetBuild) {
+ std::string android_root(GetAndroidRoot());
+ command.push_back(android_root + "/bin/printenv");
+ } else {
+ command.push_back("/usr/bin/printenv");
+ }
command.push_back(kModifiedVariable);
std::string error_msg;
// Historical note: Running on Valgrind failed due to some memory
// that leaks in thread alternate signal stacks.
- EXPECT_FALSE(exec_utils_->Exec(command, &error_msg));
+ EXPECT_FALSE(Exec(command, &error_msg));
EXPECT_NE(0U, error_msg.size()) << error_msg;
}
-TEST_P(ExecUtilsTest, EnvSnapshotDeletionsAreNotVisible) {
+TEST_F(ExecUtilsTest, EnvSnapshotDeletionsAreNotVisible) {
static constexpr const char* kDeletedVariable = "PATH";
static constexpr int kOverwrite = 1;
// Save the variable's value.
@@ -133,12 +102,17 @@
EXPECT_EQ(unsetenv(kDeletedVariable), 0);
// Test that it is not exported.
std::vector<std::string> command;
- command.push_back(GetBin("printenv"));
+ if (kIsTargetBuild) {
+ std::string android_root(GetAndroidRoot());
+ command.push_back(android_root + "/bin/printenv");
+ } else {
+ command.push_back("/usr/bin/printenv");
+ }
command.push_back(kDeletedVariable);
std::string error_msg;
// Historical note: Running on Valgrind failed due to some memory
// that leaks in thread alternate signal stacks.
- EXPECT_TRUE(exec_utils_->Exec(command, &error_msg));
+ EXPECT_TRUE(Exec(command, &error_msg));
EXPECT_EQ(0U, error_msg.size()) << error_msg;
// Restore the variable's value.
EXPECT_EQ(setenv(kDeletedVariable, save_value, kOverwrite), 0);
@@ -146,32 +120,52 @@
static std::vector<std::string> SleepCommand(int sleep_seconds) {
std::vector<std::string> command;
- command.push_back(GetBin("sleep"));
+ if (kIsTargetBuild) {
+ command.push_back(GetAndroidRoot() + "/bin/sleep");
+ } else {
+ command.push_back("/bin/sleep");
+ }
command.push_back(android::base::StringPrintf("%d", sleep_seconds));
return command;
}
-TEST_P(ExecUtilsTest, ExecTimeout) {
+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);
std::string error_msg;
bool timed_out;
- ASSERT_EQ(exec_utils_->ExecAndReturnCode(command, kWaitSeconds, &timed_out, &error_msg), -1);
- EXPECT_TRUE(timed_out) << error_msg;
+ ASSERT_EQ(ExecAndReturnCode(command, kWaitSeconds, &timed_out, &error_msg), -1);
+ EXPECT_TRUE(timed_out);
}
-TEST_P(ExecUtilsTest, ExecNoTimeout) {
+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);
std::string error_msg;
bool timed_out;
- ASSERT_EQ(exec_utils_->ExecAndReturnCode(command, kWaitSeconds, &timed_out, &error_msg), 0)
- << error_msg;
+ ASSERT_EQ(ExecAndReturnCode(command, kWaitSeconds, &timed_out, &error_msg), 0);
EXPECT_FALSE(timed_out);
}
-INSTANTIATE_TEST_SUITE_P(AlwaysOrNeverFallback, ExecUtilsTest, testing::Values(true, false));
+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