From c6f8f6667a52483c60c44c4b627347cbb73b013e Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 01:59:13 +0200 Subject: [PATCH 1/3] A session whose owner hung up is not handed again, and an open asks again for one that ended The compositor asks for a desktop session the moment its control connection ends. The supervisor answered the screen's row with its open desktop session unless that session's owner had exited, by a non-blocking wait. The kernel closes an ending process's handles before it publishes its exit, and publishes the exit only after every end below it, so an ask in between was handed the ended session: its control port answered `Gone`, the compositor said `no desktop session: ... Gone; the next launch asks again`, and opened none until something launched. Two changes, at the two ends of the answer: - The supervisor reads the owner's hang-up of its link, which that same close of handles makes, and keeps it (`Link::HungUp`): a session whose owner hung up is not handed again, and the loop stops watching a link that is readable for good. The decision takes both signs of an owner's end (`sessions::Open`), and its host test holds a hung-up owner whose exit is not published. - `toyos_session::open` asks again when the session it was handed has no controls left. However current the supervisor's reading, an owner can end after it answers; `Gone` says so, and the next ask finds the end read or a new session. Seen in guest-rows run 42089538985, where `session_lifecycle` stalled at "the desktop to open again". Closes issues/a-desktop-reopened-as-its-session-ends-is-handed-the-ended-session.md. Co-Authored-By: Claude Fable 5.2 --- Cargo.lock | 1 + ...ession-ends-is-handed-the-ended-session.md | 26 -------------- lib/session/Cargo.toml | 1 + lib/session/src/lib.rs | 29 +++++++++------ system/supervisor/lifecycle/src/sessions.rs | 34 +++++++++++------- system/supervisor/src/main.rs | 4 +-- system/supervisor/src/sessions.rs | 36 ++++++++++++------- 7 files changed, 67 insertions(+), 64 deletions(-) delete mode 100644 issues/a-desktop-reopened-as-its-session-ends-is-handed-the-ended-session.md 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..a1cf0da631e 100644 --- a/lib/session/src/lib.rs +++ b/lib/session/src/lib.rs @@ -273,22 +273,29 @@ impl Listed { /// 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.** Its owner ended +/// before the supervisor read that end, which it reads next; a failing +/// owner's opens are refused past their budget, so the asking ends. 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}")); + 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")?; + match toyos::namespace::Namespace::received(controls).open(CONTROL) { + Ok(control) => return Ok((toyos::namespace::Namespace::received(launcher), control)), + Err(toyos_abi::syscall::SyscallError::Gone) => {} + 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..7f98e1bceca 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 => { @@ -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 { "" })) From 9a234ec0625327010c17e59b653dae510f5424a9 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 02:36:27 +0200 Subject: [PATCH 2/3] supervisor: a start not told to an owner says the owner holds no link A link is now either not yet connected or hung up, and the line said only the first. Co-Authored-By: Claude Fable 5.2 --- system/supervisor/src/sessions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/supervisor/src/sessions.rs b/system/supervisor/src/sessions.rs index 7f98e1bceca..060e9099ba2 100644 --- a/system/supervisor/src/sessions.rs +++ b/system/supervisor/src/sessions.rs @@ -497,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()); From 9b3e6e28657e6ff04217c0f85d6791924634ca8f Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 03:50:06 +0200 Subject: [PATCH 3/3] Review r1 of #1206: the re-ask is bounded and says its expiry, and the guest lockfile names the dependency `toyos_session::open` asked again for as long as the supervisor handed over a session whose controls were gone: with neither sign of the owner's end read it would have asked for good, silently, holding its caller's thread. It now asks for at most `ENDED_UNREAD`, 5 s, with no delay between asks, and then returns an error naming the state and the number of asks. Its doc comment says what ends the asking. `tests/guest/rust-tests/Cargo.lock` lists `toyos-abi` under `toyos-session`, as the manifest has since the fix. Co-Authored-By: Claude Fable 5.2 --- lib/session/src/lib.rs | 24 ++++++++++++++++++++---- tests/guest/rust-tests/Cargo.lock | 1 + 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/session/src/lib.rs b/lib/session/src/lib.rs index a1cf0da631e..6bb4684914d 100644 --- a/lib/session/src/lib.rs +++ b/lib/session/src/lib.rs @@ -270,17 +270,26 @@ 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.** Its owner ended -/// before the supervisor read that end, which it reads next; a failing -/// owner's opens are refused past their budget, so the asking ends. +/// **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 mut rx = Box::new(toyos::ipc::FrameRx::<{ toyos::ipc::MAX_FRAME_LEN as usize }>::new()); + 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:?}"))?; @@ -290,9 +299,16 @@ pub fn open( 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) => {} + 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:?}")), } } 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]]