From a57e3efbedb12c26506c689b5d11bcc17d7e70bb Mon Sep 17 00:00:00 2001 From: David Zaslavsky Date: Fri, 4 Jul 2025 22:36:44 -0700 Subject: [PATCH 1/4] Make running the exit code tests optional To support new waiting methods that don't provide the exit code of the process being waited for, I'm changing the test script to recognize an environment variable PWAIT_SKIP_EXIT_CODE_TESTS, which if set to any nonempty value will cause the exit code tests to be skipped. Now that the main test function in the test script, run_all_tests(), supports skipping exit code tests, I changed the test configuration so that testing the poll method will use that function as well. The tests of the --delay option, which are specific to the polling method, will be run from a separate test function. --- test/CMakeLists.txt | 2 +- test/test_pwait.sh | 28 ++++++++++++---------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ff2b662..598aed2 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -24,4 +24,4 @@ add_test( NAME test-pwait-poll COMMAND "${CMAKE_CURRENT_LIST_DIR}/test_pwait.sh" ) -set_tests_properties(test-pwait-poll PROPERTIES ENVIRONMENT "PWAIT=$;PWAIT_METHOD=poll") +set_tests_properties(test-pwait-poll PROPERTIES ENVIRONMENT "PWAIT=$;PWAIT_METHOD=poll;PWAIT_SKIP_EXIT_CODE_TESTS=1") diff --git a/test/test_pwait.sh b/test/test_pwait.sh index aa1c808..7bd243e 100755 --- a/test/test_pwait.sh +++ b/test/test_pwait.sh @@ -127,9 +127,11 @@ test_pwait_and_target_exit_times() { run_all_tests() { - test_pwait_exit_code 0 - test_pwait_exit_code 1 - test_pwait_exit_code 128 + if [[ -z "${PWAIT_SKIP_EXIT_CODE_TESTS:-}" ]]; then + test_pwait_exit_code 0 + test_pwait_exit_code 1 + test_pwait_exit_code 128 + fi test_target_does_not_exist_after_pwait_exit test_pwait_and_target_exit_times 0.1s test_pwait_and_target_exit_times 1s @@ -138,9 +140,7 @@ run_all_tests() { } -run_poll_tests() { - pwait_options=("--method=poll") - test_target_does_not_exist_after_pwait_exit +run_poll_delay_tests() { for delay in 1 2 5; do pwait_options=("--method=poll" "--delay=$delay") test_pwait_and_target_exit_times 10s "${delay}" @@ -148,13 +148,9 @@ run_poll_tests() { } -pwait_options=() -case "${PWAIT_METHOD:-}" in - poll) - run_poll_tests - ;; - *) - pwait_options=("--method=${PWAIT_METHOD}") - run_all_tests - ;; -esac +pwait_options=("--method=${PWAIT_METHOD}") +run_all_tests + +if [[ "${PWAIT_METHOD:-}" == "poll" ]]; then + run_poll_delay_tests +fi From a0702e4defffb60babec604f5dbc122ed91b658a Mon Sep 17 00:00:00 2001 From: David Zaslavsky Date: Sun, 8 Sep 2024 02:30:46 -0700 Subject: [PATCH 2/4] Add a wait implementation that uses the pidfd_open syscall This is very similar to the implementation in the man page for pidfd_open (https://man7.org/linux/man-pages/man2/poll.2.html), but that's mostly because there isn't a lot of flexibility in how you can write code to do this. --- src/CMakeLists.txt | 2 +- src/pidfd.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++ src/pwait.c | 5 +++- src/pwait.h | 1 + 4 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/pidfd.c diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a76333d..ecbba5d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -6,7 +6,7 @@ include(CheckSymbolExists) check_symbol_exists(__GLIBC__ features.h _GNU_SOURCE) configure_file(config.h.in config.h) -add_executable(pwait pwait.c ptrace.c netlink.c poll.c capabilities.c) +add_executable(pwait pwait.c ptrace.c netlink.c poll.c pidfd.c capabilities.c) target_link_libraries(pwait cap) install(TARGETS pwait RUNTIME DESTINATION bin) diff --git a/src/pidfd.c b/src/pidfd.c new file mode 100644 index 0000000..850ca26 --- /dev/null +++ b/src/pidfd.c @@ -0,0 +1,71 @@ +/* Implements waiting for a process using the `pidfd_open` syscall + * + * This implementation is similar to the one in + * [the man page for `pidfd_open`](https://man7.org/linux/man-pages/man2/pidfd_open.2.html) + * but that's effectively forced by the task; there isn't a lot of variety in + * how you can write this. + */ + +#define _GNU_SOURCE + +#include "pwait.h" +#include +#include +#include +#include +#include +#include + + +/** + * Open a process file descriptor. + * + * This is a thin wrapper around the pidfd_open() syscall. + * + * @return The file descriptor opened, or -1 if opening failed. + */ +static int pidfd_open(pid_t pid) { + long result = syscall(SYS_pidfd_open, pid, 0); + int pidfd = (int)result; + assert((long)pidfd == result); + return pidfd; +} + + +int wait_using_pidfd(pid_t pid) { + int fd = pidfd_open(pid); + if (fd < 0) { + // ESRCH indicates that no process with that ID was found + return errno == ESRCH ? -1 : EX_OSERR; + } + + struct pollfd pfd = { + .fd = fd, + .events = POLLIN, + }; + + int pfd_status; + while ((pfd_status = poll(&pfd, 1, -1)) >= 0) { + if (pfd_status < 0) { + if (errno == EINTR) { + // poll() was interrupted by a signal, so we can just keep going + continue; + } + else { + // Some more serious error occurred + return EX_OSERR; + } + } + else if (pfd_status == 0) { + // This shouldn't happen because we set timeout to -1, i.e. infinite + return -1; + } + else { + assert(pfd_status == 1); + if (pfd.revents & POLLIN) { + break; + } + } + } + return 0; +} diff --git a/src/pwait.c b/src/pwait.c index e39b8ea..0afc1be 100644 --- a/src/pwait.c +++ b/src/pwait.c @@ -45,7 +45,7 @@ static void help(const char* name) { printf(" -d, --delay=SECONDS set the polling frequency when --method=poll\n"); printf(" -h, --help print this help message and exit\n"); printf(" -m, --method=METHOD use METHOD to wait for the process\n"); - printf(" METHOD is one of 'netlink' (default), 'ptrace', or 'poll'\n"); + printf(" METHOD is one of 'netlink' (default), 'ptrace', 'poll', or 'pidfd'\n"); printf(" -v, --verbose print diagnostic output to stderr\n"); #else printf(" -h print this help message and exit\n"); @@ -101,6 +101,9 @@ int main(const int argc, char* const* argv) { else if (strncmp(optarg, "poll", 5) == 0) { wait_function = wait_using_polling; } + else if (strncmp(optarg, "pidfd", 6) == 0) { + wait_function = wait_using_pidfd; + } else { wait_function = NULL; } diff --git a/src/pwait.h b/src/pwait.h index 222ac3c..87c686d 100644 --- a/src/pwait.h +++ b/src/pwait.h @@ -29,4 +29,5 @@ int acquire_capabilities(size_t n, const cap_value_t* capabilities_to_acquire); int wait_using_ptrace(pid_t pid); int wait_using_netlink(pid_t pid); int wait_using_polling(pid_t pid); +int wait_using_pidfd(pid_t pid); void set_delay(unsigned int delay); From ffd439da22f8538dd9b5dfc1ffbcf00556f42ad1 Mon Sep 17 00:00:00 2001 From: David Zaslavsky Date: Fri, 4 Jul 2025 22:11:32 -0700 Subject: [PATCH 3/4] Add tests for pidfd mode --- test/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 598aed2..bfcc95a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -25,3 +25,9 @@ add_test( COMMAND "${CMAKE_CURRENT_LIST_DIR}/test_pwait.sh" ) set_tests_properties(test-pwait-poll PROPERTIES ENVIRONMENT "PWAIT=$;PWAIT_METHOD=poll;PWAIT_SKIP_EXIT_CODE_TESTS=1") + +add_test( + NAME test-pwait-pidfd + COMMAND "${CMAKE_CURRENT_LIST_DIR}/test_pwait.sh" +) +set_tests_properties(test-pwait-pidfd PROPERTIES ENVIRONMENT "PWAIT=$;PWAIT_METHOD=pidfd;PWAIT_SKIP_EXIT_CODE_TESTS=1") From 4da11d99dae8a249d5d4f86e5e5f8d02ef81e16c Mon Sep 17 00:00:00 2001 From: David Zaslavsky Date: Sat, 5 Jul 2025 02:46:43 -0700 Subject: [PATCH 4/4] Update branch name in GHA workflow file from master to main This is necessary to have GitHub Actions workflows run after renaming the default branch from master to main. --- .github/workflows/build-and-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 20b7011..57e470c 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -3,10 +3,10 @@ name: Build and test with CMake on: push: branches: - - master + - main pull_request: branches: - - master + - main jobs: build: