From 538ad308a656f45ebbbbccc1eb679ddee1d4395c Mon Sep 17 00:00:00 2001 From: OceanLi <122793010+ohdearquant@users.noreply.github.com> Date: Sun, 2 Aug 2026 06:11:26 -0400 Subject: [PATCH] fix(rvf-runtime,rvlite): read errno portably so every Unix target compiles is_pid_alive runs under #[cfg(unix)] but the errno accessor it calls was defined only for linux, android, macos, ios and freebsd. On any other Unix the caller is compiled and the callee is not, so the build fails to resolve it: error[E0425]: cannot find function `libc_errno` in this scope --> crates/rvf/rvf-runtime/src/locking.rs:283:29 Reproduced with cargo check -p rvf-runtime --target x86_64-unknown-netbsd, and via rvlite with --features rvf-backend, which reaches the same shape in storage/writer_lease.rs. Extending the per-OS symbol list would leave the same gap open for the next target, since the trigger condition is all of unix while the hand-declared reach is a fixed list. std::io::Error::last_os_error() reads errno through each platform's own accessor, so it covers every Unix without naming any symbol, needs no new dependency, and removes two unsafe blocks and four extern declarations. Adds a direct test for rvlite's is_pid_alive. Replacing the errno comparison with a constant true left rvlite's suite fully green, so that line had no coverage; the new test fails under that mutation and passes with the fix. rvf-runtime already had coverage: the same mutation fails stale_lock_detection. --- crates/rvf/rvf-runtime/src/locking.rs | 29 +++------------ crates/rvlite/src/storage/writer_lease.rs | 44 ++++++++++++----------- 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/crates/rvf/rvf-runtime/src/locking.rs b/crates/rvf/rvf-runtime/src/locking.rs index dc6a5931d1..e7d0411c72 100644 --- a/crates/rvf/rvf-runtime/src/locking.rs +++ b/crates/rvf/rvf-runtime/src/locking.rs @@ -279,9 +279,10 @@ fn is_pid_alive(pid: u32) -> bool { if ret == 0 { return true; } - // Check errno for EPERM -- process exists but we lack permission - let err = unsafe { *libc_errno() }; - err == EPERM + // Check errno for EPERM -- process exists but we lack permission. + // last_os_error reads errno through each platform's own accessor, so this + // works on every Unix rather than only those with a hand-declared symbol. + std::io::Error::last_os_error().raw_os_error() == Some(EPERM) } #[cfg(not(unix))] { @@ -299,16 +300,6 @@ extern "C" { fn kill(pid: i32, sig: i32) -> i32; } -#[cfg(any(target_os = "linux", target_os = "android"))] -extern "C" { - fn __errno_location() -> *mut i32; -} - -#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd"))] -extern "C" { - fn __error() -> *mut i32; -} - /// Permission denied errno -- process exists but belongs to another user. #[cfg(unix)] const EPERM: i32 = 1; @@ -318,18 +309,6 @@ fn libc_kill(pid: i32, sig: i32) -> i32 { unsafe { kill(pid, sig) } } -/// Get a pointer to the thread-local errno value. -#[cfg(any(target_os = "linux", target_os = "android"))] -fn libc_errno() -> *mut i32 { - unsafe { __errno_location() } -} - -/// Get a pointer to the thread-local errno value (macOS/BSD). -#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd"))] -fn libc_errno() -> *mut i32 { - unsafe { __error() } -} - /// Simple CRC32 (not CRC32C) for lock file checksumming. fn simple_crc32(data: &[u8]) -> u32 { let mut crc: u32 = 0xFFFFFFFF; diff --git a/crates/rvlite/src/storage/writer_lease.rs b/crates/rvlite/src/storage/writer_lease.rs index 21e1660117..a25dd97dae 100644 --- a/crates/rvlite/src/storage/writer_lease.rs +++ b/crates/rvlite/src/storage/writer_lease.rs @@ -307,8 +307,9 @@ fn is_pid_alive(pid: u32) -> bool { return true; } // EPERM means the process exists but belongs to another user. - let errno = unsafe { *errno_location() }; - errno == 1 // EPERM + // last_os_error reads errno through each platform's own accessor, so this + // works on every Unix rather than only those with a hand-declared symbol. + std::io::Error::last_os_error().raw_os_error() == Some(EPERM) } #[cfg(not(unix))] { @@ -322,31 +323,15 @@ extern "C" { fn kill(pid: i32, sig: i32) -> i32; } -#[cfg(any(target_os = "linux", target_os = "android"))] -extern "C" { - fn __errno_location() -> *mut i32; -} - -#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd"))] -extern "C" { - fn __error() -> *mut i32; -} +/// Permission denied errno -- process exists but belongs to another user. +#[cfg(unix)] +const EPERM: i32 = 1; #[cfg(unix)] unsafe fn libc_kill(pid: i32, sig: i32) -> i32 { unsafe { kill(pid, sig) } } -#[cfg(any(target_os = "linux", target_os = "android"))] -unsafe fn errno_location() -> *mut i32 { - unsafe { __errno_location() } -} - -#[cfg(any(target_os = "macos", target_os = "ios", target_os = "freebsd"))] -unsafe fn errno_location() -> *mut i32 { - unsafe { __error() } -} - #[cfg(test)] mod tests { use super::*; @@ -373,6 +358,23 @@ mod tests { let _ = fs::remove_dir_all(dir); } + #[cfg(unix)] + #[test] + fn is_pid_alive_distinguishes_self_from_an_unused_pid() { + assert!( + is_pid_alive(std::process::id()), + "own process reads as dead" + ); + + // A PID far above every mainstream system's pid_max cannot be running. + // This is the arm that exercises the errno read: kill sets ESRCH, which + // must not be mistaken for EPERM and reported as alive. + assert!( + !is_pid_alive(999999999u32), + "an unused PID reads as alive, so stale leases would never be broken" + ); + } + #[test] fn lock_path_computation() { let p = Path::new("/tmp/store.rvf");