diff --git a/Cargo.lock b/Cargo.lock index ba849675b5d..36dd71f651d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4054,6 +4054,7 @@ name = "toyos-session" version = "0.1.0" dependencies = [ "toyos", + "toyos-abi", ] [[package]] diff --git a/issues/a-desktop-reopened-as-its-session-ends-is-handed-the-ended-session.md b/issues/a-desktop-reopened-as-its-session-ends-is-handed-the-ended-session.md deleted file mode 100644 index e56ecff6eb6..00000000000 --- a/issues/a-desktop-reopened-as-its-session-ends-is-handed-the-ended-session.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-11-05 ---- - -# A desktop reopened as its session ends is handed the ended session - -When the desktop's session ends, the compositor asks the supervisor for a new -one at once (`system/compositor/src/signout.rs`). The supervisor answers the -screen's row with its open desktop session, one whose owner it does not yet -see as ended (`system/supervisor/src/sessions.rs`, `serve_login` through -`desktop_session` and `exit_of`). An ask that lands between the owner's exit -and the supervisor reading it is handed the ended session, whose control port -then answers `Gone`; the compositor says `no desktop session: … Gone; the next -launch asks again`, and opens none until something launches. - -Seen once, in guest rows run 42089538985 (`session_lifecycle`, QEMU x86-64, -`tests/images/sessioncase.toml`), which stalled at "the desktop to open -again": the session owner exited at 4.967 s, the supervisor said `session 8 is -signed out` at 4.973 s, and the compositor's refusal is stamped 4.973 s. The -row passed at that branch in runs 42018194698, 42040884795 and 42080820355. - -**Exit**: an ask the supervisor answers is never an ended session's, or the -compositor asks again when the one it was handed is gone; a host test of the -supervisor's decision with the owner's exit not yet read holds it. diff --git a/lib/session/Cargo.toml b/lib/session/Cargo.toml index 5e3263d8bb5..72ae1833e2c 100644 --- a/lib/session/Cargo.toml +++ b/lib/session/Cargo.toml @@ -11,6 +11,7 @@ doctest = false [dependencies] # A login authority's open of a session, through its namespace. toyos = { path = "../../sdk/toyos" } +toyos-abi = { path = "../../sdk/abi" } [lints.rust] warnings = "deny" diff --git a/lib/session/src/lib.rs b/lib/session/src/lib.rs index 58433c80d46..6bb4684914d 100644 --- a/lib/session/src/lib.rs +++ b/lib/session/src/lib.rs @@ -270,25 +270,48 @@ impl Listed { } } +/// How long [`open`] goes on asking for a session whose controls are gone. +/// The supervisor reads an owner's hang-up in a round of its loop, so one it +/// still hands out after this long is one it does not read as ended. +const ENDED_UNREAD: std::time::Duration = std::time::Duration::from_secs(5); + /// A session opened on [`LOGIN_PORT`] through `login`, the namespace a /// login authority is endowed under that name: its launcher, as a namespace /// holding `toyos::launch::LAUNCHER`, and its control connection. +/// +/// **A session whose controls are gone is asked for again**, at once: its +/// owner ended, and the supervisor answers from what it has read of that +/// end, which an ask can outrun more than once. The asking ends with a +/// session whose controls connect, with the supervisor's refusal, which a +/// failing owner's budget comes to, or after [`ENDED_UNREAD`] as an error. pub fn open( login: &toyos::namespace::Namespace, ) -> Result<(toyos::namespace::Namespace, toyos::ipc::Connection), String> { - let conn = login.open(LOGIN_PORT).map_err(|e| format!("the supervisor's login port refused: {e:?}"))?; - conn.signal(MSG_OPEN).map_err(|e| format!("the open was not sent: {e:?}"))?; let mut rx = Box::new(toyos::ipc::FrameRx::<{ toyos::ipc::MAX_FRAME_LEN as usize }>::new()); - let frame = rx.recv(&conn).map_err(|e| format!("the open was not answered: {e:?}"))?; - if frame.msg_type != MSG_OPENED { - let why = String::from_utf8_lossy(rx.payload(frame.payload_len)); - return Err(format!("the supervisor refused a session: {why}")); + let began = std::time::Instant::now(); + let mut asks = 0u32; + loop { + let conn = login.open(LOGIN_PORT).map_err(|e| format!("the supervisor's login port refused: {e:?}"))?; + conn.signal(MSG_OPEN).map_err(|e| format!("the open was not sent: {e:?}"))?; + let frame = rx.recv(&conn).map_err(|e| format!("the open was not answered: {e:?}"))?; + if frame.msg_type != MSG_OPENED { + let why = String::from_utf8_lossy(rx.payload(frame.payload_len)); + return Err(format!("the supervisor refused a session: {why}")); + } + let [launcher, controls] = rx.take_handles::<2>().ok_or("the session came without its handles")?; + asks += 1; + match toyos::namespace::Namespace::received(controls).open(CONTROL) { + Ok(control) => return Ok((toyos::namespace::Namespace::received(launcher), control)), + Err(toyos_abi::syscall::SyscallError::Gone) if began.elapsed() < ENDED_UNREAD => {} + Err(toyos_abi::syscall::SyscallError::Gone) => { + return Err(format!( + "the supervisor handed over a session whose controls are gone {asks} times in {} s", + ENDED_UNREAD.as_secs() + )); + } + Err(e) => return Err(format!("the session's controls refused its control connection: {e:?}")), + } } - let [launcher, controls] = rx.take_handles::<2>().ok_or("the session came without its handles")?; - let control = toyos::namespace::Namespace::received(controls) - .open(CONTROL) - .map_err(|e| format!("the session's controls refused its control connection: {e:?}"))?; - Ok((toyos::namespace::Namespace::received(launcher), control)) } #[cfg(test)] diff --git a/system/supervisor/lifecycle/src/sessions.rs b/system/supervisor/lifecycle/src/sessions.rs index 180e591c4d3..544519f661f 100644 --- a/system/supervisor/lifecycle/src/sessions.rs +++ b/system/supervisor/lifecycle/src/sessions.rs @@ -58,20 +58,24 @@ pub fn holds_machine_launcher(program: &Program) -> bool { } /// One open session, as [`desktop_session`] judges it: whether it is a -/// desktop, the login authority that opened it, and whether its owner still -/// runs, which it may not while its end waits to be read. +/// desktop, the login authority that opened it, and the two signs that its +/// owner has ended. #[derive(Clone, Copy, Debug)] pub struct Open<'a> { pub desktop: bool, pub opener: &'a str, - pub running: bool, + /// The owner's link has hung up. The kernel closes an ending process's + /// handles before it publishes its exit, so this comes first. + pub hung_up: bool, + /// The owner's exit is published. + pub exited: bool, } /// The desktop session `opener` may be handed again, by its index: one it -/// opened whose owner still runs. **Never one whose owner has ended** though -/// its end is not read yet: its launcher's port is about to close. +/// opened whose owner shows no sign of its end. **Never one whose owner has +/// ended** though the session is still open: its controls are gone. pub fn desktop_session(open: &[Open<'_>], opener: &str) -> Option { - open.iter().position(|s| s.desktop && s.opener == opener && s.running) + open.iter().position(|s| s.desktop && s.opener == opener && !s.hung_up && !s.exited) } /// The session a launch an owner handed over runs in: its link's, never what @@ -153,15 +157,19 @@ mod tests { assert!(holds_machine_launcher(&Program { starts: vec!["toybox".into()], ..Program::default() })); } - /// A desktop whose owner ended, its end not read yet, is not handed - /// again: the next running one is, or none and one is opened. + /// A desktop whose owner ended is not handed again, whether its link + /// hung up with its exit not published yet or its exit is published and + /// the session not closed yet: the next running one is, or none and one + /// is opened. #[test] fn a_desktop_whose_owner_ended_is_not_handed_again() { - let ended = Open { desktop: true, opener: "compositor", running: false }; - let running = Open { desktop: true, opener: "compositor", running: true }; - let remote = Open { desktop: false, opener: "sshserver", running: true }; - assert_eq!(desktop_session(&[ended], "compositor"), None); - assert_eq!(desktop_session(&[remote, ended, running], "compositor"), Some(2)); + let running = Open { desktop: true, opener: "compositor", hung_up: false, exited: false }; + let hung_up = Open { hung_up: true, ..running }; + let exited = Open { exited: true, ..running }; + let remote = Open { desktop: false, opener: "sshserver", ..running }; + assert_eq!(desktop_session(&[hung_up], "compositor"), None); + assert_eq!(desktop_session(&[exited], "compositor"), None); + assert_eq!(desktop_session(&[remote, hung_up, exited, running], "compositor"), Some(3)); assert_eq!(desktop_session(&[running], "sshserver"), None); } diff --git a/system/supervisor/src/main.rs b/system/supervisor/src/main.rs index dad951ef955..f93bb322d61 100644 --- a/system/supervisor/src/main.rs +++ b/system/supervisor/src/main.rs @@ -1672,8 +1672,8 @@ impl<'a> Supervisor<'a> { poller.watch(&self.minted.login, READABLE, TOKEN_LOGIN_ACCEPTOR); poller.watch(&self.minted.admin, READABLE, TOKEN_ADMIN_ACCEPTOR); poller.watch(&self.watch.ended, READABLE, TOKEN_APPS); - for login in &self.logins.open { - poller.watch(&login.link.handle(), READABLE, TOKEN_LINKS); + for link in self.logins.open.iter().filter_map(|login| login.link.handle()) { + poller.watch(&link, READABLE, TOKEN_LINKS); } poller.watch(swap, READABLE, TOKEN_SWAP_ACCEPTOR); poller.watch(power, READABLE, TOKEN_POWER_ACCEPTOR); diff --git a/system/supervisor/src/sessions.rs b/system/supervisor/src/sessions.rs index 2c93014325d..060e9099ba2 100644 --- a/system/supervisor/src/sessions.rs +++ b/system/supervisor/src/sessions.rs @@ -76,20 +76,23 @@ pub(crate) struct Desktop { pub control: Namespace, } -/// A session owner's link: the port the owner connects, until it has, and -/// then the connection. The owner connects, so the link is counted to its -/// job. +/// A session owner's link: the port the owner connects, until it has, then +/// the connection, until the owner hangs it up. The owner connects, so the +/// link is counted to its job. pub(crate) enum Link { Waiting(Acceptor), Open(Connection), + /// The owner hung up, which only its end does. + HungUp, } impl Link { /// What the loop watches for the owner's word, or its connect. - pub fn handle(&self) -> BorrowedHandle<'_> { + pub fn handle(&self) -> Option> { match self { - Self::Waiting(acceptor) => acceptor.as_handle(), - Self::Open(conn) => conn.as_handle(), + Self::Waiting(acceptor) => Some(acceptor.as_handle()), + Self::Open(conn) => Some(conn.as_handle()), + Self::HungUp => None, } } @@ -108,11 +111,11 @@ impl Link { self.conn() } - /// The connection, once the owner has connected. + /// The connection, while the owner holds its end. fn conn(&self) -> Option<&Connection> { match self { Self::Open(conn) => Some(conn), - Self::Waiting(_) => None, + Self::Waiting(_) | Self::HungUp => None, } } } @@ -168,7 +171,7 @@ impl Logins { pub fn tell_all(&self, msg_type: u32) { for login in &self.open { let Some(link) = login.link.conn() else { - say!("supervisor: session {}'s owner has not connected its link, and is not told {msg_type}", login.session.share()); + say!("supervisor: session {}'s owner holds no link, and is not told {msg_type}", login.session.share()); continue; }; if let Err(e) = link.try_signal(msg_type) { @@ -354,7 +357,12 @@ impl Supervisor<'_> { .logins .open .iter() - .map(|l| decide::Open { desktop: l.desktop, opener: &l.opener.name, running: exit_of(&l.owner).is_none() }) + .map(|l| decide::Open { + desktop: l.desktop, + opener: &l.opener.name, + hung_up: matches!(l.link, Link::HungUp), + exited: exit_of(&l.owner).is_some(), + }) .collect(); decide::desktop_session(&open, &opener.name) } @@ -419,7 +427,11 @@ impl Supervisor<'_> { let login = &mut self.logins.open[at]; let Some(link) = login.link.open() else { break }; let (msg_type, payload, batch) = match login.rx.pump(link) { - RxStep::Idle | RxStep::Eof => break, + RxStep::Idle => break, + RxStep::Eof => { + login.link = Link::HungUp; + break; + } // The supervisor's own child broke its protocol: the session // ends with it, as a failure. RxStep::Malformed => { @@ -485,7 +497,7 @@ impl Supervisor<'_> { let started = Started { instance, name: name.to_string() }; let sent = match login.link.conn() { Some(link) => link.try_send_bytes_with(wire::MSG_STARTED, &started.encode(), [managed]).map_err(|e| format!("{e:?}")), - None => Err("its link is not connected".to_string()), + None => Err("it holds no link".to_string()), }; if let Err(e) = sent { say!("supervisor: session {}'s owner was not told of {name} ({e:?})", session.share()); @@ -579,7 +591,7 @@ impl Supervisor<'_> { login .link .conn() - .ok_or_else(|| format!("session {}'s owner has not connected its link", asked.share))? + .ok_or_else(|| format!("session {}'s owner holds no link", asked.share))? .try_send_bytes(wire::MSG_SIGN_OUT, &asked.sign_out.encode()) .map_err(|e| format!("session {}'s owner was not asked ({e:?})", asked.share))?; Ok(format!("session {} is asked to sign out{}", asked.share, if force { ", by force" } else { "" })) diff --git a/tests/guest/rust-tests/Cargo.lock b/tests/guest/rust-tests/Cargo.lock index abe32fa2605..cc7c0f9412a 100644 --- a/tests/guest/rust-tests/Cargo.lock +++ b/tests/guest/rust-tests/Cargo.lock @@ -1048,6 +1048,7 @@ name = "toyos-session" version = "0.1.0" dependencies = [ "toyos", + "toyos-abi", ] [[package]]