feat(vmm): replace ProcessMonitor polling with pidfd/kqueue event-driven detection

[lilongen]

Summary

• Replace ProcessMonitor::wait_for_exit() 500ms sleep-based polling loop with platform-native event-driven process exit detection
• Linux: pidfd_open() (kernel 5.3+) wrapped in tokio::io::unix::AsyncFd for near-zero latency
• macOS: kqueue + EVFILT_PROC + NOTE_EXIT wrapped in AsyncFd
• Fallback: 100ms polling for older kernels (Linux < 5.3) when event-driven setup fails
• Fixes violation of project Rule Fix silent initialization failures #15: "No Sleep for Events"

Motivation

ProcessMonitor::wait_for_exit() is used in production at guest_connect.rs inside a tokio::select! race to detect VM subprocess death during startup. The 500ms polling loop added up to 500ms latency to crash detection. With pidfd/kqueue, detection is near-instantaneous.

Design

wait_for_exit()
  ├── Fast path: try_wait() → already dead? return immediately
  ├── Linux: pidfd_open() + AsyncFd::readable().await
  ├── macOS: kqueue(EVFILT_PROC, NOTE_EXIT) + AsyncFd::readable().await
  └── Fallback: 100ms polling loop (for kernel < 5.3)

Key safety decisions:

• OwnedFd wraps raw FDs immediately after creation — all error paths are leak-free by construction
• fcntl(O_NONBLOCK) return checked; falls back to polling on failure (required by AsyncFd)
• Block scope for kevent struct (contains *mut c_void, not Send) — dropped before .await
• Best-effort race guard via is_alive() after FD setup, before .await
• No new crate dependencies — uses existing libc + tokio (already in Cargo.toml)

Test plan

• New async test: test_wait_for_exit_captures_exit_code — verifies exit code through event-driven path
• New async test: test_wait_for_exit_detects_delayed_exit — spawns child with 200ms delay, verifies prompt detection
• New async test: test_wait_for_exit_already_dead_pid — non-existent PID returns Unknown immediately
• All 598 existing tests pass on macOS ARM64
• All process tests (31/31) pass on Linux ARM64 (via Lima VM)
• Clippy clean (zero warnings) on both platforms
• cargo fmt --check clean

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR removes the sleep-based polling latency from ProcessMonitor::wait_for_exit() by using event-driven process-exit notifications on Linux/macOS, while retaining a polling fallback path. It also expands jailer FD-cleanup logic to use more efficient platform mechanisms before falling back to brute-force closing.

Changes:

• Implement event-driven process exit detection via Linux pidfd_open() and macOS kqueue/EVFILT_PROC, with a 100ms polling fallback.
• Add async tests covering exit-code capture, delayed exit detection, and non-existent PID behavior.
• Update jailer FD closing strategy to prefer close_range, then /proc/self/fd enumeration via getdents64, then a dynamic brute-force bound via getrlimit.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
boxlite/src/util/process.rs	Replaces polling wait_for_exit() with pidfd/kqueue event-driven waiting plus fallback and adds tests.
boxlite/src/jailer/common/fd.rs	Reworks inherited-FD cleanup to prefer close_range and /proc enumeration, with an RLIMIT-based brute-force fallback and new tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

getdents64 errors are currently treated the same as EOF (if nread <= 0 { break; }), and the function still returns true. If nread < 0 (e.g., EINTR), this will stop enumeration early and skip the brute-force fallback, potentially leaving inherited FDs open. Handle nread < 0 as a failure (close dir_fd and return false).

Suggested change

	if nread <= 0 {
	if nread < 0 {
	// Error while enumerating /proc/self/fd: treat as failure so that
	// callers can fall back to a brute-force close strategy.
	// SAFETY: close() is async-signal-safe. dir_fd was opened by us above.
	unsafe { libc::close(dir_fd) };
	return false;
	}
	if nread == 0 {
	// EOF reached: enumeration completed successfully.

[Copilot]

brute_force_close_fds() relies on get_max_fd() (soft RLIMIT_NOFILE) as an upper bound. If the process opened high-numbered FDs and then lowered its soft limit before fork, rlim_cur can be below the highest open FD, so this loop may miss closing some inherited FDs in the last-resort path. Consider using a more conservative bound (e.g., max(rlim_cur, rlim_max) and/or a minimum floor matching the old hardcoded limits) to avoid FD leaks when falling back to brute force.

[Copilot]

The PR description focuses on ProcessMonitor::wait_for_exit() (pidfd/kqueue), but this file introduces a substantial new FD-closing strategy (close_range + /proc getdents64 + rlimit). If this is intentional, it should be called out in the PR description (or split into a separate PR) so reviewers understand the added scope and security impact.

[Copilot]

fcntl(F_SETFL, O_NONBLOCK) overwrites the FD’s existing status flags. Even if pidfds typically start with 0 flags, it’s safer to preserve existing flags by reading with F_GETFL and OR-ing O_NONBLOCK; otherwise this could unexpectedly clear other status flags if they exist in future kernels/libc behavior.

Suggested change

	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) < 0 {
	return None; // OwnedFd closes the FD on drop
	}
	let mut flags = libc::fcntl(owned.as_raw_fd(), libc::F_GETFL);
	if flags < 0 {
	return None; // OwnedFd closes the FD on drop
	}
	flags |= libc::O_NONBLOCK;
	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFL, flags) < 0 {
	return None; // OwnedFd closes the FD on drop
	}

