From 2abf5e8bac8554ad91992ec0159403a4bb550780 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 24 Oct 2026 21:30:14 -0700 Subject: [PATCH] fix(daemon): reclaim serial sessions whose clients have all exited (#1429) A session is published by open_port before its client attaches a reader. If that client dies in between (or the WS attach times out), the session is left with an owner but no readers/writer, no idle close is scheduled, and the port returns EBUSY to every other opener until something else closes it. fbuild daemon locks already showed the owner as (dead). Every 2s the daemon now closes sessions in which every known client has a recorded pid and none of those processes is alive. Sessions with a live or unknown-pid client are kept, so the deploy -> attach hand-off is untouched. --- crates/fbuild-daemon/src/context.rs | 4 + crates/fbuild-daemon/src/handlers/locks.rs | 106 +++++++++++++++++++++ crates/fbuild-daemon/src/main.rs | 12 ++- 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index fe117c46e..9fde2fbfc 100644 --- a/crates/fbuild-daemon/src/context.rs +++ b/crates/fbuild-daemon/src/context.rs @@ -111,6 +111,10 @@ pub const IDLE_TIMEOUT: Duration = Duration::from_secs(43200); /// Interval for checking stale project locks. pub const STALE_LOCK_CHECK_INTERVAL: Duration = Duration::from_secs(60); +/// How often the daemon closes serial sessions whose clients have all exited +/// (FastLED/fbuild#1429). +pub const DEAD_CLIENT_SERIAL_RECLAIM_INTERVAL: Duration = Duration::from_secs(2); + use std::time::Duration; /// Path of the running binary, or an empty string if it cannot be resolved. diff --git a/crates/fbuild-daemon/src/handlers/locks.rs b/crates/fbuild-daemon/src/handlers/locks.rs index 6d3de6a4b..af50b01b4 100644 --- a/crates/fbuild-daemon/src/handlers/locks.rs +++ b/crates/fbuild-daemon/src/handlers/locks.rs @@ -105,6 +105,59 @@ fn session_stale_reason(session: &PortSessionInfo) -> Option { None } +/// Every client the session knows about has a recorded pid, and none of +/// those processes is still running. Unlike [`session_stale_reason`], a +/// session with no readers or writer but a live (or unknown) owner is kept: +/// that is the normal deploy → attach hand-off, not an abandoned port. +fn all_clients_dead(session: &PortSessionInfo) -> bool { + let mut ids: Vec<&str> = session + .reader_client_ids + .iter() + .map(String::as_str) + .chain(session.owner_client_id.as_deref()) + .chain(session.writer_client_id.as_deref()) + .collect(); + ids.extend(session.clients.iter().map(|c| c.client_id.as_str())); + !ids.is_empty() + && ids.iter().all(|id| { + session + .clients + .iter() + .find(|c| c.client_id == *id) + .and_then(|c| c.metadata.pid) + .is_some_and(|pid| !is_pid_alive(pid)) + }) +} + +/// Close serial sessions whose every client process has exited, so a port is +/// not held open (EBUSY to every other opener) for a client that is gone +/// (FastLED/fbuild#1429). Returns the ports closed. +pub async fn reclaim_dead_client_serial_sessions(ctx: &DaemonContext) -> Vec { + let mut closed = Vec::new(); + for session in ctx.serial_manager.get_port_sessions() { + if !all_clients_dead(&session) { + continue; + } + match ctx + .serial_manager + .close_port(&session.port, "dead_client_reclaim") + .await + { + Ok(()) => { + tracing::info!( + port = %session.port, + "closed serial session: every client process has exited" + ); + closed.push(session.port); + } + Err(err) => { + tracing::warn!(port = %session.port, "failed to reclaim dead-client serial session: {err}") + } + } + } + closed +} + fn live_refusal_reason(session: &PortSessionInfo) -> String { if let Some(owner_id) = session.owner_client_id.as_deref() { if let Some(owner) = session.clients.iter().find(|c| c.client_id == owner_id) { @@ -502,6 +555,59 @@ mod tests { assert!(ctx.serial_manager.get_port_sessions().is_empty()); } + #[tokio::test] + async fn reclaim_closes_session_whose_clients_are_all_dead() { + let ctx = test_context(); + insert_serial_session(&ctx, "COM_TEST_DEAD", "client-dead", Some(u32::MAX)); + + let closed = reclaim_dead_client_serial_sessions(&ctx).await; + + assert_eq!(closed, vec!["COM_TEST_DEAD".to_string()]); + assert!(ctx.serial_manager.get_port_sessions().is_empty()); + } + + #[tokio::test] + async fn reclaim_keeps_live_and_unknown_pid_sessions() { + let ctx = test_context(); + insert_serial_session( + &ctx, + "COM_TEST_LIVE", + "client-live", + Some(std::process::id()), + ); + insert_serial_session(&ctx, "COM_TEST_UNKNOWN", "client-unknown", None); + + assert!(reclaim_dead_client_serial_sessions(&ctx).await.is_empty()); + assert_eq!(ctx.serial_manager.get_port_sessions().len(), 2); + } + + #[tokio::test] + async fn reclaim_keeps_session_with_a_live_reader_beside_a_dead_owner() { + let ctx = test_context(); + let mut session = SerialSession::new("COM_TEST_MIXED".to_string(), 115200); + session.is_open = true; + session.owner_client_id = Some("client-dead".to_string()); + session.reader_client_ids.insert("client-live".to_string()); + for (client, pid) in [ + ("client-dead", u32::MAX), + ("client-live", std::process::id()), + ] { + session.client_metadata.insert( + client.to_string(), + SerialClientMetadata { + pid: Some(pid), + exe: None, + cwd: None, + argv: None, + }, + ); + } + ctx.serial_manager.insert_session_for_test(session); + + assert!(reclaim_dead_client_serial_sessions(&ctx).await.is_empty()); + assert_eq!(ctx.serial_manager.get_port_sessions().len(), 1); + } + #[tokio::test] async fn clear_locks_stale_closes_dead_owner_serial_session() { let ctx = test_context(); diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index b4e0fe611..7713214ed 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -11,7 +11,8 @@ use axum::routing::{get, post}; use clap::Parser; use fbuild_build::compile_backend::CompileBackend; use fbuild_daemon::context::{ - BroadcastHub, DaemonContext, IDLE_TIMEOUT, STALE_LOCK_CHECK_INTERVAL, self_eviction_timeout, + BroadcastHub, DEAD_CLIENT_SERIAL_RECLAIM_INTERVAL, DaemonContext, IDLE_TIMEOUT, + STALE_LOCK_CHECK_INTERVAL, self_eviction_timeout, }; use fbuild_daemon::handlers::{ boards, build_progress, cache, devices, emulator, health, libraries, locks, operations, @@ -456,6 +457,7 @@ async fn main() { tokio::spawn(async move { let mut daemon_empty_since: Option = None; let mut last_lock_cleanup = std::time::Instant::now(); + let mut last_serial_reclaim = std::time::Instant::now(); // Periodic reminder of why the daemon is staying alive, so // users can see the blocker rather than thinking self-eviction // is broken. See FastLED/fbuild#51. @@ -522,6 +524,14 @@ async fn main() { } // --- Periodic stale lock cleanup --- + // A port held for a client whose process has exited returns + // EBUSY to every other opener (FastLED/fbuild#1429). Checked + // often: the next run usually wants the port within seconds. + if last_serial_reclaim.elapsed() >= DEAD_CLIENT_SERIAL_RECLAIM_INTERVAL { + last_serial_reclaim = std::time::Instant::now(); + fbuild_daemon::handlers::locks::reclaim_dead_client_serial_sessions(&ctx).await; + } + if last_lock_cleanup.elapsed() >= STALE_LOCK_CHECK_INTERVAL { last_lock_cleanup = std::time::Instant::now(); let mut cleared = 0usize;