diff --git a/apps/nesinit/src/filesystems.rs b/apps/nesinit/src/filesystems.rs index 005c40b3..e6254ff0 100644 --- a/apps/nesinit/src/filesystems.rs +++ b/apps/nesinit/src/filesystems.rs @@ -87,6 +87,19 @@ const EARLY: &[Early] = &[ data: "", cost: "a workload that looks up a device finds nothing", }, + // One cgroup per launch, so that a stop reaches what a launch started + // however many sessions it moved them to (Wine does). `cgroup.kill` needs + // no controller. A kernel without cgroup v2 fails this and launches are + // found by their environment instead. + Early { + source: "cgroup2", + target: "/sys/fs/cgroup", + fstype: "cgroup2", + flags: NOSUID_NODEV | libc::MS_NOEXEC, + data: "", + cost: "a stop cannot reach the processes a launch moved to sessions of \ + their own, and is left to find them by environment", + }, // Writable, and the reason any of this is here. Both sockets in this // component live on a tmpfs because the root they would otherwise sit on // is read-only. diff --git a/apps/nesinit/src/services.rs b/apps/nesinit/src/services.rs index 4c396edd..dc38ee1c 100644 --- a/apps/nesinit/src/services.rs +++ b/apps/nesinit/src/services.rs @@ -752,7 +752,7 @@ impl Drop for Stack { /// and bind afresh, which is a new inode every time. type Identity = (u64, u64); -fn identity_of(path: &str) -> Option { +pub(crate) fn identity_of(path: &str) -> Option { use std::os::unix::fs::MetadataExt; std::fs::metadata(path).ok().map(|at| (at.dev(), at.ino())) } @@ -769,7 +769,7 @@ fn is_alive(pid: i32) -> bool { /// The waiting itself, with the deadline passed in so a test can assert the /// giving-up without waiting out a real one. -fn await_path( +pub(crate) fn await_path( name: &str, path: &str, before: Option, diff --git a/apps/nesinit/src/session.rs b/apps/nesinit/src/session.rs index 9c44e28d..4edff5a2 100644 --- a/apps/nesinit/src/session.rs +++ b/apps/nesinit/src/session.rs @@ -158,6 +158,8 @@ where // resolves immediately and forever, so an unguarded branch on one turns // this loop into a spin that still looks like it is waiting. let mut services_open = true; + // The box's compositor exit, once there is a compositor. + let mut compositor_exit: Option = None; loop { // The exits are guarded like the channels below: an empty set resolves @@ -177,6 +179,9 @@ where up = payload.from_workload.recv(), if relay_open => Event::FromWorkload(up), found = addresses.recv(), if carrier_open => Event::Address(found), gone = services.deaths().recv(), if services_open => Event::ServiceDied(gone), + gone = async { compositor_exit.as_mut().expect("guarded").await }, if compositor_exit.is_some() => { + Event::CompositorDied(gone) + }, }; let line = match event { @@ -281,6 +286,18 @@ where send(&mut writer, &GuestToHost::ServiceDied { name, exit }).await?; continue; } + Event::CompositorDied(gone) => { + // Every launch draws on it, so with it gone nothing in this box + // can show a window: said as a service death, then the box + // ends rather than carrying on looking healthy. + let exit = gone.unwrap_or_else(|_| Exit::code(1)); + tracing::error!(?exit, "the compositor has exited: no window in this box can draw"); + send(&mut writer, &GuestToHost::ServiceDied { name: "nescope".into(), exit }).await?; + workload.signal_stop(); + return Ok(Outcome::Refused(Failure::new(format!( + "the compositor exited ({exit:?}), so the box can no longer draw" + )))); + } Event::ServiceDied(None) => { // The supervisor is gone, which on the way down is ordinary. // Nothing to report and nothing to end: a box with no service @@ -401,8 +418,17 @@ where // A box whose own services will not come up cannot be launched // into, so this is refused rather than reported and carried on // from — unlike a launch, which is the caller's to correct. - match services.bring_up(descriptor.video) { + match services.bring_up(descriptor.video).and_then(|mut up| { + // After the services and before the box is ready: the + // first launch joins it like every other. ref(d-0085) + if let Some(compositor) = &descriptor.compositor { + workload.start_compositor(compositor)?; + up.push("nescope".to_string()); + } + Ok(up) + }) { Ok(up) => { + compositor_exit = workload.compositor_exit(); tracing::info!(services = up.len(), "the box is ready to be launched into"); send(&mut writer, &GuestToHost::Initialized { services: up }).await? } @@ -643,6 +669,7 @@ enum Event { FromWorkload(Option), Address(Option), ServiceDied(Option), + CompositorDied(std::io::Result), } /// Hand an envelope to the relay, and treat a relay that is not there as the @@ -697,6 +724,7 @@ mod tests { overlays: Vec::new(), images: Vec::new(), video: Default::default(), + compositor: None, } } @@ -895,6 +923,105 @@ mod tests { ); } + /// The box's compositor is the box's, started once with the descriptor and + /// before anything is launched. ref(d-0085) + #[tokio::test] + async fn the_compositor_is_started_by_the_box_and_not_by_a_launch() { + let (guest, host) = tokio::io::duplex(4096); + let mut caller = Caller::new(host); + let session = spawn(guest, Given::new(Double::exits_when_stopped(Exit::code(0)))); + + let mut given = descriptor(); + given.compositor = Some(nesprotocol::lifecycle::Compositor { + width: 1280, + height: 720, + fps: 30, + uid: 1001, + gid: 1001, + env: Default::default(), + }); + caller.expect_ready().await; + caller + .say(&HostToGuest::Boot { + descriptor: Box::new(given.clone()), + }) + .await; + caller.expect_booted().await; + caller.say(&HostToGuest::Shutdown).await; + + let (_, workload, _) = session.await.unwrap(); + assert_eq!(workload.compositors, vec![given.compositor.unwrap()]); + assert!(workload.started.is_empty()); + } + + /// A box whose compositor will not come up cannot be drawn on, so it is + /// refused like a box whose services will not. + #[tokio::test] + async fn a_compositor_that_will_not_start_refuses_the_box() { + let (guest, host) = tokio::io::duplex(4096); + let mut caller = Caller::new(host); + let mut double = Double::exits_when_stopped(Exit::code(0)); + double.compositor_failure = Some(Failure::new("no gpu")); + let session = spawn(guest, Given::new(double)); + + let mut given = descriptor(); + given.compositor = Some(nesprotocol::lifecycle::Compositor { + width: 1280, + height: 720, + fps: 30, + uid: 1001, + gid: 1001, + env: Default::default(), + }); + caller.expect_ready().await; + caller + .say(&HostToGuest::Boot { + descriptor: Box::new(given), + }) + .await; + assert_eq!(caller.expect().await, GuestToHost::Mounted); + assert_eq!( + caller.expect().await, + GuestToHost::InitFailed { + reason: "no gpu".into() + } + ); + let (outcome, _, _) = session.await.unwrap(); + assert!(matches!(outcome, Outcome::Refused(_)), "{outcome:?}"); + } + + /// A compositor that exits ends the box, and the host is told which + /// service it was. + #[tokio::test] + async fn a_compositor_that_exits_is_reported_and_ends_the_box() { + let (guest, host) = tokio::io::duplex(4096); + let mut caller = Caller::new(host); + let (tx, rx) = tokio::sync::oneshot::channel(); + let mut double = Double::exits_when_stopped(Exit::code(0)); + double.compositor_exit = Some(rx); + let session = spawn(guest, Given::new(double)); + + let mut given = descriptor(); + given.compositor = Some(nesprotocol::lifecycle::Compositor { + width: 1280, + height: 720, + fps: 30, + uid: 1001, + gid: 1001, + env: Default::default(), + }); + caller.expect_ready().await; + caller.say(&HostToGuest::Boot { descriptor: Box::new(given) }).await; + caller.expect_booted().await; + tx.send(Exit::signal(11)).unwrap(); + assert_eq!( + caller.expect().await, + GuestToHost::ServiceDied { name: "nescope".into(), exit: Exit::signal(11) } + ); + let (outcome, _, _) = session.await.unwrap(); + assert!(matches!(outcome, Outcome::Refused(_)), "{outcome:?}"); + } + #[tokio::test] async fn the_descriptors_video_limits_reach_the_services() { // The ceiling is useless if it stops at the descriptor. `neshub` is the diff --git a/apps/nesinit/src/workload.rs b/apps/nesinit/src/workload.rs index a3ad4d48..6684bc7d 100644 --- a/apps/nesinit/src/workload.rs +++ b/apps/nesinit/src/workload.rs @@ -9,10 +9,11 @@ use std::ffi::CString; use std::future::Future; use std::io; +use std::os::unix::ffi::OsStringExt as _; use std::pin::Pin; use nesprotocol::lifecycle::{ - AttachId, Exec, Exit, Image, LAUNCH_ID_ENV, LaunchId, Mount, Overlay, Winsize, + AttachId, Compositor, Exec, Exit, Image, LAUNCH_ID_ENV, LaunchId, Mount, Overlay, Winsize, }; use std::os::unix::process::CommandExt; @@ -84,9 +85,26 @@ pub trait Workload { /// with any of their games in it, not one game's. ref(d-0085) fn start(&mut self, id: &LaunchId, exec: &Exec) -> Result; - /// Ask one launch to stop. Idempotent, and never ends the session by - /// itself. A launch that is not running is not signalled: its pid may - /// already belong to something else. + /// Start the box's one compositor, and return once it is serving. + /// + /// Owned by the box and not by any launch: every launch draws on it, so no + /// launch's stop may reach it. ref(d-0085) + fn start_compositor(&mut self, compositor: &Compositor) -> Result<(), Failure>; + + /// The compositor's exit, once, after [`Workload::start_compositor`] has + /// returned. The session reports it and ends the box: a box that cannot + /// draw is no use to anyone in it. `None` when there is no compositor or it + /// was already taken. + fn compositor_exit(&mut self) -> Option { + None + } + + /// Ask one launch to stop: its whole process group, not its top process. + /// Idempotent, and never ends the session by itself. A launch that is not + /// running is not signalled: its pid may already belong to something else. + /// + /// The launch's exit is delivered when its group is gone, so it reports + /// the game's window closed rather than only the top process having left. fn stop_launch(&mut self, id: &LaunchId); /// Ask every launch to stop. Idempotent, and never ends the session by @@ -101,6 +119,293 @@ pub struct Process { /// delivered and `Watched::running` is what tells the two apart; it is /// replaced when the same id is started again (a restart). running: std::collections::BTreeMap, + /// The group of each launch in `running`, which outlives its top process. + groups: std::collections::BTreeMap>, + /// Where launch cgroups are made: [`CGROUP_ROOT`], except in a test. + cgroup_root: String, + /// The box's compositor, if it has one. Not a launch: nothing in + /// `running` is it, and a launch's stop cannot reach it. + compositor: Option, + /// The compositor's exit, until the session takes it. See + /// [`Workload::compositor_exit`]. + compositor_exit: Option, +} + +/// How long a launch's group has to leave after being asked before it is +/// killed. +pub(crate) const STOP_GRACE: std::time::Duration = std::time::Duration::from_secs(5); + +/// The compositor's socket, in its user's runtime directory. nescope's own +/// default, and what a launch is told to join. +pub(crate) const COMPOSITOR_SOCKET: &str = "nescope-0"; + +/// The compositor's binary in the guest image. +const COMPOSITOR_BIN: &str = "/usr/bin/nescope"; + +/// How long the compositor has to publish its socket. +const COMPOSITOR_READY: std::time::Duration = std::time::Duration::from_secs(15); + +/// Every live process in session `sid`. +/// +/// A launch is started in a session of its own (`setsid`), so its session id is +/// its top pid and everything it starts inherits it. Read from `/proc` rather +/// than asked of the kernel with `kill(-pgid)` alone because a Wine or a +/// launcher may move its own children to groups of their own, and a group that +/// has been left is still in the session. Zombies are not members: the reaper +/// clears them, and waiting on one would wait on nobody. +pub(crate) fn session_members(sid: i32) -> Vec { + let Ok(dir) = std::fs::read_dir("/proc") else { + return Vec::new(); + }; + dir.flatten() + .filter_map(|entry| entry.file_name().to_str()?.parse::().ok()) + .filter(|pid| in_session(*pid, sid)) + .collect() +} + +/// Whether `pid` is a live (not zombie) member of session `sid`. +fn in_session(pid: i32, sid: i32) -> bool { + std::fs::read_to_string(format!("/proc/{pid}/stat")) + .ok() + .and_then(|stat| live_session_of(&stat)) + == Some(sid) +} + +/// The session a `/proc//stat` line belongs to, or `None` for a process +/// that is a zombie or whose line cannot be read. +/// +/// `pid (comm) state ppid pgrp session ...`, with a comm that may hold spaces +/// and parentheses, so it is the last `)` that ends it. +fn live_session_of(stat: &str) -> Option { + let (_, after) = stat.rsplit_once(')')?; + let mut fields = after.split_whitespace(); + let state = fields.next()?; + let session = fields.nth(2)?.parse::().ok()?; + (state != "Z" && state != "X").then_some(session) +} + +/// Where launch cgroups live: the cgroup v2 root, mounted by `filesystems`. +const CGROUP_ROOT: &str = "/sys/fs/cgroup"; + +/// Everything that belongs to one launch. +/// +/// A session is not enough: Wine `setsid`s its own processes (wineserver, +/// services.exe, explorer.exe and the game itself each have a session of their +/// own), so a stop that signalled the launch's session left the game running +/// with its window open. Measured 2026-10-18 on the test box. +/// +/// What a child cannot leave is a cgroup, so a launch is put in one before it +/// execs and the kernel's own list of its members is what is signalled. Where +/// no cgroup could be made (v2 not mounted) the members are the launch's +/// session plus every process whose environment names the launch, which Wine +/// cannot drop either: it is inherited. +#[derive(Debug)] +pub(crate) struct Group { + id: String, + /// The top process, which is also the session id. + sid: i32, + cgroup: Option, + /// Set when the same id is started again: this group is no longer the + /// launch's, and nothing waiting on it may act for the launch. + superseded: std::sync::atomic::AtomicBool, +} + +/// Numbers each group, so two launches of one id never share a cgroup. +static GENERATION: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +/// An id as a directory name: only what is safe in a path. +fn sanitise(id: &str) -> String { + id.chars() + .map(|c| if c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' { c } else { '_' }) + .collect() +} + +/// Make the cgroup for launch `id`, generation `generation`, under `base`, or +/// `None` when it cannot be (no cgroup v2, no permission, or one of that name +/// already). The generation is in the name so a restart never shares a +/// directory with what is left of the launch before it. +fn make_cgroup(base: &str, id: &str, generation: u64) -> Option { + let path = std::path::Path::new(base).join(format!("nestri-launch-{}-{generation}", sanitise(id))); + match std::fs::create_dir(&path) { + Ok(()) => Some(path), + Err(error) => { + tracing::warn!(%id, base, "no cgroup for this launch ({error}); its members are found by environment"); + None + } + } +} + +/// Whether `pid` is listed in the cgroup at `path`: the proof the launch's +/// child joined it, which its own unchecked write could not give. +fn joined(path: &std::path::Path, pid: i32) -> bool { + std::fs::read_to_string(path.join("cgroup.procs")) + .map(|procs| procs.lines().any(|line| line.trim().parse::().ok() == Some(pid))) + .unwrap_or(false) +} + +/// Whether `pid` is not a zombie. +fn is_live(pid: i32) -> bool { + std::fs::read_to_string(format!("/proc/{pid}/stat")) + .ok() + .and_then(|stat| live_session_of(&stat)) + .is_some() +} + +/// Whether the NUL-separated environment `environ` carries `NAME=value`. +fn environ_names(environ: &[u8], id: &str) -> bool { + let want = format!("{LAUNCH_ID_ENV}={id}"); + environ.split(|b| *b == 0).any(|entry| entry == want.as_bytes()) +} + +impl Group { + /// The group of a launch whose top process is `sid`. + fn new(id: &LaunchId, sid: i32, cgroup: Option) -> Self { + Self { id: id.as_str().to_string(), sid, cgroup, superseded: false.into() } + } + + /// Whether the launch has been started again since this group was made. + fn is_superseded(&self) -> bool { + self.superseded.load(std::sync::atomic::Ordering::SeqCst) + } + + /// Every live process of the launch. + pub(crate) fn members(&self) -> Vec { + if let Some(path) = &self.cgroup { + if let Ok(procs) = std::fs::read_to_string(path.join("cgroup.procs")) { + let found: Vec = procs + .lines() + .filter_map(|line| line.trim().parse::().ok()) + .filter(|pid| is_live(*pid)) + .collect(); + // An empty cgroup with the top process alive is a cgroup the + // launch is not in: the scan below is what finds it. + if !found.is_empty() || !in_session(self.sid, self.sid) { + return found; + } + } + } + let mut members = session_members(self.sid); + if let Ok(dir) = std::fs::read_dir("/proc") { + for pid in dir.flatten().filter_map(|e| e.file_name().to_str()?.parse::().ok()) { + if members.contains(&pid) || !is_live(pid) { + continue; + } + let named = std::fs::read(format!("/proc/{pid}/environ")) + .map(|environ| environ_names(&environ, &self.id)) + .unwrap_or(false); + if named { + members.push(pid); + } + } + } + members + } + + /// Signal everything of the launch. + pub(crate) fn signal(&self, signal: libc::c_int) { + let members = self.members(); + if members.is_empty() { + // Only a number again: a group call now could reach whatever the + // kernel gave it to. + return; + } + // SAFETY: integers only; a group that has gone fails with ESRCH. + unsafe { libc::kill(-self.sid, signal) }; + for pid in members { + // SAFETY: as above. + unsafe { libc::kill(pid, signal) }; + } + } + + /// End everything of the launch now, with no chance to refuse. + pub(crate) fn kill(&self) { + if let Some(path) = &self.cgroup { + if std::fs::write(path.join("cgroup.kill"), "1").is_ok() { + return; + } + // Before Linux 5.14 there is no `cgroup.kill`: kill by pid. + } + self.signal(libc::SIGKILL); + } + + /// Remove the launch's cgroup, once it is empty. A zombie still counts as + /// a member to the kernel until it is reaped, so `EBUSY` is retried for a + /// moment before it is given up on. + async fn remove(&self) { + let Some(path) = &self.cgroup else { return }; + let mut error = None; + for _ in 0..40 { + match std::fs::remove_dir(path) { + Ok(()) => return, + Err(e) if e.kind() == io::ErrorKind::NotFound => return, + Err(e) => error = Some(e), + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + if let Some(error) = error { + tracing::warn!(path = %path.display(), "a launch's cgroup was not removed: {error}"); + } + } +} + +/// Wait for a launch's group to be gone, ending it if it will not go alone. +/// +/// Reached once the top process has been reaped. When it leaves, whatever it +/// started is ended with it (TERM, then KILL after the grace): a launch is not +/// reported ended while something of it is still running, and what it left +/// behind is not kept alive. +async fn group_gone(group: std::sync::Arc) { + if group.members().is_empty() { + group.remove().await; + return; + } + tracing::info!(sid = group.sid, "a launch's top process has gone and its group has not: ending the group"); + group.signal(libc::SIGTERM); + let deadline = std::time::Instant::now() + STOP_GRACE; + let mut killed = false; + // Past the grace plus as long again: a process in uninterruptible sleep + // ignores SIGKILL, and a launch that never ends is worse than one that + // ended with something stuck in it. + let give_up = deadline + STOP_GRACE * 2; + loop { + if group.members().is_empty() { + group.remove().await; + return; + } + let now = std::time::Instant::now(); + if now >= give_up { + tracing::warn!(sid = group.sid, "a launch's group would not die; reporting the launch ended anyway"); + group.remove().await; + return; + } + if now >= deadline && !killed { + killed = true; + group.kill(); + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } +} + +/// Stop a group: TERM now, then `cgroup.kill` (or KILL) after the grace if it +/// is still there. On a thread of its own, because the caller cannot wait and +/// the runtime may be busy with the exits it is meant to deliver. +fn stop_group(group: std::sync::Arc) { + group.signal(libc::SIGTERM); + std::thread::spawn(move || { + let deadline = std::time::Instant::now() + STOP_GRACE; + while std::time::Instant::now() < deadline { + // The launch was started again under this id: what is in the + // environment-named fallback is now the new launch's, and a kill + // here would be its death. + if group.is_superseded() || group.members().is_empty() { + return; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + if !group.is_superseded() { + group.kill(); + } + }); } impl Process { @@ -110,17 +415,29 @@ impl Process { Self { waiters, running: std::collections::BTreeMap::new(), + groups: std::collections::BTreeMap::new(), + cgroup_root: CGROUP_ROOT.to_string(), + compositor: None, + compositor_exit: None, } } /// Send a signal to the workload and nothing else. /// - /// Nothing here ever signals the process group or every process: the order - /// a shutdown promises is only true if the workload can be stopped alone. + /// The launches' top processes only, never every process: the order a + /// shutdown promises is only true if the workload can be stopped alone. + /// The one exception is SIGKILL, which is the end of the order and takes + /// each launch's whole group; a launch's own stop does too + /// ([`Workload::stop_launch`]). pub fn signal(&self, signal: libc::c_int) { for watched in self.running.values() { Self::signal_one(watched, signal); } + if signal == libc::SIGKILL { + for group in self.groups.values() { + group.kill(); + } + } } fn signal_one(watched: &Watched, signal: libc::c_int) { @@ -305,11 +622,38 @@ impl Workload for Process { }; command.envs(launch_environment(exec, runtime.as_deref(), id)); + // The launch's cgroup, made here and joined before exec: what a child + // that makes its own session still cannot leave. The path is built + // now because the closure below may not allocate. + let generation = GENERATION.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + let cgroup = make_cgroup(&self.cgroup_root, id.as_str(), generation); + let procs_file = cgroup + .as_ref() + .and_then(|path| std::ffi::CString::new(path.join("cgroup.procs").into_os_string().into_vec()).ok()); + // SAFETY: the closure runs between fork and exec in the child, where - // only async-signal-safe calls are allowed. These two are, and it + // only async-signal-safe calls are allowed. These are, and it // allocates nothing. unsafe { command.pre_exec(move || { + // A session of its own: the launch is a group that can be + // stopped alone, which is what makes one launch's stop leave + // another's windows open. Before the drops below, which do + // not care. + if libc::setsid() == -1 { + return Err(io::Error::last_os_error()); + } + // Into the launch's cgroup, as root, before the uid drops. + // A failure is not one here, where nothing may be said: the + // parent checks the child is in the cgroup after the spawn and + // falls back to the environment when it is not. + if let Some(file) = &procs_file { + let fd = libc::open(file.as_ptr(), libc::O_WRONLY | libc::O_CLOEXEC); + if fd >= 0 { + libc::write(fd, b"0".as_ptr().cast(), 1); + libc::close(fd); + } + } // gid first: dropping the uid first would lose the privilege // needed to set the gid at all. if libc::setgid(gid) != 0 { @@ -326,6 +670,9 @@ impl Workload for Process { .waiters .watch(|| Ok(command.spawn()?.id() as i32)) .map_err(|error| { + if let Some(path) = &cgroup { + let _ = std::fs::remove_dir(path); + } // The program, the user, and what the system said. // // `Permission denied` on its own is the least useful true @@ -346,15 +693,126 @@ impl Workload for Process { self.running.retain(|_, watched| watched.running()); self.running.insert(id.clone(), watched); + // The session id is the top pid (`setsid` above), and stays that + // number for as long as anything is in it. + let sid = self.running.get(id).map(|w| w.pid).expect("just inserted"); + // The join was unchecked in the child. A cgroup the child is not in + // would read as an empty launch, and a stop of it would do nothing, so + // it is dropped for the session-and-environment fallback. + let cgroup = match cgroup { + Some(path) if !joined(&path, sid) => { + tracing::warn!( + %id, + path = %path.display(), + "the launch did not join its cgroup; its members are found by environment" + ); + let _ = std::fs::remove_dir(&path); + None + } + other => other, + }; + // Groups with nothing left in them are dropped here, as `running` is. + self.groups.retain(|_, group| !group.members().is_empty()); + let group = std::sync::Arc::new(Group::new(id, sid, cgroup)); + if let Some(old) = self.groups.insert(id.clone(), group.clone()) { + old.superseded.store(true, std::sync::atomic::Ordering::SeqCst); + } Ok(Box::pin(async move { - exit.await - .map_err(|_| io::Error::other("the workload's exit was not delivered")) + let exit = exit + .await + .map_err(|_| io::Error::other("the workload's exit was not delivered"))?; + // The launch ended when its group has, not when its top process + // did. See `group_gone`. + group_gone(group).await; + Ok(exit) })) } + fn start_compositor(&mut self, compositor: &Compositor) -> Result<(), Failure> { + let (uid, gid) = (compositor.uid, compositor.gid); + let runtime = crate::system::runtime_dir(uid, gid).map_err(|error| { + Failure::new(format!("no runtime directory for the compositor (uid {uid}): {error}")) + })?; + let exec = Exec { + argv: vec![ + COMPOSITOR_BIN.to_string(), + "--width".into(), + compositor.width.to_string(), + "--height".into(), + compositor.height.to_string(), + "--fps".into(), + compositor.fps.to_string(), + ], + env: compositor.env.clone(), + cwd: None, + uid, + gid, + }; + let mut command = std::process::Command::new(&exec.argv[0]); + command.args(&exec.argv[1..]); + command.env_clear(); + command.envs(environment(&exec, Some(&runtime))); + // SAFETY: async-signal-safe calls only, nothing allocated. + unsafe { + command.pre_exec(move || { + if libc::setsid() == -1 { + return Err(io::Error::last_os_error()); + } + if libc::setgid(gid) != 0 { + return Err(io::Error::last_os_error()); + } + if libc::setuid(uid) != 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } + let socket = format!("{runtime}/{COMPOSITOR_SOCKET}"); + let before = crate::services::identity_of(&socket); + let mut watched = self + .waiters + .watch(|| Ok(command.spawn()?.id() as i32)) + .map_err(|error| { + Failure::new(format!("{} as {uid}:{gid}: {error}", exec.argv[0])) + })?; + let exit = watched.take_exit().expect("a new watch has its exit"); + let pid = watched.pid; + let flag = &watched; + if let Err(failure) = crate::services::await_path( + "the compositor", + &socket, + before, + &|| flag.running(), + COMPOSITOR_READY, + ) { + // One that never served is not left running behind a box that + // reports it could not be made. + if watched.running() { + // SAFETY: two integers; the pid is still this child's. + unsafe { libc::kill(-pid, libc::SIGKILL) }; + unsafe { libc::kill(pid, libc::SIGKILL) }; + } + return Err(failure); + } + tracing::info!(pid, %socket, "the compositor is serving"); + // Handed to the session, which reports it and ends the box: with it + // gone no launch has anywhere to draw. + self.compositor_exit = Some(Box::pin(async move { + exit.await.map_err(|_| io::Error::other("the compositor's exit was not delivered")) + })); + self.compositor = Some(watched); + Ok(()) + } + + fn compositor_exit(&mut self) -> Option { + self.compositor_exit.take() + } + fn stop_launch(&mut self, id: &LaunchId) { - if let Some(watched) = self.running.get(id) { - Self::signal_one(watched, libc::SIGTERM); + if let Some(group) = self.groups.get(id) { + // Whether or not the top process is still there: its children may + // be, and they are what holds the window. + stop_group(group.clone()); } } @@ -1059,6 +1517,255 @@ mod loop_tests { mod tests { use super::*; + #[test] + fn a_stat_line_names_its_session_whatever_the_command_is_called() { + assert_eq!(live_session_of("42 (launcher) S 1 42 42 0 -1"), Some(42)); + assert_eq!(live_session_of("43 (a) b (c) R 42 42 42 0"), Some(42)); + assert_eq!(live_session_of("44 (x) Z 42 42 42 0"), None); + assert_eq!(live_session_of("garbage"), None); + } + + /// Reaps one pid, as the guest's reaper does for every child. + fn reap_for(waiters: crate::reap::Waiters, pid: i32) { + tokio::spawn(async move { + loop { + let mut status = 0; + // SAFETY: writes only into `status`. + let seen = unsafe { libc::waitpid(pid, &mut status, libc::WNOHANG) }; + if seen == pid { + waiters.deliver(pid, crate::reap::exit_of(status)); + return; + } + if seen == -1 { + return; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + }); + } + + /// A process whose cgroups are made in a directory of the test's own, so + /// no test touches the real `/sys/fs/cgroup`. It is a plain directory with + /// no `cgroup.procs`, so a join into it fails, which is the fallback path. + fn process_in_tmp(waiters: crate::reap::Waiters) -> (Process, std::path::PathBuf) { + static NEXT: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let root = std::env::temp_dir().join(format!( + "nesinit-cg-{}-{}", + std::process::id(), + NEXT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + )); + std::fs::create_dir_all(&root).unwrap(); + let mut process = Process::new(waiters); + process.cgroup_root = root.to_string_lossy().into_owned(); + (process, root) + } + + fn me(script: &str) -> Exec { + // SAFETY: no arguments. + let (uid, gid) = unsafe { (libc::getuid(), libc::getgid()) }; + Exec { + argv: vec!["/bin/sh".into(), "-c".into(), script.into()], + env: Default::default(), + cwd: None, + uid, + gid, + } + } + + async fn started(script: &str) -> (Process, LaunchId, i32, Exited) { + let waiters = crate::reap::Waiters::new(); + let (mut process, _root) = process_in_tmp(waiters.clone()); + static NEXT: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + // Unique per test: members are found by the id in their environment. + let id = LaunchId::new(format!( + "g-{}-{}", + std::process::id(), + NEXT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + )); + let exited = process.start(&id, &me(script)).expect("it starts"); + let top = process.running.get(&id).unwrap().pid; + reap_for(waiters, top); + // Long enough for the shell to have started its children. + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + (process, id, top, exited) + } + + /// The bug this exists for: a stop that reached the top process only left + /// everything it had started running, and with it the game's window. + #[tokio::test] + async fn stopping_a_launch_ends_its_children_and_it_ends_when_they_have() { + let (mut process, id, _top, exited) = started("sleep 300 & sleep 300 & wait").await; + let group = process.groups[&id].clone(); + assert!(group.members().len() >= 3, "{:?}", group.members()); + process.stop_launch(&id); + tokio::time::timeout(std::time::Duration::from_secs(10), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert!(group.members().is_empty(), "something of the launch is left"); + } + + /// A child that ignores SIGTERM is killed after the grace, and the launch + /// is not reported ended before it has gone. + #[tokio::test] + async fn a_child_that_ignores_the_stop_is_killed_after_the_grace() { + let (mut process, id, _top, exited) = + started("trap '' TERM; (trap '' TERM; sleep 300) & wait").await; + let group = process.groups[&id].clone(); + process.stop_launch(&id); + let began = std::time::Instant::now(); + tokio::time::timeout(std::time::Duration::from_secs(15), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert!(began.elapsed() >= STOP_GRACE.saturating_sub(std::time::Duration::from_millis(500))); + assert!(group.members().is_empty()); + } + + /// The top process leaving by itself does not end the launch while + /// something of it runs; the leftovers are ended and then it has ended. + #[tokio::test] + async fn a_launch_whose_top_process_left_is_not_ended_until_its_group_is() { + let (process, id, _top, exited) = started("sleep 300 & exit 3").await; + let group = process.groups[&id].clone(); + let exit = tokio::time::timeout(std::time::Duration::from_secs(10), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert_eq!(exit, Exit::code(3)); + assert!(group.members().is_empty()); + } + + /// A cgroup the child could not join is dropped, not trusted: it would + /// read as an empty launch and a stop would do nothing. + #[tokio::test] + async fn a_launch_that_cannot_join_its_cgroup_falls_back_and_is_still_stopped() { + let waiters = crate::reap::Waiters::new(); + let (mut process, root) = process_in_tmp(waiters.clone()); + let id = LaunchId::new(format!("join-{}", std::process::id())); + let exited = process.start(&id, &me("sleep 300 & wait")).unwrap(); + reap_for(waiters, process.running[&id].pid); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + let group = process.groups[&id].clone(); + assert!(group.cgroup.is_none(), "the cgroup was kept"); + assert_eq!(std::fs::read_dir(&root).unwrap().count(), 0, "its directory was left"); + assert!(group.members().len() >= 2, "{:?}", group.members()); + process.stop_launch(&id); + tokio::time::timeout(std::time::Duration::from_secs(10), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert!(group.members().is_empty()); + } + + /// A stop that is still waiting out its grace must not kill a launch that + /// has taken the same id since. + #[tokio::test] + async fn a_pending_stop_does_not_kill_the_launch_that_replaced_it() { + let (mut process, id, _top, _exited) = started("sleep 300 & wait").await; + let old = process.groups[&id].clone(); + process.stop_launch(&id); + let waiters = process.waiters.clone(); + let _again = process.start(&id, &me("sleep 300 & wait")).unwrap(); + reap_for(waiters, process.running[&id].pid); + assert!(old.is_superseded()); + tokio::time::sleep(STOP_GRACE + std::time::Duration::from_secs(1)).await; + let new = process.groups[&id].clone(); + let alive = new.members(); + new.kill(); + assert!(alive.len() >= 2, "the new launch was killed by the old stop: {alive:?}"); + } + + /// Two launches are two groups: stopping one leaves the other running. + #[tokio::test] + async fn stopping_one_launch_leaves_another_running() { + let waiters = crate::reap::Waiters::new(); + let (mut process, _root) = process_in_tmp(waiters.clone()); + let (a, b) = (LaunchId::new("iso-a"), LaunchId::new("iso-b")); + let exited_a = process.start(&a, &me("sleep 300 & wait")).unwrap(); + let _exited_b = process.start(&b, &me("sleep 300 & wait")).unwrap(); + let (top_a, top_b) = ( + process.running[&a].pid, + process.running[&b].pid, + ); + reap_for(waiters.clone(), top_a); + reap_for(waiters, top_b); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + process.stop_launch(&a); + tokio::time::timeout(std::time::Duration::from_secs(10), exited_a) + .await + .unwrap() + .unwrap(); + assert!(process.groups[&a].members().is_empty()); + assert!(process.groups[&b].members().len() >= 2, "the other launch was touched"); + process.groups[&b].kill(); + } + + /// The case that broke #433: Wine `setsid`s its own processes, so they are + /// in no session of the launch's. The launch still owns them, by its + /// environment (no cgroup here: the test is not root), and a stop ends + /// them. + #[tokio::test] + async fn a_child_that_makes_its_own_session_is_still_stopped() { + let (mut process, id, top, exited) = + started("setsid sleep 300 & setsid sh -c 'sleep 300 & wait' & wait").await; + let group = process.groups[&id].clone(); + assert!( + session_members(top).len() < group.members().len(), + "the setsid children should be outside the launch's session: {:?}", + group.members() + ); + process.stop_launch(&id); + tokio::time::timeout(std::time::Duration::from_secs(10), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert!(group.members().is_empty(), "something of the launch is left"); + } + + /// The same through a real cgroup. Needs a cgroup v2 directory the test + /// may create in, named by `NESTRI_TEST_CGROUP_BASE` (a systemd user + /// delegation, or `/sys/fs/cgroup` as root). + #[tokio::test] + #[ignore = "needs a writable cgroup v2 directory in NESTRI_TEST_CGROUP_BASE"] + async fn a_cgroup_holds_a_child_that_makes_its_own_session() { + let base = std::env::var("NESTRI_TEST_CGROUP_BASE").expect("NESTRI_TEST_CGROUP_BASE"); + let waiters = crate::reap::Waiters::new(); + let mut process = Process::new(waiters.clone()); + process.cgroup_root = base; + let id = LaunchId::new(format!("cg-{}", std::process::id())); + let exited = process + .start(&id, &me("setsid sleep 300 & setsid sh -c 'sleep 300 & wait' & wait")) + .unwrap(); + let top = process.running[&id].pid; + reap_for(waiters, top); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + let group = process.groups[&id].clone(); + let path = group.cgroup.clone().expect("the cgroup was made"); + assert!(group.members().len() >= 4, "{:?}", group.members()); + process.stop_launch(&id); + tokio::time::timeout(std::time::Duration::from_secs(10), exited) + .await + .expect("the launch did not end") + .unwrap(); + assert!(group.members().is_empty()); + assert!(!path.exists(), "the cgroup was not removed"); + } + + #[test] + fn the_launch_id_is_found_whole_in_an_environment() { + let env = b"A=1\0NESTRI_LAUNCH_ID=g-10\0B=2\0"; + assert!(environ_names(env, "g-10")); + assert!(!environ_names(env, "g-1")); + assert!(!environ_names(b"X_NESTRI_LAUNCH_ID=g-10\0", "g-10")); + } + + #[test] + fn an_id_becomes_a_safe_directory_name() { + assert_eq!(sanitise("a/../b c"), "a_.._b_c"); + } + + /// The driver override has to reach the workload, because nothing else /// carries it: the image's profile script never runs for an exec'd /// process. Without it a game's OpenGL finds no driver at all. @@ -1370,6 +2077,11 @@ pub mod double { /// Every stop asked for, one launch or all (`None`). pub stopped: Vec>, pub stops: usize, + /// Every compositor asked for. + pub compositors: Vec, + pub compositor_failure: Option, + /// When the compositor "exits": the session takes this after it starts. + pub compositor_exit: Option>, pub mount_failure: Option, pub image_failure: Option, /// Like `image_failure`, but only for a call that names an image, so a @@ -1407,6 +2119,9 @@ pub mod double { started_ids: Vec::new(), stopped: Vec::new(), stops: 0, + compositors: Vec::new(), + compositor_failure: None, + compositor_exit: None, mount_failure: None, image_failure: None, named_image_failure: None, @@ -1496,6 +2211,22 @@ pub mod double { })) } + fn start_compositor(&mut self, compositor: &Compositor) -> Result<(), Failure> { + self.order.push("compositor"); + self.compositors.push(compositor.clone()); + match &self.compositor_failure { + Some(failure) => Err(failure.clone()), + None => Ok(()), + } + } + + fn compositor_exit(&mut self) -> Option { + let rx = self.compositor_exit.take()?; + Some(Box::pin(async move { + rx.await.map_err(|_| io::Error::other("the compositor was dropped")) + })) + } + fn stop_launch(&mut self, id: &LaunchId) { self.stops += 1; self.stopped.push(Some(id.clone())); diff --git a/crates/nesprotocol/src/lifecycle.rs b/crates/nesprotocol/src/lifecycle.rs index e88a2aa2..a98edae6 100644 --- a/crates/nesprotocol/src/lifecycle.rs +++ b/crates/nesprotocol/src/lifecycle.rs @@ -284,6 +284,35 @@ pub struct BootDescriptor { /// A number travels to where the thing that reads it is started. #[serde(default)] pub video: VideoLimits, + /// The box's one compositor, started by the init when the box is ready to + /// be launched into. ref(d-0085) + /// + /// Every launch draws on it, the first as much as the tenth: a launch is a + /// process group that can be stopped alone, and a compositor owned by one + /// launch could not be, because stopping that launch would take every other + /// window with it. `None` is a box with no compositor of its own, which is + /// what a workload that draws nothing wants. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub compositor: Option, +} + +/// What the compositor is started with. +/// +/// Geometry and rate are the box's, set once: every window in the box is a +/// stream of its own, but they share one output. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Compositor { + pub width: u32, + pub height: u32, + pub fps: u32, + /// Who it runs as: the same user the box's launches run as, because the + /// socket it publishes is in that user's runtime directory. + pub uid: u32, + pub gid: u32, + /// Environment on top of the guest's own, sorted like [`Exec::env`]. + #[serde(default)] + pub env: BTreeMap, } /// Ceilings on what a box's video may cost. @@ -577,6 +606,7 @@ mod tests { overlays: Vec::new(), images: Vec::new(), video: VideoLimits::default(), + compositor: None, } } @@ -882,6 +912,7 @@ mod video_limits_tests { overlays: Vec::new(), images: Vec::new(), video: VideoLimits::default(), + compositor: None, }; let json = serde_json::to_string(&d).unwrap(); assert!(!json.contains("bitrate"), "{json}"); @@ -896,6 +927,7 @@ mod video_limits_tests { video: VideoLimits { bitrate_kbps: Some(8_000), }, + compositor: None, }; let back: BootDescriptor = serde_json::from_str(&serde_json::to_string(&d).unwrap()).unwrap(); @@ -946,6 +978,7 @@ mod image_tests { at: "/nestri/compat".into(), }], video: VideoLimits::default(), + compositor: None, }; let json = serde_json::to_string(&d).unwrap(); assert!(json.contains(r#""images":[{"device":"/dev/vdd","at":"/nestri/compat"}]"#)); @@ -1008,4 +1041,28 @@ mod attach_tests { fn the_data_port_is_its_own() { assert_ne!(ATTACH_PORT, CONTROL_PORT); } + + /// A box with no compositor says nothing about one, so a descriptor from + /// before the field existed reads the same, and the compositor survives a + /// round trip with its environment. + #[test] + fn a_compositor_is_optional_and_round_trips() { + let none: BootDescriptor = serde_json::from_str("{}").unwrap(); + assert!(none.compositor.is_none()); + assert!(!serde_json::to_string(&none).unwrap().contains("compositor")); + let d = BootDescriptor { + compositor: Some(Compositor { + width: 1920, + height: 1080, + fps: 60, + uid: 1001, + gid: 1001, + env: BTreeMap::from([("A".to_string(), "b".to_string())]), + }), + ..none + }; + let back: BootDescriptor = + serde_json::from_str(&serde_json::to_string(&d).unwrap()).unwrap(); + assert_eq!(back, d); + } }