[Copilot]

Same issue here: fcntl(F_SETFL, O_NONBLOCK) replaces the current file status flags rather than adding non-blocking mode. Prefer F_GETFL + F_SETFL(old | O_NONBLOCK) to avoid clearing any existing flags on the kqueue FD.

Suggested change

	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) < 0 {
	return None; // OwnedFd closes the kqueue FD on drop
	}
	let flags = libc::fcntl(owned.as_raw_fd(), libc::F_GETFL);
	if flags < 0 {
	return None; // OwnedFd closes the kqueue FD on drop
	}
	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFL, flags | libc::O_NONBLOCK) < 0 {
	return None; // OwnedFd closes the kqueue FD on drop
	}

[DorianZheng]

Hi @lilongen. Would you mind add some test cases?

[lilongen]

Hi @lilongen. Would you mind add some test cases?

no problem!

[lilongen]

add some test cases ...

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

get_max_fd() calls libc::getrlimit and the docs assert it is async-signal-safe. In a pre_exec hook (post-fork in a potentially multi-threaded parent), calling non-async-signal-safe libc wrappers can deadlock/hang the spawn path. Consider switching to a raw syscall-based implementation (e.g., syscall(SYS_prlimit64, ...)/syscall(SYS_getrlimit, ...) where available) or avoid querying rlimits in pre_exec and keep a conservative fixed upper bound for the brute-force fallback.

