Skip to content

Commit 40cbb57

Browse files
committed
test(sandbox): harden procfs binary_path tests against fork/exec race and ETXTBSY
Three tests in `procfs::tests` are flaky on arm64 CI containers: * `binary_path_strips_deleted_suffix`, `binary_path_preserves_live_deleted_basename` — `std::process::Command::spawn()` returns once the child is scheduled, not once it has completed `exec()`. On a contended arm64 runner, reading `/proc/<pid>/exe` immediately after spawn can return the parent (test-harness) path for a brief window. The tests then assert the child's exe matches their target path and fail with the harness path on the left. * `binary_path_strips_suffix_for_non_utf8_filename` — sporadically gets ETXTBSY on spawn despite the existing `sync_all()` + scope drop. The writer fd is closed, but the kernel's write-lock release races the immediately-following `execveat()` under load. Adds two test helpers and applies them to the three tests: * `wait_for_child_exec(pid, target)` polls `/proc/<pid>/exe` until the readlink's byte prefix matches `target`, with a 2 s deadline. Tolerates the kernel's `" (deleted)"` suffix on unlinked binaries. * `spawn_retrying_on_etxtbsy(cmd)` retries `Command::spawn()` up to 20 times with a 50 ms backoff when it returns `ETXTBSY`. Any other spawn error surfaces immediately. Both helpers are `#[cfg(target_os = "linux")]` and live in the test module; they don't change any production behaviour. `binary_path` itself is unchanged.
1 parent ae7e901 commit 40cbb57

1 file changed

Lines changed: 61 additions & 12 deletions

File tree

crates/openshell-sandbox/src/procfs.rs

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,55 @@ mod tests {
399399
use super::*;
400400
use std::io::Write;
401401

402+
/// Block until `/proc/<pid>/exe` points at `target`. `std::process::Command::spawn`
403+
/// returns once the child is scheduled, not once it has completed `exec()`. On
404+
/// contended runners (notably arm64 CI containers) the readlink can still return
405+
/// the parent (test harness) binary for a brief window after spawn. Without this
406+
/// wait, tests that inspect the child's exe path race the kernel and observe the
407+
/// harness path instead of the target.
408+
///
409+
/// After `remove_file`, the readlink ends with a literal `" (deleted)"` — we
410+
/// compare byte-level with `starts_with` so the suffix is tolerated.
411+
#[cfg(target_os = "linux")]
412+
fn wait_for_child_exec(pid: i32, target: &std::path::Path) {
413+
use std::os::unix::ffi::OsStrExt as _;
414+
let target_bytes = target.as_os_str().as_bytes();
415+
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2);
416+
loop {
417+
if let Ok(link) = std::fs::read_link(format!("/proc/{pid}/exe"))
418+
&& link.as_os_str().as_bytes().starts_with(target_bytes)
419+
{
420+
return;
421+
}
422+
assert!(
423+
std::time::Instant::now() < deadline,
424+
"child pid {pid} did not exec into {target:?} within 2s"
425+
);
426+
std::thread::sleep(std::time::Duration::from_millis(10));
427+
}
428+
}
429+
430+
/// Spawn a command, retrying on `ETXTBSY`. A freshly-written executable can
431+
/// briefly be unspawnable under concurrent test load — the writer fd has been
432+
/// closed but the kernel's write-lock on the inode hasn't been released yet.
433+
/// Retrying with a small backoff absorbs that window without hiding real
434+
/// spawn failures.
435+
#[cfg(target_os = "linux")]
436+
fn spawn_retrying_on_etxtbsy(
437+
mut cmd: std::process::Command,
438+
) -> std::io::Result<std::process::Child> {
439+
for _ in 0..20 {
440+
match cmd.spawn() {
441+
Ok(child) => return Ok(child),
442+
Err(e) if e.raw_os_error() == Some(libc::ETXTBSY) => {
443+
std::thread::sleep(std::time::Duration::from_millis(50));
444+
}
445+
Err(e) => return Err(e),
446+
}
447+
}
448+
cmd.spawn()
449+
}
450+
402451
#[test]
403452
fn file_sha256_computes_correct_hash() {
404453
let mut tmp = tempfile::NamedTempFile::new().unwrap();
@@ -457,11 +506,11 @@ mod tests {
457506
// child is still running. The child keeps the exec mapping via
458507
// `/proc/<pid>/exe`, but readlink will now return the tainted
459508
// "<path> (deleted)" string.
460-
let mut child = std::process::Command::new(&exe_path)
461-
.arg("5")
462-
.spawn()
463-
.unwrap();
509+
let mut cmd = std::process::Command::new(&exe_path);
510+
cmd.arg("5");
511+
let mut child = spawn_retrying_on_etxtbsy(cmd).unwrap();
464512
let pid: i32 = child.id().cast_signed();
513+
wait_for_child_exec(pid, &exe_path);
465514
std::fs::remove_file(&exe_path).unwrap();
466515

467516
// Sanity check: the raw readlink should contain " (deleted)".
@@ -507,11 +556,11 @@ mod tests {
507556
std::fs::copy("/bin/sleep", &exe_path).unwrap();
508557
std::fs::set_permissions(&exe_path, std::fs::Permissions::from_mode(0o755)).unwrap();
509558

510-
let mut child = std::process::Command::new(&exe_path)
511-
.arg("5")
512-
.spawn()
513-
.unwrap();
559+
let mut cmd = std::process::Command::new(&exe_path);
560+
cmd.arg("5");
561+
let mut child = spawn_retrying_on_etxtbsy(cmd).unwrap();
514562
let pid: i32 = child.id().cast_signed();
563+
wait_for_child_exec(pid, &exe_path);
515564

516565
// File is still linked — binary_path must return the path unchanged,
517566
// suffix and all.
@@ -564,11 +613,11 @@ mod tests {
564613
f.sync_all().expect("sync_all before exec");
565614
}
566615

567-
let mut child = std::process::Command::new(&exe_path)
568-
.arg("5")
569-
.spawn()
570-
.unwrap();
616+
let mut cmd = std::process::Command::new(&exe_path);
617+
cmd.arg("5");
618+
let mut child = spawn_retrying_on_etxtbsy(cmd).unwrap();
571619
let pid: i32 = child.id().cast_signed();
620+
wait_for_child_exec(pid, &exe_path);
572621
std::fs::remove_file(&exe_path).unwrap();
573622

574623
// Sanity: raw readlink ends with " (deleted)" and is not valid UTF-8.

0 commit comments

Comments
 (0)