diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index fe117c46..9fde2fbf 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 6d3de6a4..af50b01b 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 b4e0fe61..7713214e 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;