Suggested change

	//! with a dynamic upper bound from `getrlimit(RLIMIT_NOFILE)`.
	//! 3. **macOS**: Brute-force close with dynamic upper bound from
	//! `getrlimit(RLIMIT_NOFILE)` instead of a hardcoded limit.
	/// Query the soft limit for `RLIMIT_NOFILE`. Async-signal-safe.
	///
	/// Returns the soft limit (current max open files), or a safe default
	/// if the query fails. `getrlimit` is async-signal-safe per POSIX.
	///
	/// Note: Cannot reuse `rlimit::get_rlimit()` because it returns `io::Error`
	/// which heap-allocates (not async-signal-safe for pre_exec context).
	fn get_max_fd() -> i32 {
	let mut rlim = libc::rlimit {
	rlim_cur: 0,
	rlim_max: 0,
	};
	// SAFETY: getrlimit is async-signal-safe per POSIX. rlim is a valid stack-allocated struct.
	let result = unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut rlim) };
	if result == 0 && rlim.rlim_cur > 0 {
	if rlim.rlim_cur < i32::MAX as u64 {
	rlim.rlim_cur as i32
	} else {
	// rlim_cur is RLIM_INFINITY or very large; cap to a safe maximum.
	// 1048576 (2^20) matches Linux's default /proc/sys/fs/nr_open.
	1_048_576
	}
	} else {
	// getrlimit failed or returned 0; safe default per POSIX OPEN_MAX
	//! with a dynamic upper bound queried via a raw syscall for
	//! `RLIMIT_NOFILE`.
	//! 3. **Non-Linux**: Brute-force close with a conservative fixed upper bound
	//! to avoid non-async-signal-safe libc wrappers in `pre_exec`.
	/// Query the soft limit for `RLIMIT_NOFILE` without using libc wrappers in
	/// the `pre_exec` path.
	///
	/// On Linux, this uses a raw `prlimit64` syscall to read the current soft
	/// limit for `RLIMIT_NOFILE`. On other targets, it returns a conservative
	/// fixed default to avoid calling potentially non-async-signal-safe libc
	/// wrapper functions after `fork()`.
	fn get_max_fd() -> i32 {
	#[cfg(target_os = "linux")]
	{
	let mut rlim = libc::rlimit64 {
	rlim_cur: 0,
	rlim_max: 0,
	};
	// SAFETY: `syscall` performs a raw kernel syscall without going through
	// higher-level libc wrappers. `rlim` is a valid stack-allocated output
	// buffer, `pid=0` means current process, and `new_limit=NULL` requests
	// a read-only query of the current limit.
	let result = unsafe {
	libc::syscall(
	libc::SYS_prlimit64,
	0 as libc::pid_t,
	libc::RLIMIT_NOFILE,
	core::ptr::null::<libc::rlimit64>(),
	&mut rlim as *mut libc::rlimit64,
	)
	};
	if result == 0 && rlim.rlim_cur > 0 {
	if rlim.rlim_cur < i32::MAX as u64 {
	rlim.rlim_cur as i32
	} else {
	// rlim_cur is RLIM_INFINITY or very large; cap to a safe maximum.
	// 1048576 (2^20) matches Linux's default /proc/sys/fs/nr_open.
	1_048_576
	}
	} else {
	// Syscall failed or returned 0; safe default per POSIX OPEN_MAX.
	1024
	}
	}
	#[cfg(not(target_os = "linux"))]
	{
	// Avoid non-async-signal-safe libc wrapper calls in pre_exec on
	// non-Linux targets; use a conservative fixed fallback instead.

[Copilot]

close_fds_via_proc() uses libc::open(...) even though the comment says “raw syscall”, and it’s executed from pre_exec. If the async-signal-safe constraint is important here, prefer invoking openat via libc::syscall (or otherwise ensure this does not go through libc paths that may take locks/allocate).

Suggested change

	// Open /proc/self/fd with raw syscall (no allocation)
	let proc_path = b"/proc/self/fd\0";
	// SAFETY: proc_path is a valid null-terminated string literal. open() is async-signal-safe.
	let dir_fd = unsafe {
	libc::open(
	proc_path.as_ptr().cast::<libc::c_char>(),
	libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
	)
	// Open /proc/self/fd with a raw syscall (no allocation, no libc wrapper)
	let proc_path = b"/proc/self/fd\0";
	// SAFETY: proc_path is a valid null-terminated string literal. syscall(SYS_openat, ...)
	// invokes the kernel directly, which is appropriate for pre_exec async-signal-safe use.
	let dir_fd = unsafe {
	libc::syscall(
	libc::SYS_openat,
	libc::AT_FDCWD,
	proc_path.as_ptr().cast::<libc::c_char>(),
	libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
	0,
	) as libc::c_int

[Copilot]

The getdents64 loop treats any nread <= 0 as EOF and returns true. If getdents64 returns -1 with EINTR (or another error), this can stop enumeration early and leave some inherited FDs open while still reporting success (skipping the brute-force fallback). Handle EINTR by retrying, and on other errors return false (after closing dir_fd) so the caller can fall back to brute-force closing.

Suggested change

	if nread <= 0 {
	break;
	}
	if nread == 0 {
	break;
	}
	if nread < 0 {
	// SAFETY: errno is thread-local and may be read immediately after
	// the failing syscall to determine whether to retry or fall back.
	let errno = unsafe { *libc::__errno_location() };
	if errno == libc::EINTR {
	continue;
	}
	// SAFETY: close() is async-signal-safe. dir_fd was opened by us above.
	unsafe { libc::close(dir_fd) };
	return false;
	}

[Copilot]

fcntl(F_SETFD, FD_CLOEXEC) return value is ignored. If it fails, the pidfd could leak across an exec in the current process. Please check the return and fall back to polling (or otherwise handle the error) consistently with the O_NONBLOCK failure path.

Suggested change

	libc::fcntl(owned.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC);
	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC) < 0 {
	return None; // OwnedFd closes the FD on drop
	}

[Copilot]

fcntl(F_SETFD, FD_CLOEXEC) return value is ignored. If it fails, the kqueue FD may leak across exec. Please check the return value and fall back to polling on failure (similar to the O_NONBLOCK failure path).

Suggested change

	libc::fcntl(owned.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC);
	if libc::fcntl(owned.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC) < 0 {
	return None; // OwnedFd closes the kqueue FD on drop
	}

[Copilot]

When getrlimit(RLIMIT_NOFILE) fails or returns 0, get_max_fd() falls back to 1024. In the contexts where this function is used (closing inherited FDs for isolation), under-closing is more dangerous than over-closing: if the process has open FDs > 1024, they would remain inherited/leaked. Consider using a more conservative fallback upper bound (even if slower) to avoid leaving high-numbered FDs open when rlimit queries fail.

[Copilot]

In test_wait_for_exit_signal_kill, the return value of libc::kill is ignored. If SIGKILL delivery fails, the test will block until the child’s sleep 60 finishes, slowing/hanging CI. Please assert kill(...) == 0 (or handle failure by terminating the child another way / reducing the child sleep duration).

Suggested change

	unsafe { libc::kill(pid as i32, libc::SIGKILL) };
	assert_eq!(
	unsafe { libc::kill(pid as i32, libc::SIGKILL) },
	0,
	"Failed to send SIGKILL to child process"
	);

[lilongen]

Added 3 test cases per review feedback

Commit: 43d1a98

1. test_wait_for_exit_signal_kill (HIGH value)

Spawns sh -c "sleep 60", sends SIGKILL via libc::kill(), verifies ProcessExit::Code(128 + 9). Covers the WIFSIGNALED decode path through event-driven detection — existing tests only covered normal exits.

2. test_wait_for_exit_concurrent_monitors (HIGH value)

Spawns 3 children with different exit codes (1, 2, 3). Creates 3 ProcessMonitors, tokio::join! all three wait_for_exit() calls. Verifies each gets the correct exit code. Tests concurrent pidfd/kqueue FD registrations.

3. test_wait_polling_captures_exit_code (MEDIUM value)

Calls wait_polling() directly (private async fn, accessible from same-module tests). Spawns sh -c "exit 5", verifies exit code. Validates the 100ms polling fallback path independently of event-driven setup.

Verification

• macOS ARM64: 26/26 process tests pass, clippy clean, fmt clean
• Linux ARM64 (Lima): 26/26 process tests pass