fix(lifecycle): wait for a Darwin exit to register after EPERM
XNU stops signalling a process as soon as it starts exiting but posts NOTE_EXIT later in the same exit. A KILL that lands in that window gets EPERM while the exit probe still reads "alive", so stopping a child that TERM had just ended could fail with "Operation not permitted". This flaked the macOS run of contained_exit_probe_preserves_a_live_child. The EPERM branch now re-probes for up to 250 ms before treating the error as a live, unsignalable root. Live roots, reaped roots and probe failures are still errors.
This commit is contained in:
@@ -205,5 +205,6 @@ reconciliation on every desktop platform.
|
|||||||
|
|
||||||
A process that exits while shutdown is signalling it can report a macOS
|
A process that exits while shutdown is signalling it can report a macOS
|
||||||
permission error. VoiceStudio accepts this only after confirming the original
|
permission error. VoiceStudio accepts this only after confirming the original
|
||||||
process exited without being reaped, then still waits for nested operations to
|
process exited without being reaped (macOS can take a moment to report that
|
||||||
drain. Live-process permission errors and lost process ownership remain failures.
|
exit, so it waits up to a quarter of a second), then still waits for nested
|
||||||
|
operations to drain. Live-process permission errors and lost process ownership remain failures.
|
||||||
|
|||||||
@@ -73,14 +73,23 @@ pub struct OwnedProcessTree {
|
|||||||
job: std::os::windows::io::OwnedHandle,
|
job: std::os::windows::io::OwnedHandle,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// How long a Darwin EPERM waits for the root's exit to register. XNU stops
|
||||||
|
// signalling a process as soon as it starts exiting, but posts NOTE_EXIT only
|
||||||
|
// later in the same exit, so a KILL landing in between sees EPERM while the
|
||||||
|
// exit probe still reads "alive". The gap is normally microseconds; this
|
||||||
|
// bounds the wait for a root that really is alive and unsignalable.
|
||||||
|
#[cfg(unix)]
|
||||||
|
const DARWIN_EXIT_SETTLE: Duration = Duration::from_millis(250);
|
||||||
|
|
||||||
// Keep the delivery and post-error ownership probe together: the root may
|
// Keep the delivery and post-error ownership probe together: the root may
|
||||||
// exit between any earlier liveness check and either TERM or KILL delivery.
|
// exit between any earlier liveness check and either TERM or KILL delivery.
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
fn signal_process_group_with(
|
fn signal_process_group_with(
|
||||||
signal: libc::c_int,
|
signal: libc::c_int,
|
||||||
darwin: bool,
|
darwin: bool,
|
||||||
|
settle: Duration,
|
||||||
send: impl FnOnce(libc::c_int) -> io::Result<()>,
|
send: impl FnOnce(libc::c_int) -> io::Result<()>,
|
||||||
root_exited_unreaped: impl FnOnce() -> io::Result<bool>,
|
mut root_exited_unreaped: impl FnMut() -> io::Result<bool>,
|
||||||
) -> io::Result<()> {
|
) -> io::Result<()> {
|
||||||
match send(signal) {
|
match send(signal) {
|
||||||
Ok(()) => Ok(()),
|
Ok(()) => Ok(()),
|
||||||
@@ -90,10 +99,15 @@ fn signal_process_group_with(
|
|||||||
// during delivery. Accept only a newly verified unreaped root:
|
// during delivery. Accept only a newly verified unreaped root:
|
||||||
// live roots, lost identity and probe failures remain errors.
|
// live roots, lost identity and probe failures remain errors.
|
||||||
// Callers must still join nested drain before reaping that root.
|
// Callers must still join nested drain before reaping that root.
|
||||||
if root_exited_unreaped()? {
|
let deadline = std::time::Instant::now() + settle;
|
||||||
Ok(())
|
loop {
|
||||||
} else {
|
if root_exited_unreaped()? {
|
||||||
Err(error)
|
return Ok(());
|
||||||
|
}
|
||||||
|
if std::time::Instant::now() >= deadline {
|
||||||
|
return Err(error);
|
||||||
|
}
|
||||||
|
std::thread::sleep(Duration::from_millis(1));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(error) => Err(error),
|
Err(error) => Err(error),
|
||||||
@@ -171,6 +185,7 @@ impl OwnedProcessTree {
|
|||||||
signal_process_group_with(
|
signal_process_group_with(
|
||||||
signal,
|
signal,
|
||||||
cfg!(target_os = "macos"),
|
cfg!(target_os = "macos"),
|
||||||
|
DARWIN_EXIT_SETTLE,
|
||||||
|signal| {
|
|signal| {
|
||||||
if unsafe { libc::kill(-self.process_group, signal) } == 0 {
|
if unsafe { libc::kill(-self.process_group, signal) } == 0 {
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -1224,6 +1239,7 @@ mod uv_tests {
|
|||||||
signal_process_group_with(
|
signal_process_group_with(
|
||||||
signal,
|
signal,
|
||||||
true,
|
true,
|
||||||
|
Duration::ZERO,
|
||||||
|delivered| {
|
|delivered| {
|
||||||
assert_eq!(delivered, signal);
|
assert_eq!(delivered, signal);
|
||||||
assert!(!exited.replace(true));
|
assert!(!exited.replace(true));
|
||||||
@@ -1238,6 +1254,26 @@ mod uv_tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(unix)]
|
||||||
|
#[test]
|
||||||
|
fn darwin_group_signal_waits_for_an_exit_that_registers_after_eperm() {
|
||||||
|
// The flake behind contained_exit_probe_preserves_a_live_child: TERM
|
||||||
|
// started the exit, KILL got EPERM, and NOTE_EXIT had not arrived yet.
|
||||||
|
let probes = std::cell::Cell::new(0);
|
||||||
|
signal_process_group_with(
|
||||||
|
libc::SIGKILL,
|
||||||
|
true,
|
||||||
|
Duration::from_secs(5),
|
||||||
|
|_| Err(io::Error::from_raw_os_error(libc::EPERM)),
|
||||||
|
|| {
|
||||||
|
probes.set(probes.get() + 1);
|
||||||
|
Ok(probes.get() >= 3)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.expect("an exit that registers within the settle window is accepted");
|
||||||
|
assert_eq!(probes.get(), 3);
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
#[test]
|
#[test]
|
||||||
fn darwin_group_signal_rejects_live_reaped_or_unverifiable_roots() {
|
fn darwin_group_signal_rejects_live_reaped_or_unverifiable_roots() {
|
||||||
@@ -1250,6 +1286,7 @@ mod uv_tests {
|
|||||||
let error = signal_process_group_with(
|
let error = signal_process_group_with(
|
||||||
signal,
|
signal,
|
||||||
true,
|
true,
|
||||||
|
Duration::from_millis(20),
|
||||||
|_| Err(io::Error::from_raw_os_error(libc::EPERM)),
|
|_| Err(io::Error::from_raw_os_error(libc::EPERM)),
|
||||||
|| probe.map_err(io::Error::from_raw_os_error),
|
|| probe.map_err(io::Error::from_raw_os_error),
|
||||||
)
|
)
|
||||||
@@ -1266,6 +1303,7 @@ mod uv_tests {
|
|||||||
let error = signal_process_group_with(
|
let error = signal_process_group_with(
|
||||||
libc::SIGKILL,
|
libc::SIGKILL,
|
||||||
darwin,
|
darwin,
|
||||||
|
Duration::ZERO,
|
||||||
|_| Err(io::Error::from_raw_os_error(errno)),
|
|_| Err(io::Error::from_raw_os_error(errno)),
|
||||||
|| panic!("unrelated errors must not use the Darwin exit exception"),
|
|| panic!("unrelated errors must not use the Darwin exit exception"),
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user