From 13ebc3adb639e764f195194accaa8826f2a0c895 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Cs=C3=A1rdi?= Date: Mon, 20 Apr 2026 13:04:12 +0200 Subject: [PATCH] More accurate process startup time on Linux For https://github.com/r-lib/processx/issues/394 and https://github.com/r-lib/processx/issues/402. --- DESCRIPTION | 2 +- NEWS.md | 8 +++ src/api-linux.c | 105 ++++++++++++++++++++++++++++------- tests/testthat/test-common.R | 9 ++- tests/testthat/test-linux.R | 67 ++++++++++++++++++++++ 5 files changed, 170 insertions(+), 21 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0f856cbe..c6e7a435 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: ps Title: List, Query, Manipulate System Processes -Version: 1.9.2.9000 +Version: 1.9.2.9001 Authors@R: c( person("Jay", "Loden", role = "aut"), person("Dave", "Daeschler", role = "aut"), diff --git a/NEWS.md b/NEWS.md index 746e78cd..a592c3db 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,13 @@ # ps (development version) +* On Linux, process create times are now computed using + `CLOCK_REALTIME - CLOCK_MONOTONIC` instead of `/proc/stat btime`, giving + sub-second precision (previously, integer-second boot time caused up to 1s + error). Handle validation accepts both the precise and the legacy boot time, + so handles created by older versions of processx continue to work. + For https://github.com/r-lib/processx/issues/394 and + https://github.com/r-lib/processx/issues/402. + # ps 1.9.2 * New `ps_string()` for uniquely identifying a process (#208, @dansmith01). diff --git a/src/api-linux.c b/src/api-linux.c index 1dabfce2..e9ef14fe 100644 --- a/src/api-linux.c +++ b/src/api-linux.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #ifndef __linux__ @@ -36,7 +37,8 @@ #define SYS_pidfd_open 434 #endif -double psll_linux_boot_time = 0; +double psll_linux_boot_time = 0; /* precise: CLOCK_REALTIME - CLOCK_MONOTONIC */ +double psll_linux_boot_time_legacy = 0; /* integer seconds from /proc/stat btime */ double psll_linux_clock_period = 0; typedef struct { @@ -50,15 +52,28 @@ typedef struct { #define PS__TV2DOUBLE(t) ((t).tv_sec + (t).tv_usec / 1000000.0) -#define PS__CHECK_STAT(stat, handle) \ - do { \ - double starttime = psll_linux_boot_time + \ - ((double)(stat.starttime) * psll_linux_clock_period); \ - double diff = starttime - (handle)->create_time; \ - if (fabs(diff) > psll_linux_clock_period) { \ - ps__no_such_process((handle)->pid, 0); \ - ps__throw_error(); \ - } \ +/* Try both precise (CLOCK_REALTIME-CLOCK_MONOTONIC) and legacy (/proc/stat + btime) boot times. This lets new ps validate handles created by old + processx (which used the legacy integer boot time) while also correctly + validating handles from new processx (which uses the precise boot time). + The two checks are orthogonal: PID-reuse detection depends only on + starttime_ticks being different, so accepting either match is safe. */ +#define PS__CHECK_STAT(stat, handle) \ + do { \ + double ticks_ = (double)(stat.starttime) * psll_linux_clock_period; \ + int match_ = 0; \ + if (psll_linux_boot_time && \ + fabs(psll_linux_boot_time + ticks_ - (handle)->create_time) \ + <= psll_linux_clock_period) \ + match_ = 1; \ + if (!match_ && psll_linux_boot_time_legacy && \ + fabs(psll_linux_boot_time_legacy + ticks_ - (handle)->create_time) \ + <= psll_linux_clock_period) \ + match_ = 1; \ + if (!match_) { \ + ps__no_such_process((handle)->pid, 0); \ + ps__throw_error(); \ + } \ } while (0) #define PS__CHECK_HANDLE(handle) \ @@ -270,23 +285,37 @@ void ps__check_for_zombie(ps_handle_t *handle, int err) { } int psll_linux_get_boot_time(void) { + struct timespec real_time, mono_time; + + /* Use CLOCK_REALTIME - CLOCK_MONOTONIC for sub-second boot time precision. + /proc/stat btime is integer seconds only, causing up to 1s error in + create_time. CLOCK_MONOTONIC uses the same boot reference as + /proc//stat starttime. See https://github.com/r-lib/processx/issues/394 */ + if (clock_gettime(CLOCK_REALTIME, &real_time) == -1) return -1; + if (clock_gettime(CLOCK_MONOTONIC, &mono_time) == -1) return -1; + + psll_linux_boot_time = + (real_time.tv_sec - mono_time.tv_sec) + + (real_time.tv_nsec - mono_time.tv_nsec) * 1e-9; + return 0; +} + +int psll_linux_get_boot_time_legacy(void) { int ret; char *buf; - char *needle = "\nbtime "; + const char *needle = "\nbtime "; size_t needle_len = strlen(needle); char *hit; unsigned long btime; ret = ps__read_file("/proc/stat", &buf, /* buffer= */ 2048); if (ret == -1) return -1; - *(buf + ret - 1) = '\0'; hit = ps__memmem(buf, ret, needle, needle_len); if (!hit) return -1; - ret = sscanf(hit + needle_len, "%lu", &btime); if (ret != 1) return -1; - psll_linux_boot_time = (double) btime; + psll_linux_boot_time_legacy = (double) btime; return 0; } @@ -328,6 +357,26 @@ SEXP psll_handle(SEXP pid, SEXP time) { double ctime; ps_handle_t *handle; SEXP res; + int ret; + + /* Initialize both boot times and clock period so PS__CHECK_STAT can try + either boot time computation (precise and legacy) for validation. */ + if (!psll_linux_boot_time) { + ret = psll_linux_get_boot_time(); + if (ret) { + ps__set_error_from_errno(); + ps__throw_error(); + } + } + if (!psll_linux_boot_time_legacy) { + psll_linux_get_boot_time_legacy(); /* best-effort; failure is non-fatal */ + } + if (!psll_linux_clock_period) { + ret = psll_linux_get_clock_period(); + if (ret) { + ps__throw_error(); + } + } if (!isNull(time)) { ctime = REAL(time)[0]; @@ -425,15 +474,33 @@ SEXP psll_ppid(SEXP p) { SEXP psll_is_running(SEXP p) { ps_handle_t *handle = R_ExternalPtrAddr(p); - double ctime; - int ret; + psl_stat_t stat; + double ticks; if (!handle) error("Process pointer cleaned up already"); - ret = psll_linux_ctime(handle->pid, &ctime); - if (ret) return ScalarLogical(0); + if (psll__parse_stat_file(handle->pid, &stat, 0)) return ScalarLogical(0); - return ScalarLogical(ctime == handle->create_time); + if (!psll_linux_clock_period) { + if (psll_linux_get_clock_period()) return ScalarLogical(0); + } + ticks = (double)stat.starttime * psll_linux_clock_period; + + /* Try precise boot time (CLOCK_REALTIME - CLOCK_MONOTONIC). */ + if (!psll_linux_boot_time) psll_linux_get_boot_time(); /* ignore failure */ + if (psll_linux_boot_time && + fabs(psll_linux_boot_time + ticks - handle->create_time) + <= psll_linux_clock_period) + return ScalarLogical(1); + + /* Try legacy boot time (integer seconds from /proc/stat btime). + This handles handles created by old processx, which used the legacy + integer boot time for create_time. */ + if (!psll_linux_boot_time_legacy) psll_linux_get_boot_time_legacy(); + return ScalarLogical( + psll_linux_boot_time_legacy != 0 && + fabs(psll_linux_boot_time_legacy + ticks - handle->create_time) + <= psll_linux_clock_period); } SEXP psll_name(SEXP p) { diff --git a/tests/testthat/test-common.R b/tests/testthat/test-common.R index b7d4a6d7..ee795dd3 100644 --- a/tests/testthat/test-common.R +++ b/tests/testthat/test-common.R @@ -52,7 +52,14 @@ test_that("create_time", { p1 <- processx::process$new(px(), c("sleep", "10")) on.exit(p1$kill(), add = TRUE) ps <- ps_handle(p1$get_pid()) - expect_identical(p1$get_start_time(), ps_create_time(ps)) + ## processx$get_start_time() applies a before_start lower bound that can + ## push the reported time up to one clock tick (10ms) above the kernel's + ## recorded time, so exact equality is not guaranteed. + expect_equal( + as.numeric(p1$get_start_time()), + as.numeric(ps_create_time(ps)), + tolerance = 0.02 + ) }) test_that("is_running", { diff --git a/tests/testthat/test-linux.R b/tests/testthat/test-linux.R index 9a274318..660a48be 100644 --- a/tests/testthat/test-linux.R +++ b/tests/testthat/test-linux.R @@ -24,6 +24,73 @@ test_that("status", { ## TODO: cpu_times ??? We apparently cannot get them from ps +## Helper: read starttime (ticks since boot) from /proc//stat, +## handling process names that contain spaces or parentheses. +proc_starttime_ticks <- function(pid) { + raw <- readLines(sprintf("/proc/%d/stat", pid)) + ## Everything after the last ')' is the fixed-format tail + after_paren <- sub("^.*\\) ", "", raw) + fields <- strsplit(after_paren, " ")[[1]] + ## starttime is the 20th field after the name (field 22 overall) + as.numeric(fields[20]) +} + +## Helper: integer boot time from /proc/stat btime (what old ps used) +proc_stat_btime <- function() { + line <- grep("^btime ", readLines("/proc/stat"), value = TRUE) + as.numeric(strsplit(line, " +")[[1]][2]) +} + +test_that("ps_handle validates legacy (integer /proc/stat btime) create_time", { + skip_if_no_processx() + + p <- processx::process$new("sleep", "100") + on.exit(p$kill(), add = TRUE) + pid <- p$get_pid() + + ## Build the create_time the way old ps did: integer boot time + ticks/CLK_TCK + btime <- proc_stat_btime() + ticks <- proc_starttime_ticks(pid) + clk_tck <- as.integer(system2("getconf", "CLK_TCK", stdout = TRUE)) + legacy_ct <- structure( + btime + ticks / clk_tck, + class = c("POSIXct", "POSIXt"), + tzone = "GMT" + ) + + h <- ps_handle(pid, legacy_ct) + expect_true(ps_is_running(h)) + expect_equal(ps_name(h), "sleep") + ps_suspend(h) + wait_for_status(h, "stopped") + expect_equal(ps_status(h), "stopped") + ps_resume(h) + wait_for_status(h, "sleeping") + expect_equal(ps_status(h), "sleeping") +}) + +test_that("ps_handle validates precise (CLOCK_REALTIME-CLOCK_MONOTONIC) create_time", { + skip_if_no_processx() + + p <- processx::process$new("sleep", "100") + on.exit(p$kill(), add = TRUE) + pid <- p$get_pid() + + ## ps_handle(pid) auto-discovers the precise create_time internally + precise_ct <- ps_create_time(ps_handle(pid)) + + ## Re-create the handle with that explicit precise time + h <- ps_handle(pid, precise_ct) + expect_true(ps_is_running(h)) + expect_equal(ps_name(h), "sleep") + ps_suspend(h) + wait_for_status(h, "stopped") + expect_equal(ps_status(h), "stopped") + ps_resume(h) + wait_for_status(h, "sleeping") + expect_equal(ps_status(h), "sleeping") +}) + test_that("memory_info", { ## Argument check expect_error(ps_memory_info(123), class = "invalid_argument")