mirror of
https://github.com/vhaudiquet/ecrou.git
synced 2026-11-12 22:22:44 +00:00
fix(supervise): kill and reap the machine when a run errors after the fork
A parent-side failure after the fork (a console stat or read that went wrong, a wait that could not complete) used to return and leave the VM booting unattended. PDEATHSIG only fires when the caller's process exits; a library consumer that catches the error keeps running, and the scratch rootfs goes away under the machine. kill_machine is best effort: a machine that already exited reaps through ECHILD. Also drops a duplicated fork-error block left behind by the supervise split.
This commit is contained in:
1 parent
2e6c84b65b
commit
3d86f30dd3
2 files changed
+60
-10
No files matched your search
@@ -67,13 +67,6 @@ pub(crate) fn spawn_machine(
|
|||||||
}
|
}
|
||||||
bail("start_enter returned".to_string(), 125);
|
bail("start_enter returned".to_string(), 125);
|
||||||
}
|
}
|
||||||
if child < 0 {
|
|
||||||
return Err(Error::Other(
|
|
||||||
anyhow::Error::from(std::io::Error::last_os_error())
|
|
||||||
.context("forking the machine process"),
|
|
||||||
));
|
|
||||||
}
|
|
||||||
|
|
||||||
if child < 0 {
|
if child < 0 {
|
||||||
return Err(Error::Other(
|
return Err(Error::Other(
|
||||||
anyhow::Error::from(std::io::Error::last_os_error())
|
anyhow::Error::from(std::io::Error::last_os_error())
|
||||||
@@ -189,6 +182,28 @@ pub(crate) fn stream_console(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Kill the machine and collect it. Any parent-side failure after the
|
||||||
|
/// fork must land here: a library caller keeps running after a run
|
||||||
|
/// error, and the kill contract (PDEATHSIG) only fires when their
|
||||||
|
/// process exits. Best effort by construction — a machine that already
|
||||||
|
/// exited reaps through ECHILD and the call is a no-op.
|
||||||
|
pub(crate) fn kill_machine(child: libc::pid_t) {
|
||||||
|
// Safety: the pid is our own child; SIGKILL is uncatchable and has
|
||||||
|
// no handler side effects to mind.
|
||||||
|
unsafe { libc::kill(child, libc::SIGKILL) };
|
||||||
|
let mut status: libc::c_int = 0;
|
||||||
|
// Safety: waitpid on our own child; the pointer targets valid
|
||||||
|
// storage.
|
||||||
|
loop {
|
||||||
|
let r = unsafe { libc::waitpid(child, &mut status, 0) };
|
||||||
|
// EINTR: interrupted, not ended — collect again.
|
||||||
|
if r == -1 && std::io::Error::last_os_error().kind() == std::io::ErrorKind::Interrupted {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Collect the machine: its exit status is the payload's exit code (the
|
/// Collect the machine: its exit status is the payload's exit code (the
|
||||||
/// guest init blob reports it and the VMM exits the child with it).
|
/// guest init blob reports it and the VMM exits the child with it).
|
||||||
pub(crate) fn wait_machine(
|
pub(crate) fn wait_machine(
|
||||||
@@ -284,7 +299,11 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn a_signaled_machine_is_an_engine_failure() {
|
fn a_signaled_machine_is_an_engine_failure() {
|
||||||
let dir = tempfile::tempdir().unwrap();
|
let dir = tempfile::tempdir().unwrap();
|
||||||
for raw in [signaled(libc::SIGSEGV), signaled(libc::SIGKILL), signaled(libc::SIGTERM)] {
|
for raw in [
|
||||||
|
signaled(libc::SIGSEGV),
|
||||||
|
signaled(libc::SIGKILL),
|
||||||
|
signaled(libc::SIGTERM),
|
||||||
|
] {
|
||||||
let err = decode_status(raw, &dir.path().join("absent")).unwrap_err();
|
let err = decode_status(raw, &dir.path().join("absent")).unwrap_err();
|
||||||
assert!(
|
assert!(
|
||||||
err.to_string().contains("engine or hypervisor failure"),
|
err.to_string().contains("engine or hypervisor failure"),
|
||||||
@@ -328,4 +347,26 @@ mod tests {
|
|||||||
std::fs::write(&exit_file, "not a number").unwrap();
|
std::fs::write(&exit_file, "not a number").unwrap();
|
||||||
assert_eq!(decode_status(exited(42), &exit_file).unwrap(), 42);
|
assert_eq!(decode_status(exited(42), &exit_file).unwrap(), 42);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn kill_machine_reaps_a_living_child() {
|
||||||
|
// Safety: the child body only pauses and _exits; nothing
|
||||||
|
// allocates after the fork.
|
||||||
|
let child = unsafe { libc::fork() };
|
||||||
|
assert!(child >= 0, "fork: {}", std::io::Error::last_os_error());
|
||||||
|
if child == 0 {
|
||||||
|
loop {
|
||||||
|
unsafe { libc::pause() };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
kill_machine(child);
|
||||||
|
// The child is reaped: waitpid has nothing left to report.
|
||||||
|
let mut status: libc::c_int = 0;
|
||||||
|
let r = unsafe { libc::waitpid(child, &mut status, libc::WNOHANG) };
|
||||||
|
assert_eq!(r, -1);
|
||||||
|
assert_eq!(
|
||||||
|
std::io::Error::last_os_error().raw_os_error(),
|
||||||
|
Some(libc::ECHILD)
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
+11
-2
@@ -286,9 +286,18 @@ pub fn run_prepared(rootfs: &PreparedRootfs, options: &RunOptions) -> Result<i32
|
|||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
let reaped = std::sync::atomic::AtomicI32::new(-1);
|
let reaped = std::sync::atomic::AtomicI32::new(-1);
|
||||||
crate::supervise::stream_console(&console_path, child, &reaped)?;
|
crate::supervise::stream_console(&console_path, child, &reaped)
|
||||||
crate::supervise::wait_machine(child, &reaped, &exit_file_host)
|
.and_then(|()| crate::supervise::wait_machine(child, &reaped, &exit_file_host))
|
||||||
};
|
};
|
||||||
|
// A parent-side failure (a console stat or read that went wrong, a
|
||||||
|
// wait that could not complete) must not leave the machine booting
|
||||||
|
// unattended: a library caller keeps running after a run error,
|
||||||
|
// and the kill contract only reaches through their process exit.
|
||||||
|
// On a run that failed after the child already exited this is a
|
||||||
|
// no-op reaping.
|
||||||
|
if result.is_err() {
|
||||||
|
crate::supervise::kill_machine(child);
|
||||||
|
}
|
||||||
tty_restore.restore();
|
tty_restore.restore();
|
||||||
result
|
result
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user