From c6b4a6ce2b3aa27cf277115bf7f843433468cb9a Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Sun, 5 Jul 2026 14:53:34 +0300 Subject: [PATCH] =?UTF-8?q?fix(bench):=20harden=20the=20harness=20?= =?UTF-8?q?=E2=80=94=20EINTR=20retry,=20loud=20setrlimit=20failure,=20cfg(?= =?UTF-8?q?unix)=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile review follow-ups: wait4 now retries on EINTR (a delivered signal previously panicked the harness mid-run via the reaped-pid assert); a failed setrlimit warns on inherited stderr instead of silently running the child uncapped while the parent JSON records the requested cap; and the Unix-only syscalls are cfg(unix)-gated with an inert Windows stub main so cargo bench compiles everywhere. --- crates/omnigraph/benches/scenarios.rs | 32 ++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/crates/omnigraph/benches/scenarios.rs b/crates/omnigraph/benches/scenarios.rs index 664bc806..fb88081f 100644 --- a/crates/omnigraph/benches/scenarios.rs +++ b/crates/omnigraph/benches/scenarios.rs @@ -23,7 +23,12 @@ //! ignores RLIMIT_AS — the cap variant is primarily a Linux tool, while //! peak-RSS reporting works everywhere). +// The harness is Unix-only (wait4/rusage/setrlimit); a Windows host gets an +// inert stub so `cargo bench`/`cargo build --benches` still compile there. +#![cfg_attr(not(unix), allow(dead_code, unused_imports))] + #[path = "../tests/helpers/mod.rs"] +#[cfg(unix)] mod helpers; use std::fmt::Write as _; @@ -131,6 +136,12 @@ impl Args { // Parent: spawn child, reap with wait4, merge rusage into the JSON record // --------------------------------------------------------------------------- +#[cfg(not(unix))] +fn main() { + eprintln!("the scenario harness requires a Unix platform (wait4/rusage/setrlimit)"); +} + +#[cfg(unix)] fn main() { let args = Args::parse(); if args.scenario.is_empty() { @@ -211,7 +222,14 @@ fn run_once(args: &Args, run: usize) -> serde_json::Value { fn wait4_rusage(pid: i32) -> (i64, u64) { let mut status: libc::c_int = 0; let mut rusage: libc::rusage = unsafe { std::mem::zeroed() }; - let reaped = unsafe { libc::wait4(pid, &mut status, 0, &mut rusage) }; + // Retry on EINTR: a delivered signal (SIGTERM from the shell, SIGCHLD + // from an unrelated child) interrupts the blocking wait with -1. + let reaped = loop { + let r = unsafe { libc::wait4(pid, &mut status, 0, &mut rusage) }; + if r != -1 || std::io::Error::last_os_error().raw_os_error() != Some(libc::EINTR) { + break r; + } + }; assert_eq!(reaped, pid, "wait4 reaped unexpected pid"); let exit: i64 = if libc::WIFEXITED(status) { libc::WEXITSTATUS(status) as i64 @@ -238,8 +256,16 @@ fn run_child(args: &Args) { rlim_max: cap_mb * 1024 * 1024, }; // RLIMIT_AS is enforced on Linux; macOS frequently ignores it. Applied - // best-effort everywhere so the same command line works on both. - unsafe { libc::setrlimit(libc::RLIMIT_AS, &cap) }; + // best-effort everywhere so the same command line works on both — but + // a FAILED setrlimit must be loud (stderr is inherited): the parent's + // JSON records the requested cap, and silently running uncapped would + // misrepresent the test conditions. + if unsafe { libc::setrlimit(libc::RLIMIT_AS, &cap) } != 0 { + eprintln!( + "WARNING: setrlimit(RLIMIT_AS, {cap_mb} MiB) failed ({}); child runs UNCAPPED", + std::io::Error::last_os_error() + ); + } } let runtime = tokio::runtime::Builder::new_multi_thread() .enable_all()