From 38984eef084c6bc16627ed30e6e0a2247583f89c Mon Sep 17 00:00:00 2001 From: Sebastian Gieseler Date: Sat, 31 Oct 2026 00:26:05 +0100 Subject: [PATCH] fix(hub): a start ends a hub that holds the lock and no longer answers (#398) A hub stuck for hours kept port 4250 and its state lock; every start (`up`, `ui`, `hub start`, the MCP server) failed on the lock until it was ended by hand. `ensure_running` now ends a hub that runs, said it listens on the port, started more than a minute ago and does not answer, then starts a new one; `hub_hung` goes to the log. The pid helper moves from the CLI to devstack-proc (`end_pid`) to serve both. The hang's cause stays open in #398. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 +++ crates/devstack-hub/src/lib.rs | 65 +++++++++++++++++++++++++++++++++ crates/devstack-proc/src/lib.rs | 41 +++++++++++++++++++++ crates/devstack/src/client.rs | 42 +-------------------- 4 files changed, 113 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82f51c30..45c18297 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,12 @@ and listed with their assets on the ### Fixed +- **A hub that stopped answering no longer blocks every start** (#398): it + held its lock, so `devstack up`, `devstack ui`, `devstack hub start` and + the MCP server each failed with "another devstack instance owns …\hub\lock" + until it was ended by hand. A hub that runs, holds the lock, started more + than a minute ago and does not answer on its port is now ended by the + next start, which takes its place (`hub_hung` in the hub's log). - **A node no longer leaves a gone lane's containers running** (#394): a slot removed without its purge (one without `slot.json`, or by an agent of an earlier release) left its Compose projects and networks running diff --git a/crates/devstack-hub/src/lib.rs b/crates/devstack-hub/src/lib.rs index 6f277567..2e14cfc8 100644 --- a/crates/devstack-hub/src/lib.rs +++ b/crates/devstack-hub/src/lib.rs @@ -3183,6 +3183,7 @@ pub fn ensure_running() -> Result { Listener::Foreign => return Err(foreign(port)), Listener::Nothing => {} } + end_hung_hub(port); let exe = devstack_exe().map_err(|e| e.to_string())?; // What the hub prints before its own log exists (a failed start) goes to // hub.out, like the daemon's daemon.out. @@ -3228,6 +3229,48 @@ pub fn ensure_running() -> Result { )) } +/// How long a hub may take to answer after it started before one that holds +/// the lock and does not answer counts as hung. +const HUNG_AFTER: Duration = Duration::from_secs(60); + +/// A hub that runs, holds the state lock and does not answer on `port` +/// (seen: its runtime stuck, every start after it refused by the lock) is +/// ended, so the start that follows can take its place. One that just +/// started, or a lock nobody holds, is left alone. +fn end_hung_hub(port: u16) { + let Some(info) = read_info() else { return }; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default(); + if !is_hung(&info, port, now, devstack_proc::pid_alive(info.pid)) { + return; + } + // Nobody holds the lock: that pid is no hub (any more). + if devstack_proc::StateLock::acquire(&state_dir()).is_ok() { + return; + } + diag::warn( + "hub_hung", + serde_json::json!({ "pid": info.pid, "port": port, "version": info.version }), + ); + if devstack_proc::end_pid(info.pid).is_err() { + return; + } + let t0 = Instant::now(); + while devstack_proc::pid_alive(info.pid) && t0.elapsed() < Duration::from_secs(10) { + std::thread::sleep(Duration::from_millis(100)); + } + let _ = std::fs::remove_file(state_dir().join("hub.json")); +} + +/// Whether the hub `info` names counts as hung: it runs, it said it listens +/// on `port`, and it started more than [`HUNG_AFTER`] before `now` (the +/// caller found nothing answering there). +fn is_hung(info: &HubInfo, port: u16, now: Duration, alive: bool) -> bool { + let started = Duration::from_millis(u64::try_from(info.started).unwrap_or(u64::MAX)); + alive && info.port == port && now.saturating_sub(started) >= HUNG_AFTER +} + /// Is a hub (or anything) answering on `port`? pub fn answers(port: u16) -> bool { std::net::TcpStream::connect_timeout( @@ -3848,3 +3891,25 @@ mod daemon_send_tests { assert_eq!(daemon_in(d.path()), DaemonNow::Absent { starting: false }); } } + +#[cfg(test)] +mod hung_hub_tests { + use super::*; + + #[test] + fn a_hub_is_hung_when_it_runs_listened_here_and_is_old_enough() { + let info = HubInfo { + pid: 42, + port: 4250, + version: "0.32.0".into(), + started: 1_000_000, + }; + let at = |s: u64| Duration::from_millis(1_000_000) + Duration::from_secs(s); + assert!(is_hung(&info, 4250, at(61), true)); + // Just started: it may still be coming up. + assert!(!is_hung(&info, 4250, at(10), true)); + // Gone, or a hub of another port. + assert!(!is_hung(&info, 4250, at(61), false)); + assert!(!is_hung(&info, 4251, at(61), true)); + } +} diff --git a/crates/devstack-proc/src/lib.rs b/crates/devstack-proc/src/lib.rs index e947e763..6bc957ad 100644 --- a/crates/devstack-proc/src/lib.rs +++ b/crates/devstack-proc/src/lib.rs @@ -92,6 +92,47 @@ pub fn pid_alive(pid: u32) -> bool { backend::pid_alive(pid) } +/// End the process `pid`, one devstack did not start (a daemon or a hub +/// that does not answer): Windows terminates it, which closes its job +/// objects and with them every process tree it owns (ADR-0009); elsewhere +/// `SIGTERM`, then `SIGKILL` after 5 s. +pub fn end_pid(pid: u32) -> std::io::Result<()> { + #[cfg(windows)] + { + use windows_sys::Win32::Foundation::CloseHandle; + use windows_sys::Win32::System::Threading::{ + OpenProcess, PROCESS_TERMINATE, TerminateProcess, + }; + // SAFETY: plain Win32 calls on a handle we open and close here. + unsafe { + let h = OpenProcess(PROCESS_TERMINATE, 0, pid); + if h.is_null() { + return Err(std::io::Error::last_os_error()); + } + let ok = TerminateProcess(h, 1) != 0; + let err = std::io::Error::last_os_error(); + CloseHandle(h); + if ok { Ok(()) } else { Err(err) } + } + } + #[cfg(not(windows))] + { + // SAFETY: a signal to one process id. + if unsafe { libc::kill(pid as i32, libc::SIGTERM) } != 0 { + return Err(std::io::Error::last_os_error()); + } + let t0 = std::time::Instant::now(); + while pid_alive(pid) && t0.elapsed() < std::time::Duration::from_secs(5) { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + if pid_alive(pid) { + // SAFETY: as above. + unsafe { libc::kill(pid as i32, libc::SIGKILL) }; + } + Ok(()) + } +} + /// What a process tree costs right now (plan §12). Callers derive CPU load /// from two samples of `cpu_us`. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize)] diff --git a/crates/devstack/src/client.rs b/crates/devstack/src/client.rs index bf830d0e..aa8cb141 100644 --- a/crates/devstack/src/client.rs +++ b/crates/devstack/src/client.rs @@ -446,46 +446,6 @@ pub(crate) fn pid_alive(pid: u32) -> bool { } } -/// End the process `pid` (a daemon that does not answer): Windows -/// terminates it, which closes its job objects and with them every process -/// tree it owns (ADR-0009); elsewhere `SIGTERM`, then `SIGKILL` after 5 s. -fn end_process(pid: u32) -> std::io::Result<()> { - #[cfg(windows)] - { - use windows_sys::Win32::Foundation::CloseHandle; - use windows_sys::Win32::System::Threading::{ - OpenProcess, PROCESS_TERMINATE, TerminateProcess, - }; - // SAFETY: plain Win32 calls on a handle we open and close here. - unsafe { - let h = OpenProcess(PROCESS_TERMINATE, 0, pid); - if h.is_null() { - return Err(std::io::Error::last_os_error()); - } - let ok = TerminateProcess(h, 1) != 0; - let err = std::io::Error::last_os_error(); - CloseHandle(h); - if ok { Ok(()) } else { Err(err) } - } - } - #[cfg(not(windows))] - { - // SAFETY: a signal to one process id. - if unsafe { libc::kill(pid as i32, libc::SIGTERM) } != 0 { - return Err(std::io::Error::last_os_error()); - } - let t0 = Instant::now(); - while pid_alive(pid) && t0.elapsed() < Duration::from_secs(5) { - std::thread::sleep(Duration::from_millis(100)); - } - if pid_alive(pid) { - // SAFETY: as above. - unsafe { libc::kill(pid as i32, libc::SIGKILL) }; - } - Ok(()) - } -} - /// Nothing listens (yet): the connection was refused, unlike a daemon that /// accepted and did not answer. fn is_refused(e: &anyhow::Error) -> bool { @@ -2498,7 +2458,7 @@ fn end_unanswering_daemon(state_dir: &Path, why: anyhow::Error, all: bool) -> Re } eprintln!("{why:#}"); println!("ending the daemon by its pid ({})", info.pid); - end_process(info.pid).with_context(|| format!("cannot end process {}", info.pid))?; + devstack_proc::end_pid(info.pid).with_context(|| format!("cannot end process {}", info.pid))?; let t0 = Instant::now(); while pid_alive(info.pid) && t0.elapsed() < Duration::from_secs(15) { std::thread::sleep(Duration::from_millis(200));