From ed4ddc13dcc23612d7f6f947e5a87bd56a745399 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 10:49:39 -0700 Subject: [PATCH 1/8] =?UTF-8?q?fix(daemon):=20controlled=20exit=20on=20SIG?= =?UTF-8?q?TERM=20=E2=80=94=20gate=20new=20work,=20drain=205=20s,=20flush?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, SIGTERM on Linux/macOS (`fbuild daemon kill`, the escalation in `fbuild daemon stop`, docker stop, CI teardown) killed the daemon with no zccache flush and stale pid/port files. Routing it through the graceful HTTP drain would instead wait out every running build. - New fbuild_daemon::shutdown: - refuse_new_operations_when_shutting_down: operation routes (build, deploy, monitor, install-deps, reset, test-emu) answer 503 with a failed OperationResponse once shutdown has started (SIGTERM or HTTP shutdown). - exit_on_terminate: on SIGTERM, refuse new work, wait up to 5 s for in-flight operations, flush zccache (4 s cap), clean up records, exit. - persist_and_clean_up: the pid/port/claim/status cleanup + bounded flush now shared by every clean exit. - DaemonContext::active_operations: exact in-flight count kept by OperationGuard (the operation_in_progress bool is cleared by the first of two concurrent operations to finish), plus wait_for_operations(budget). - fbuild_core::daemon_health: SHUTDOWN_DRAIN_BUDGET (5 s), EXIT_FLUSH_BUDGET (4 s), TERMINATE_EXIT_BUDGET (their sum). - `fbuild daemon stop`: after a delivered graceful terminate, wait TERMINATE_EXIT_BUDGET + 1 s before the forced kill so it never lands mid-flush; a refused terminate keeps the 5 s wait. Follow-up to #1480 / #1482. --- crates/fbuild-cli/src/cli/daemon_stop.rs | 23 +- crates/fbuild-core/src/daemon_health.rs | 16 ++ crates/fbuild-daemon/src/README.md | 1 + crates/fbuild-daemon/src/context.rs | 25 +++ .../src/handlers/operations/common.rs | 19 ++ crates/fbuild-daemon/src/lib.rs | 1 + crates/fbuild-daemon/src/main.rs | 62 +++--- crates/fbuild-daemon/src/shutdown.rs | 207 ++++++++++++++++++ 8 files changed, 316 insertions(+), 38 deletions(-) create mode 100644 crates/fbuild-daemon/src/shutdown.rs diff --git a/crates/fbuild-cli/src/cli/daemon_stop.rs b/crates/fbuild-cli/src/cli/daemon_stop.rs index 0d2c0d5dd..a75cfbce6 100644 --- a/crates/fbuild-cli/src/cli/daemon_stop.rs +++ b/crates/fbuild-cli/src/cli/daemon_stop.rs @@ -75,11 +75,18 @@ const DAEMON_PROCESS_STEM: &str = "fbuild-daemon"; /// shutting down, it is stuck. const GRACEFUL_STOP_BUDGET: std::time::Duration = std::time::Duration::from_secs(5); -/// How long a signalled process gets to actually disappear. Termination is +/// How long a force-killed process gets to actually disappear. Termination is /// asynchronous on both OS families, but a process that has not gone in 5 s /// is not going. const TERMINATION_BUDGET: std::time::Duration = std::time::Duration::from_secs(5); +/// How long a daemon sent a graceful terminate (SIGTERM on Unix) gets before +/// the forced kill. The daemon answers SIGTERM with a bounded controlled exit +/// (drain in-flight operations, then flush zccache); escalating before that +/// budget would kill it mid-flush. +const GRACEFUL_TERMINATION_BUDGET: std::time::Duration = + std::time::Duration::from_secs(fbuild_core::daemon_health::TERMINATE_EXIT_BUDGET.as_secs() + 1); + /// `fbuild daemon stop` — stop the daemon for *this* endpoint and report what /// actually happened. /// @@ -186,10 +193,16 @@ pub async fn run_daemon_stop(client: &DaemonClient) -> fbuild_core::Result<()> { /// anomaly — which is why the graceful attempt's exit status is ignored and /// only the liveness check decides. async fn terminate_and_confirm(pid: u32) -> fbuild_core::Result<()> { - if let Err(error) = kill_process(pid, false).await { - tracing::debug!(pid, %error, "graceful terminate refused; escalating to a forced kill"); - } - if wait_for_process_exit(pid, TERMINATION_BUDGET).await { + // A delivered terminate gets the daemon's full controlled-exit budget; a + // refused one keeps the plain liveness wait. + let graceful_budget = match kill_process(pid, false).await { + Ok(()) => GRACEFUL_TERMINATION_BUDGET, + Err(error) => { + tracing::debug!(pid, %error, "graceful terminate refused; escalating to a forced kill"); + TERMINATION_BUDGET + } + }; + if wait_for_process_exit(pid, graceful_budget).await { return Ok(()); } diff --git a/crates/fbuild-core/src/daemon_health.rs b/crates/fbuild-core/src/daemon_health.rs index dc5ff4ce6..98ba74b11 100644 --- a/crates/fbuild-core/src/daemon_health.rs +++ b/crates/fbuild-core/src/daemon_health.rs @@ -30,6 +30,22 @@ pub const STARTING_BUDGET: Duration = Duration::from_secs(120); /// Interval between `/health` polls. pub const POLL_INTERVAL: Duration = Duration::from_millis(100); +/// Longest a daemon told to terminate (SIGTERM) waits for in-flight +/// operations before it exits anyway. New operations are refused from the +/// moment the signal arrives. +pub const SHUTDOWN_DRAIN_BUDGET: Duration = Duration::from_secs(5); + +/// Cap on the daemon's final zccache flush. A normal flush takes well under +/// 100 ms; the cap only matters when zccache is stuck behind a slow disk or a +/// startup load (zackees/zccache#1652). +pub const EXIT_FLUSH_BUDGET: Duration = Duration::from_secs(4); + +/// Longest a terminated daemon can take to exit: drain, then flush. Clients +/// that send SIGTERM must wait at least this long before escalating to a +/// forced kill, or the kill lands mid-flush. +pub const TERMINATE_EXIT_BUDGET: Duration = + Duration::from_secs(SHUTDOWN_DRAIN_BUDGET.as_secs() + EXIT_FLUSH_BUDGET.as_secs()); + /// What one `/health` probe says about the daemon. #[derive(Debug, Clone, PartialEq, Eq)] pub enum DaemonHealth { diff --git a/crates/fbuild-daemon/src/README.md b/crates/fbuild-daemon/src/README.md index 0e6cdfb88..323ba09ec 100644 --- a/crates/fbuild-daemon/src/README.md +++ b/crates/fbuild-daemon/src/README.md @@ -7,6 +7,7 @@ - **`context.rs`** -- `DaemonContext` (shared state), `BroadcastHub`, self-eviction/idle timeout constants - **`device_manager.rs`** -- `DeviceManager` with exclusive/monitor leases, preemption, and stale device cleanup - **`models.rs`** -- Request/response serde types for all API endpoints (build, deploy, monitor, devices, locks, reset) +- **`shutdown.rs`** -- Exit paths: `refuse_new_operations_when_shutting_down` middleware (503 once shutdown starts), SIGTERM controlled exit (`exit_on_terminate`: drain in-flight operations up to 5 s, then flush), and `persist_and_clean_up` (pid/port/status cleanup + bounded zccache flush) shared by every clean exit - **`startup.rs`** -- `StartupGate`: answers every request with `503 {"status":"starting","phase":...}` on a duplicate of the bound listener while `main` initializes, then hands the endpoint to the full router (FastLED/fbuild#1480) - **`status_manager.rs`** -- `StatusManager` for atomic read-modify-write of `daemon_status.json` - **`handlers/`** -- HTTP and WebSocket route handler modules diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index 4bde29011..333d811f4 100644 --- a/crates/fbuild-daemon/src/context.rs +++ b/crates/fbuild-daemon/src/context.rs @@ -158,6 +158,11 @@ pub struct DaemonContext { pub is_shutting_down: Arc, /// Whether a build/deploy operation is currently in progress. pub operation_in_progress: Arc, + /// Number of build/deploy/... operations in flight (`OperationGuard`s + /// alive). Unlike `operation_in_progress`, which the first of two + /// concurrent operations clears when it ends, this stays exact, so a + /// shutdown can wait for the last one (FastLED/fbuild#1480 follow-up). + pub active_operations: Arc, /// Number of WebSocket connections currently inside the serial-monitor /// handler (e.g. waiting for a port to open). Counted independently of /// `serial_manager` sessions because a port may take seconds to open @@ -266,6 +271,7 @@ impl DaemonContext { serial_manager: Arc::new(SharedSerialManager::new()), is_shutting_down: Arc::new(AtomicBool::new(false)), operation_in_progress: Arc::new(AtomicBool::new(false)), + active_operations: Arc::new(AtomicUsize::new(0)), pending_serial_attaches: Arc::new(AtomicUsize::new(0)), pending_serial_attach_details: DashMap::new(), pending_serial_attach_next_id: AtomicU64::new(1), @@ -405,6 +411,25 @@ impl DaemonContext { self.started_at.elapsed() } + /// Wait up to `budget` for every in-flight operation to finish. Returns + /// whether none are left. + pub async fn wait_for_operations(&self, budget: Duration) -> bool { + let deadline = Instant::now() + budget; + loop { + if self + .active_operations + .load(std::sync::atomic::Ordering::Acquire) + == 0 + { + return true; + } + if Instant::now() >= deadline { + return false; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + } + /// How long since the last activity (request processed). pub fn idle_duration(&self) -> Duration { self.last_activity diff --git a/crates/fbuild-daemon/src/handlers/operations/common.rs b/crates/fbuild-daemon/src/handlers/operations/common.rs index 01365b582..9f66172a5 100644 --- a/crates/fbuild-daemon/src/handlers/operations/common.rs +++ b/crates/fbuild-daemon/src/handlers/operations/common.rs @@ -172,6 +172,7 @@ impl OperationGuard { description: Option, ) -> Self { ctx.touch_activity(); + ctx.active_operations.fetch_add(1, Ordering::AcqRel); ctx.operation_in_progress.store(true, Ordering::Relaxed); if let Ok(mut s) = ctx.daemon_state.write() { *s = daemon_state; @@ -190,6 +191,7 @@ impl OperationGuard { impl Drop for OperationGuard { fn drop(&mut self) { + self.ctx.active_operations.fetch_sub(1, Ordering::AcqRel); self.flag.store(false, Ordering::Relaxed); if let Ok(mut s) = self.state.write() { *s = fbuild_core::DaemonState::Idle; @@ -205,6 +207,23 @@ impl Drop for OperationGuard { mod tests { use super::*; + #[test] + fn operation_guards_count_concurrent_operations_exactly() { + let (shutdown_tx, _shutdown_rx) = tokio::sync::watch::channel(false); + let ctx = Arc::new(DaemonContext::new(0, shutdown_tx, ".".to_string())); + let first = OperationGuard::new(&ctx, fbuild_core::DaemonState::Building, None); + let second = OperationGuard::new(&ctx, fbuild_core::DaemonState::Building, None); + assert_eq!(ctx.active_operations.load(Ordering::Acquire), 2); + + drop(first); + // The shutdown drain waits on this count, not on the bool the first + // finisher clears while the second is still running. + assert_eq!(ctx.active_operations.load(Ordering::Acquire), 1); + + drop(second); + assert_eq!(ctx.active_operations.load(Ordering::Acquire), 0); + } + #[test] fn operation_guard_drop_clears_dependency_install_through_context() { let (shutdown_tx, _shutdown_rx) = tokio::sync::watch::channel(false); diff --git a/crates/fbuild-daemon/src/lib.rs b/crates/fbuild-daemon/src/lib.rs index e2e091484..db5e50280 100644 --- a/crates/fbuild-daemon/src/lib.rs +++ b/crates/fbuild-daemon/src/lib.rs @@ -38,6 +38,7 @@ pub mod heap_profile; pub mod lock_models; pub mod log_layer; pub mod models; +pub mod shutdown; pub mod startup; pub mod status_manager; pub mod watch_set_cache; diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index a49c3cdce..89c346845 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -241,7 +241,22 @@ async fn main() { DaemonContext::install_dependency_status_subscriber(&context); fbuild_daemon::broker::backend::spawn_backend_endpoint_if_requested(context.clone()); + // Operation routes refuse new work once shutdown has started, so an + // exiting daemon never begins a build it would abandon. + let operation_routes = Router::new() + .route("/api/build", post(operations::build)) + .route("/api/deploy", post(operations::deploy)) + .route("/api/monitor", post(operations::monitor)) + .route("/api/install-deps", post(operations::install_deps)) + .route("/api/reset", post(operations::reset)) + .route("/api/test-emu", post(emulator::test_emu)) + .route_layer(axum::middleware::from_fn_with_state( + context.clone(), + fbuild_daemon::shutdown::refuse_new_operations_when_shutting_down, + )); + let app = Router::new() + .merge(operation_routes) .route("/", get(health::root)) .route("/health", get(health::health_check)) .route("/api/daemon/image-hash", get(health::image_hash)) @@ -251,9 +266,6 @@ async fn main() { // misbehaving. Restarting to enable a profiler would destroy the // leak being investigated, which is what made #1360 hard to chase. .route("/api/daemon/heap-dump", post(health::heap_dump)) - .route("/api/build", post(operations::build)) - .route("/api/deploy", post(operations::deploy)) - .route("/api/monitor", post(operations::monitor)) .route("/api/devices/list", post(devices::list_devices)) .route("/api/devices/:port/status", get(devices::device_status)) .route("/api/devices/:port/lease", post(devices::device_lease)) @@ -263,9 +275,6 @@ async fn main() { .route("/api/locks/clear", post(locks::clear_locks)) .route("/api/cache/stats", get(cache::cache_stats)) .route("/api/cache/gc", post(cache::run_gc)) - .route("/api/install-deps", post(operations::install_deps)) - .route("/api/reset", post(operations::reset)) - .route("/api/test-emu", post(emulator::test_emu)) .route( "/api/emulator/avr8js/:session_id", get(emulator::avr8js_session_json), @@ -395,6 +404,17 @@ async fn main() { ); } + // SIGTERM (Linux/macOS): bounded controlled exit — refuse new operations, + // give in-flight ones a few seconds, flush zccache, exit. The graceful + // HTTP drain below would instead wait out every running build. + tokio::spawn({ + let ctx = context.clone(); + async move { + fbuild_daemon::shutdown::terminate_signal().await; + fbuild_daemon::shutdown::exit_on_terminate(ctx).await + } + }); + // Spawn background maintenance task (self-eviction, idle timeout, stale lock cleanup) { let ctx = context.clone(); @@ -602,33 +622,9 @@ async fn main() { tracing::error!("server error: {}", e); }); - // Clean up PID and port files, and the soldr-style owner claim. - let _ = fbuild_core::fs::remove_file(&pid_file).await; - let _ = fbuild_core::fs::remove_file(&port_file).await; - fbuild_paths::daemon_ownership::remove_owner_claim(); - // ...and the status file, which was previously left behind on every clean - // shutdown, so `daemon status` kept reporting a dead PID (#1213 part 2). - let _ = fbuild_core::fs::remove_file(&fbuild_paths::get_daemon_status_file()).await; - - // FastLED/fbuild#1480: `process::exit` runs no destructors and the - // backend lives in a `OnceLock`, so without this flush zccache never - // persisted `metadata.bin` (or the latest depgraph/index) and every - // restart began cold (zackees/zccache#1652). A normal flush takes well - // under 100 ms. The bound stays below `fbuild daemon stop`'s 5 s graceful - // budget and the 10 s a replacement daemon waits for the root-ownership - // lock this process still holds, so a slow flush never turns a stop or - // restart into a kill or a failed spawn. - if let Some(backend) = fbuild_build::compile_backend::get_global() { - const EXIT_FLUSH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(4); - match tokio::time::timeout(EXIT_FLUSH_TIMEOUT, backend.service().flush()).await { - Ok(Ok(())) => tracing::info!("zccache backend flushed"), - Ok(Err(err)) => tracing::warn!("zccache backend flush on exit failed: {err}"), - Err(_) => tracing::warn!( - "zccache backend flush on exit timed out after {}s", - EXIT_FLUSH_TIMEOUT.as_secs() - ), - } - } + // Clean up PID/port/status files and the soldr-style owner claim, and + // flush the embedded zccache backend. + fbuild_daemon::shutdown::persist_and_clean_up().await; tracing::info!("daemon exiting"); std::process::exit(0); diff --git a/crates/fbuild-daemon/src/shutdown.rs b/crates/fbuild-daemon/src/shutdown.rs new file mode 100644 index 000000000..f99196786 --- /dev/null +++ b/crates/fbuild-daemon/src/shutdown.rs @@ -0,0 +1,207 @@ +//! Daemon exit paths: refuse new work once shutdown starts, a bounded exit on +//! SIGTERM, and the persist-and-clean-up step every clean exit runs. +//! +//! Before this module a SIGTERM on Linux/macOS (`fbuild daemon kill`, the +//! escalation in `fbuild daemon stop`, `docker stop`, CI teardown) killed the +//! daemon outright: no zccache flush, stale pid/port files. Routing SIGTERM +//! through the graceful HTTP drain is not an option either, because that +//! drain waits for every open request and a build is one long request. +//! +//! SIGTERM instead gets a controlled exit: new operations are refused at +//! once, in-flight ones get [`SHUTDOWN_DRAIN_BUDGET`] to finish, then the +//! zccache state is flushed (bounded by [`EXIT_FLUSH_BUDGET`]) and the process +//! exits. Build children still running at that point are reaped by the +//! process containment group when the daemon exits. + +use crate::context::DaemonContext; +use crate::models::OperationResponse; +use axum::Json; +use axum::extract::{Request, State}; +use axum::http::StatusCode; +use axum::middleware::Next; +use axum::response::{IntoResponse, Response}; +use fbuild_core::daemon_health::{EXIT_FLUSH_BUDGET, SHUTDOWN_DRAIN_BUDGET}; +use std::sync::Arc; +use std::sync::atomic::Ordering; + +/// Message returned to operations refused because the daemon is stopping. +pub const SHUTTING_DOWN_MESSAGE: &str = "fbuild daemon is shutting down and accepts no new work; rerun the command to start a fresh daemon"; + +/// Middleware for operation routes (build, deploy, ...): once shutdown has +/// started, answer `503` with a failed [`OperationResponse`] instead of +/// starting work the exiting daemon would abandon. +pub async fn refuse_new_operations_when_shutting_down( + State(ctx): State>, + request: Request, + next: Next, +) -> Response { + if ctx.is_shutting_down.load(Ordering::Acquire) { + tracing::info!(path = %request.uri().path(), "refused operation: daemon is shutting down"); + return ( + StatusCode::SERVICE_UNAVAILABLE, + Json(OperationResponse::fail( + String::new(), + SHUTTING_DOWN_MESSAGE.to_string(), + )), + ) + .into_response(); + } + next.run(request).await +} + +/// Resolve when the process receives SIGTERM. Never resolves on Windows, +/// where close/logoff/shutdown events go through the console handler +/// (`register_daemon_shutdown_handler`) instead. +pub async fn terminate_signal() { + #[cfg(unix)] + { + use tokio::signal::unix::{SignalKind, signal}; + match signal(SignalKind::terminate()) { + Ok(mut sigterm) => { + sigterm.recv().await; + return; + } + Err(error) => { + tracing::warn!( + "cannot install SIGTERM handler ({error}); SIGTERM will kill the daemon without a flush" + ); + } + } + } + std::future::pending::<()>().await +} + +/// Controlled exit on SIGTERM: refuse new operations, give in-flight ones +/// [`SHUTDOWN_DRAIN_BUDGET`], persist, exit. +pub async fn exit_on_terminate(ctx: Arc) -> ! { + ctx.is_shutting_down.store(true, Ordering::Release); + let in_flight = ctx.active_operations.load(Ordering::Acquire); + tracing::info!( + in_flight, + "SIGTERM received: refusing new operations, waiting up to {}s for in-flight ones", + SHUTDOWN_DRAIN_BUDGET.as_secs() + ); + if ctx.wait_for_operations(SHUTDOWN_DRAIN_BUDGET).await { + tracing::info!("in-flight operations finished"); + } else { + tracing::warn!( + remaining = ctx.active_operations.load(Ordering::Acquire), + "in-flight operations still running after {}s; exiting anyway", + SHUTDOWN_DRAIN_BUDGET.as_secs() + ); + } + persist_and_clean_up().await; + tracing::info!("daemon exiting (SIGTERM)"); + std::process::exit(0) +} + +/// Remove this daemon's pid/port/claim/status records and flush the embedded +/// zccache backend. Every clean exit runs this before `process::exit`. +pub async fn persist_and_clean_up() { + let _ = fbuild_core::fs::remove_file(&fbuild_paths::get_daemon_pid_file()).await; + let _ = fbuild_core::fs::remove_file(&fbuild_paths::get_daemon_port_file()).await; + fbuild_paths::daemon_ownership::remove_owner_claim(); + // ...and the status file, which was previously left behind on every clean + // shutdown, so `daemon status` kept reporting a dead PID (#1213 part 2). + let _ = fbuild_core::fs::remove_file(&fbuild_paths::get_daemon_status_file()).await; + + // FastLED/fbuild#1480: `process::exit` runs no destructors and the + // backend lives in a `OnceLock`, so without this flush zccache never + // persisted `metadata.bin` (or the latest depgraph/index) and every + // restart began cold (zackees/zccache#1652). Bounded so a slow flush + // never outlasts the budgets of `fbuild daemon stop` / `kill` or the 10 s + // a replacement daemon waits for the root-ownership lock. + if let Some(backend) = fbuild_build::compile_backend::get_global() { + match tokio::time::timeout(EXIT_FLUSH_BUDGET, backend.service().flush()).await { + Ok(Ok(())) => tracing::info!("zccache backend flushed"), + Ok(Err(err)) => tracing::warn!("zccache backend flush on exit failed: {err}"), + Err(_) => tracing::warn!( + "zccache backend flush on exit timed out after {}s", + EXIT_FLUSH_BUDGET.as_secs() + ), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use axum::Router; + use axum::routing::post; + use std::time::{Duration, Instant}; + + fn context() -> Arc { + let (shutdown_tx, _shutdown_rx) = tokio::sync::watch::channel(false); + Arc::new(DaemonContext::new(0, shutdown_tx, ".".to_string())) + } + + async fn serve_gated(ctx: Arc) -> String { + let app = Router::new() + .route("/api/build", post(|| async { "built" })) + .route_layer(axum::middleware::from_fn_with_state( + Arc::clone(&ctx), + refuse_new_operations_when_shutting_down, + )) + .with_state(ctx); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}/api/build", listener.local_addr().unwrap()); + tokio::spawn(async move { axum::serve(listener, app).await }); + url + } + + #[tokio::test] + async fn operations_run_while_the_daemon_is_up() { + let url = serve_gated(context()).await; + let resp = fbuild_core::http::client().post(&url).send().await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(resp.text().await.unwrap(), "built"); + } + + #[tokio::test] + async fn operations_are_refused_once_shutdown_starts() { + let ctx = context(); + let url = serve_gated(Arc::clone(&ctx)).await; + ctx.is_shutting_down.store(true, Ordering::Release); + + let resp = fbuild_core::http::client().post(&url).send().await.unwrap(); + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + let body: serde_json::Value = resp.json().await.unwrap(); + assert_eq!(body["success"], false); + assert_eq!(body["message"], SHUTTING_DOWN_MESSAGE); + } + + #[tokio::test] + async fn drain_returns_as_soon_as_the_last_operation_ends() { + let ctx = context(); + ctx.active_operations.store(2, Ordering::Release); + let finisher = Arc::clone(&ctx); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(100)).await; + finisher.active_operations.fetch_sub(1, Ordering::AcqRel); + tokio::time::sleep(Duration::from_millis(100)).await; + finisher.active_operations.fetch_sub(1, Ordering::AcqRel); + }); + let started = Instant::now(); + assert!(ctx.wait_for_operations(Duration::from_secs(5)).await); + let waited = started.elapsed(); + assert!(waited >= Duration::from_millis(200), "{waited:?}"); + assert!(waited < Duration::from_secs(2), "{waited:?}"); + } + + #[tokio::test] + async fn drain_gives_up_at_its_budget() { + let ctx = context(); + ctx.active_operations.store(1, Ordering::Release); + let started = Instant::now(); + assert!(!ctx.wait_for_operations(Duration::from_millis(200)).await); + assert!(started.elapsed() < Duration::from_secs(2)); + } + + #[test] + fn terminate_exit_budget_covers_drain_and_flush() { + assert!( + fbuild_core::daemon_health::TERMINATE_EXIT_BUDGET + >= SHUTDOWN_DRAIN_BUDGET + EXIT_FLUSH_BUDGET + ); + } +} From 6c4e08b54049b0091ee9bf43c692a9f418ac054f Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 11:14:42 -0700 Subject: [PATCH 2/8] fix(daemon): move SIGTERM listener into the platform layer The platform boundary forbids #[cfg(unix)] outside fbuild_core::platform. Add fbuild_core::platform::process::daemon_terminate_signal() (tokio SIGTERM on Linux/macOS, never resolves on Windows, whose termination requests already arrive via register_daemon_shutdown_handler) and use it from the daemon. Regenerate ci/platform_boundary_research.tsv for the shifted windows/process.rs rows. --- ci/platform_boundary_research.tsv | 6 ++--- .../fbuild-core/src/platform/linux/process.rs | 15 +++++++++++++ .../fbuild-core/src/platform/macos/process.rs | 15 +++++++++++++ crates/fbuild-core/src/platform/process.rs | 7 ++++++ .../src/platform/windows/process.rs | 6 +++++ crates/fbuild-daemon/src/main.rs | 2 +- crates/fbuild-daemon/src/shutdown.rs | 22 ------------------- 7 files changed, 47 insertions(+), 26 deletions(-) diff --git a/ci/platform_boundary_research.tsv b/ci/platform_boundary_research.tsv index ce9ed91d4..328a317da 100644 --- a/ci/platform_boundary_research.tsv +++ b/ci/platform_boundary_research.tsv @@ -54,11 +54,11 @@ crates/fbuild-core/src/platform/windows/ipc.rs 4 native_path interprocess::os::w crates/fbuild-core/src/platform/windows/ipc.rs 5 native_path socket2:: ipc host_mechanic crates/fbuild-core/src/platform/windows/ipc.rs 6 native_path std::os::windows::io::AsRawSocket ipc host_mechanic crates/fbuild-core/src/platform/windows/mod.rs 13 compile_host_fact std::env::consts::ARCH host host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 77 native_path std::os::windows::ffi::OsStrExt process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 79 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 82 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 83 native_path std::os::windows::ffi::OsStrExt process host_mechanic crates/fbuild-core/src/platform/windows/process.rs 85 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/process.rs 88 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 91 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 94 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 13 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 24 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 28 native_path windows_sys:: process host_mechanic diff --git a/crates/fbuild-core/src/platform/linux/process.rs b/crates/fbuild-core/src/platform/linux/process.rs index 6ce457717..cf2a5aacb 100644 --- a/crates/fbuild-core/src/platform/linux/process.rs +++ b/crates/fbuild-core/src/platform/linux/process.rs @@ -9,6 +9,21 @@ pub(crate) fn register_daemon_shutdown_handler( Ok(()) } +pub(crate) async fn daemon_terminate_signal() { + use tokio::signal::unix::{SignalKind, signal}; + match signal(SignalKind::terminate()) { + Ok(mut sigterm) => { + sigterm.recv().await; + } + Err(error) => { + tracing::warn!( + "cannot install SIGTERM handler ({error}); SIGTERM will kill the daemon without a flush" + ); + std::future::pending::<()>().await + } + } +} + pub(crate) fn configure_tokio_owner_death( command: &mut tokio::process::Command, ) -> std::io::Result<()> { diff --git a/crates/fbuild-core/src/platform/macos/process.rs b/crates/fbuild-core/src/platform/macos/process.rs index 5c9ec2c11..d61ec9c96 100644 --- a/crates/fbuild-core/src/platform/macos/process.rs +++ b/crates/fbuild-core/src/platform/macos/process.rs @@ -9,6 +9,21 @@ pub(crate) fn register_daemon_shutdown_handler( Ok(()) } +pub(crate) async fn daemon_terminate_signal() { + use tokio::signal::unix::{SignalKind, signal}; + match signal(SignalKind::terminate()) { + Ok(mut sigterm) => { + sigterm.recv().await; + } + Err(error) => { + tracing::warn!( + "cannot install SIGTERM handler ({error}); SIGTERM will kill the daemon without a flush" + ); + std::future::pending::<()>().await + } + } +} + pub(crate) fn configure_tokio_owner_death( command: &mut tokio::process::Command, ) -> std::io::Result<()> { diff --git a/crates/fbuild-core/src/platform/process.rs b/crates/fbuild-core/src/platform/process.rs index c6fc59367..938d06a08 100644 --- a/crates/fbuild-core/src/platform/process.rs +++ b/crates/fbuild-core/src/platform/process.rs @@ -254,6 +254,13 @@ pub fn register_daemon_shutdown_handler( super::selected::process::register_daemon_shutdown_handler(shutdown_tx) } +/// Resolve when the host asks the daemon process to terminate (SIGTERM on +/// Unix). Never resolves on hosts whose termination requests arrive through +/// [`register_daemon_shutdown_handler`] instead (Windows close/logoff/shutdown). +pub async fn daemon_terminate_signal() { + super::selected::process::daemon_terminate_signal().await +} + /// Build the host-correct child environment while preserving caller overlays. pub(crate) fn command_environment( program: &str, diff --git a/crates/fbuild-core/src/platform/windows/process.rs b/crates/fbuild-core/src/platform/windows/process.rs index 4b223a89d..0b7bc3fab 100644 --- a/crates/fbuild-core/src/platform/windows/process.rs +++ b/crates/fbuild-core/src/platform/windows/process.rs @@ -17,6 +17,12 @@ unsafe impl Sync for JobHandle {} static TOKIO_JOB: OnceLock = OnceLock::new(); static SHUTDOWN_TX: OnceLock> = OnceLock::new(); +/// Windows delivers termination requests as console control events, which +/// `register_daemon_shutdown_handler` already routes to graceful shutdown. +pub(crate) async fn daemon_terminate_signal() { + std::future::pending::<()>().await +} + pub(crate) fn register_daemon_shutdown_handler( shutdown_tx: tokio::sync::watch::Sender, ) -> std::io::Result<()> { diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 89c346845..2dceaa221 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -410,7 +410,7 @@ async fn main() { tokio::spawn({ let ctx = context.clone(); async move { - fbuild_daemon::shutdown::terminate_signal().await; + fbuild_core::platform::process::daemon_terminate_signal().await; fbuild_daemon::shutdown::exit_on_terminate(ctx).await } }); diff --git a/crates/fbuild-daemon/src/shutdown.rs b/crates/fbuild-daemon/src/shutdown.rs index f99196786..263e58385 100644 --- a/crates/fbuild-daemon/src/shutdown.rs +++ b/crates/fbuild-daemon/src/shutdown.rs @@ -49,28 +49,6 @@ pub async fn refuse_new_operations_when_shutting_down( next.run(request).await } -/// Resolve when the process receives SIGTERM. Never resolves on Windows, -/// where close/logoff/shutdown events go through the console handler -/// (`register_daemon_shutdown_handler`) instead. -pub async fn terminate_signal() { - #[cfg(unix)] - { - use tokio::signal::unix::{SignalKind, signal}; - match signal(SignalKind::terminate()) { - Ok(mut sigterm) => { - sigterm.recv().await; - return; - } - Err(error) => { - tracing::warn!( - "cannot install SIGTERM handler ({error}); SIGTERM will kill the daemon without a flush" - ); - } - } - } - std::future::pending::<()>().await -} - /// Controlled exit on SIGTERM: refuse new operations, give in-flight ones /// [`SHUTDOWN_DRAIN_BUDGET`], persist, exit. pub async fn exit_on_terminate(ctx: Arc) -> ! { From cd4a4759826e4a137fc51a063388292969b96e55 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 13:50:55 -0700 Subject: [PATCH 3/8] fix(daemon): synchronize shutdown admission --- crates/fbuild-cli/src/cli/daemon_stop.rs | 4 + crates/fbuild-daemon/src/context.rs | 82 +++++++++++++++++-- crates/fbuild-daemon/src/handlers/health.rs | 3 +- .../src/handlers/operations/build.rs | 13 +-- .../src/handlers/operations/common.rs | 16 +++- crates/fbuild-daemon/src/shutdown.rs | 34 ++++++-- 6 files changed, 128 insertions(+), 24 deletions(-) diff --git a/crates/fbuild-cli/src/cli/daemon_stop.rs b/crates/fbuild-cli/src/cli/daemon_stop.rs index a75cfbce6..21bdd2e8b 100644 --- a/crates/fbuild-cli/src/cli/daemon_stop.rs +++ b/crates/fbuild-cli/src/cli/daemon_stop.rs @@ -84,6 +84,7 @@ const TERMINATION_BUDGET: std::time::Duration = std::time::Duration::from_secs(5 /// the forced kill. The daemon answers SIGTERM with a bounded controlled exit /// (drain in-flight operations, then flush zccache); escalating before that /// budget would kill it mid-flush. +#[cfg(unix)] const GRACEFUL_TERMINATION_BUDGET: std::time::Duration = std::time::Duration::from_secs(fbuild_core::daemon_health::TERMINATE_EXIT_BUDGET.as_secs() + 1); @@ -196,7 +197,10 @@ async fn terminate_and_confirm(pid: u32) -> fbuild_core::Result<()> { // A delivered terminate gets the daemon's full controlled-exit budget; a // refused one keeps the plain liveness wait. let graceful_budget = match kill_process(pid, false).await { + #[cfg(unix)] Ok(()) => GRACEFUL_TERMINATION_BUDGET, + #[cfg(not(unix))] + Ok(()) => TERMINATION_BUDGET, Err(error) => { tracing::debug!(pid, %error, "graceful terminate refused; escalating to a forced kill"); TERMINATION_BUDGET diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index 333d811f4..1255d2158 100644 --- a/crates/fbuild-daemon/src/context.rs +++ b/crates/fbuild-daemon/src/context.rs @@ -156,13 +156,15 @@ pub struct DaemonContext { pub serial_manager: Arc, /// Flag for graceful shutdown. pub is_shutting_down: Arc, + /// Serializes operation admission with the transition into shutdown. + operation_admission_gate: std::sync::Mutex<()>, /// Whether a build/deploy operation is currently in progress. pub operation_in_progress: Arc, /// Number of build/deploy/... operations in flight (`OperationGuard`s - /// alive). Unlike `operation_in_progress`, which the first of two - /// concurrent operations clears when it ends, this stays exact, so a - /// shutdown can wait for the last one (FastLED/fbuild#1480 follow-up). + /// alive), including work that outlives creation of a streaming response. pub active_operations: Arc, + /// Number of operation request futures admitted by the route middleware. + admitted_operation_requests: Arc, /// Number of WebSocket connections currently inside the serial-monitor /// handler (e.g. waiting for a port to open). Counted independently of /// `serial_manager` sessions because a port may take seconds to open @@ -270,8 +272,10 @@ impl DaemonContext { port, serial_manager: Arc::new(SharedSerialManager::new()), is_shutting_down: Arc::new(AtomicBool::new(false)), + operation_admission_gate: std::sync::Mutex::new(()), operation_in_progress: Arc::new(AtomicBool::new(false)), active_operations: Arc::new(AtomicUsize::new(0)), + admitted_operation_requests: Arc::new(AtomicUsize::new(0)), pending_serial_attaches: Arc::new(AtomicUsize::new(0)), pending_serial_attach_details: DashMap::new(), pending_serial_attach_next_id: AtomicU64::new(1), @@ -309,6 +313,60 @@ impl DaemonContext { } } + /// Admit an operation unless shutdown has begun. + /// + /// The gate is shared with [`Self::begin_shutdown`], so an admitted + /// request is counted before shutdown can observe the admission boundary. + pub fn begin_operation_admission(&self) -> Option { + let _gate = self.operation_admission_gate.lock().ok()?; + if self + .is_shutting_down + .load(std::sync::atomic::Ordering::Acquire) + { + return None; + } + self.admitted_operation_requests + .fetch_add(1, std::sync::atomic::Ordering::AcqRel); + Some(OperationAdmission { + admitted_operation_requests: Arc::clone(&self.admitted_operation_requests), + }) + } + + /// Atomically close admission and return the amount of work to drain. + pub fn try_begin_shutdown(&self, force: bool) -> Option { + let _gate = self + .operation_admission_gate + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let admitted = self + .admitted_operation_requests + .load(std::sync::atomic::Ordering::Acquire); + let active = self + .active_operations + .load(std::sync::atomic::Ordering::Acquire); + if !force && (admitted != 0 || active != 0) { + return None; + } + self.is_shutting_down + .store(true, std::sync::atomic::Ordering::Release); + Some(admitted + active) + } + + /// Atomically close operation admission for an unconditional shutdown. + pub fn begin_shutdown(&self) -> usize { + self.try_begin_shutdown(true) + .expect("forced shutdown admission cannot be refused") + } + + /// Number of admitted request futures plus active handler operations. + pub fn operation_drain_count(&self) -> usize { + self.admitted_operation_requests + .load(std::sync::atomic::Ordering::Acquire) + + self + .active_operations + .load(std::sync::atomic::Ordering::Acquire) + } + pub fn begin_pending_serial_attach(&self) -> u64 { let id = self .pending_serial_attach_next_id @@ -416,11 +474,7 @@ impl DaemonContext { pub async fn wait_for_operations(&self, budget: Duration) -> bool { let deadline = Instant::now() + budget; loop { - if self - .active_operations - .load(std::sync::atomic::Ordering::Acquire) - == 0 - { + if self.operation_drain_count() == 0 { return true; } if Instant::now() >= deadline { @@ -588,6 +642,18 @@ impl DaemonContext { } } +/// Keeps an admitted operation counted until its response future completes. +pub struct OperationAdmission { + admitted_operation_requests: Arc, +} + +impl Drop for OperationAdmission { + fn drop(&mut self) { + self.admitted_operation_requests + .fetch_sub(1, std::sync::atomic::Ordering::AcqRel); + } +} + fn now_unix() -> f64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/crates/fbuild-daemon/src/handlers/health.rs b/crates/fbuild-daemon/src/handlers/health.rs index b852ed52e..fdefc9f0d 100644 --- a/crates/fbuild-daemon/src/handlers/health.rs +++ b/crates/fbuild-daemon/src/handlers/health.rs @@ -235,7 +235,7 @@ pub async fn shutdown( let force = query.force.unwrap_or(false); let caller = ShutdownCaller::from_headers(peer, &headers); - if !force && ctx.operation_in_progress.load(Ordering::Relaxed) { + if ctx.try_begin_shutdown(force).is_none() { tracing::warn!( peer = %caller.peer, client_pid = caller.pid.as_deref().unwrap_or("unknown"), @@ -253,7 +253,6 @@ pub async fn shutdown( ); } - ctx.is_shutting_down.store(true, Ordering::Relaxed); let _ = ctx.shutdown_tx.send(true); tracing::info!( peer = %caller.peer, diff --git a/crates/fbuild-daemon/src/handlers/operations/build.rs b/crates/fbuild-daemon/src/handlers/operations/build.rs index 6838065cf..48f8ccc4c 100644 --- a/crates/fbuild-daemon/src/handlers/operations/build.rs +++ b/crates/fbuild-daemon/src/handlers/operations/build.rs @@ -288,7 +288,15 @@ pub async fn build( let guard_request_id = request_id.clone(); let worker_cancel = Arc::clone(&cancel_notify); let worker_fired_normal_terminal = Arc::clone(&fired_normal_terminal); + // Construct before spawning so middleware admission cannot end before + // the streaming worker is represented in the shutdown drain count. + let op_guard = OperationGuard::new( + &ctx, + fbuild_core::DaemonState::Building, + Some(format!("Building {}", project_dir_desc)), + ); tokio::spawn(async move { + let _op_guard = op_guard; let mut termination_guard = StreamTerminationGuard::new(async_tx.clone(), guard_request_id); // FBUILD_PERF_LOG=1 enables daemon-side coarse phase timing @@ -298,11 +306,6 @@ pub async fn build( .unwrap_or(false); let daemon_start = std::time::Instant::now(); - let _op_guard = OperationGuard::new( - &ctx, - fbuild_core::DaemonState::Building, - Some(format!("Building {}", project_dir_desc)), - ); // Daemon state goes to `Building` *before* the lock is taken // (the OperationGuard above). Without per-phase tracing, an // indefinite stall here looks identical to an indefinite diff --git a/crates/fbuild-daemon/src/handlers/operations/common.rs b/crates/fbuild-daemon/src/handlers/operations/common.rs index 9f66172a5..3ae1fc7cf 100644 --- a/crates/fbuild-daemon/src/handlers/operations/common.rs +++ b/crates/fbuild-daemon/src/handlers/operations/common.rs @@ -216,14 +216,26 @@ mod tests { assert_eq!(ctx.active_operations.load(Ordering::Acquire), 2); drop(first); - // The shutdown drain waits on this count, not on the bool the first - // finisher clears while the second is still running. assert_eq!(ctx.active_operations.load(Ordering::Acquire), 1); drop(second); assert_eq!(ctx.active_operations.load(Ordering::Acquire), 0); } + #[tokio::test] + async fn streaming_handoff_never_exposes_a_zero_drain_count() { + let (shutdown_tx, _shutdown_rx) = tokio::sync::watch::channel(false); + let ctx = Arc::new(DaemonContext::new(0, shutdown_tx, ".".to_string())); + let admission = ctx.begin_operation_admission().unwrap(); + let guard = OperationGuard::new(&ctx, fbuild_core::DaemonState::Building, None); + + drop(admission); + assert!(!ctx.wait_for_operations(std::time::Duration::ZERO).await); + + drop(guard); + assert!(ctx.wait_for_operations(std::time::Duration::ZERO).await); + } + #[test] fn operation_guard_drop_clears_dependency_install_through_context() { let (shutdown_tx, _shutdown_rx) = tokio::sync::watch::channel(false); diff --git a/crates/fbuild-daemon/src/shutdown.rs b/crates/fbuild-daemon/src/shutdown.rs index 263e58385..fb0147e1b 100644 --- a/crates/fbuild-daemon/src/shutdown.rs +++ b/crates/fbuild-daemon/src/shutdown.rs @@ -22,7 +22,6 @@ use axum::middleware::Next; use axum::response::{IntoResponse, Response}; use fbuild_core::daemon_health::{EXIT_FLUSH_BUDGET, SHUTDOWN_DRAIN_BUDGET}; use std::sync::Arc; -use std::sync::atomic::Ordering; /// Message returned to operations refused because the daemon is stopping. pub const SHUTTING_DOWN_MESSAGE: &str = "fbuild daemon is shutting down and accepts no new work; rerun the command to start a fresh daemon"; @@ -35,7 +34,7 @@ pub async fn refuse_new_operations_when_shutting_down( request: Request, next: Next, ) -> Response { - if ctx.is_shutting_down.load(Ordering::Acquire) { + let Some(_admission) = ctx.begin_operation_admission() else { tracing::info!(path = %request.uri().path(), "refused operation: daemon is shutting down"); return ( StatusCode::SERVICE_UNAVAILABLE, @@ -45,15 +44,14 @@ pub async fn refuse_new_operations_when_shutting_down( )), ) .into_response(); - } + }; next.run(request).await } /// Controlled exit on SIGTERM: refuse new operations, give in-flight ones /// [`SHUTDOWN_DRAIN_BUDGET`], persist, exit. pub async fn exit_on_terminate(ctx: Arc) -> ! { - ctx.is_shutting_down.store(true, Ordering::Release); - let in_flight = ctx.active_operations.load(Ordering::Acquire); + let in_flight = ctx.begin_shutdown(); tracing::info!( in_flight, "SIGTERM received: refusing new operations, waiting up to {}s for in-flight ones", @@ -63,7 +61,7 @@ pub async fn exit_on_terminate(ctx: Arc) -> ! { tracing::info!("in-flight operations finished"); } else { tracing::warn!( - remaining = ctx.active_operations.load(Ordering::Acquire), + remaining = ctx.operation_drain_count(), "in-flight operations still running after {}s; exiting anyway", SHUTDOWN_DRAIN_BUDGET.as_secs() ); @@ -106,6 +104,7 @@ mod tests { use super::*; use axum::Router; use axum::routing::post; + use std::sync::atomic::Ordering; use std::time::{Duration, Instant}; fn context() -> Arc { @@ -139,7 +138,7 @@ mod tests { async fn operations_are_refused_once_shutdown_starts() { let ctx = context(); let url = serve_gated(Arc::clone(&ctx)).await; - ctx.is_shutting_down.store(true, Ordering::Release); + ctx.begin_shutdown(); let resp = fbuild_core::http::client().post(&url).send().await.unwrap(); assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); @@ -148,6 +147,27 @@ mod tests { assert_eq!(body["message"], SHUTTING_DOWN_MESSAGE); } + #[tokio::test] + async fn shutdown_closes_admission_after_counting_existing_request() { + let ctx = context(); + let admission = ctx.begin_operation_admission().unwrap(); + + assert_eq!(ctx.begin_shutdown(), 1); + assert!(ctx.begin_operation_admission().is_none()); + + drop(admission); + assert!(ctx.wait_for_operations(Duration::ZERO).await); + } + + #[test] + fn non_forced_shutdown_is_refused_during_admission_gap() { + let ctx = context(); + let _admission = ctx.begin_operation_admission().unwrap(); + + assert!(ctx.try_begin_shutdown(false).is_none()); + assert!(ctx.begin_operation_admission().is_some()); + } + #[tokio::test] async fn drain_returns_as_soon_as_the_last_operation_ends() { let ctx = context(); From 8255c87cb5d7de007265fbe9f45c6146b7e5d3a8 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 14:11:11 -0700 Subject: [PATCH 4/8] fix(daemon): keep termination policy behind platform facade --- ci/platform_boundary_research.tsv | 10 +++++----- crates/fbuild-cli/src/cli/daemon_stop.rs | 13 +------------ crates/fbuild-core/src/platform/linux/process.rs | 4 ++++ crates/fbuild-core/src/platform/macos/process.rs | 4 ++++ crates/fbuild-core/src/platform/process.rs | 6 ++++++ crates/fbuild-core/src/platform/windows/process.rs | 4 ++++ crates/fbuild-daemon/src/context.rs | 5 ++++- 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/ci/platform_boundary_research.tsv b/ci/platform_boundary_research.tsv index 328a317da..2472463aa 100644 --- a/ci/platform_boundary_research.tsv +++ b/ci/platform_boundary_research.tsv @@ -54,11 +54,11 @@ crates/fbuild-core/src/platform/windows/ipc.rs 4 native_path interprocess::os::w crates/fbuild-core/src/platform/windows/ipc.rs 5 native_path socket2:: ipc host_mechanic crates/fbuild-core/src/platform/windows/ipc.rs 6 native_path std::os::windows::io::AsRawSocket ipc host_mechanic crates/fbuild-core/src/platform/windows/mod.rs 13 compile_host_fact std::env::consts::ARCH host host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 83 native_path std::os::windows::ffi::OsStrExt process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 85 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 88 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 91 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/process.rs 94 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 87 native_path std::os::windows::ffi::OsStrExt process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 89 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 92 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 95 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/process.rs 98 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 13 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 24 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/usb_pnp.rs 28 native_path windows_sys:: process host_mechanic diff --git a/crates/fbuild-cli/src/cli/daemon_stop.rs b/crates/fbuild-cli/src/cli/daemon_stop.rs index 21bdd2e8b..dfce1a8a5 100644 --- a/crates/fbuild-cli/src/cli/daemon_stop.rs +++ b/crates/fbuild-cli/src/cli/daemon_stop.rs @@ -80,14 +80,6 @@ const GRACEFUL_STOP_BUDGET: std::time::Duration = std::time::Duration::from_secs /// is not going. const TERMINATION_BUDGET: std::time::Duration = std::time::Duration::from_secs(5); -/// How long a daemon sent a graceful terminate (SIGTERM on Unix) gets before -/// the forced kill. The daemon answers SIGTERM with a bounded controlled exit -/// (drain in-flight operations, then flush zccache); escalating before that -/// budget would kill it mid-flush. -#[cfg(unix)] -const GRACEFUL_TERMINATION_BUDGET: std::time::Duration = - std::time::Duration::from_secs(fbuild_core::daemon_health::TERMINATE_EXIT_BUDGET.as_secs() + 1); - /// `fbuild daemon stop` — stop the daemon for *this* endpoint and report what /// actually happened. /// @@ -197,10 +189,7 @@ async fn terminate_and_confirm(pid: u32) -> fbuild_core::Result<()> { // A delivered terminate gets the daemon's full controlled-exit budget; a // refused one keeps the plain liveness wait. let graceful_budget = match kill_process(pid, false).await { - #[cfg(unix)] - Ok(()) => GRACEFUL_TERMINATION_BUDGET, - #[cfg(not(unix))] - Ok(()) => TERMINATION_BUDGET, + Ok(()) => fbuild_core::platform::process::daemon_graceful_termination_budget(), Err(error) => { tracing::debug!(pid, %error, "graceful terminate refused; escalating to a forced kill"); TERMINATION_BUDGET diff --git a/crates/fbuild-core/src/platform/linux/process.rs b/crates/fbuild-core/src/platform/linux/process.rs index cf2a5aacb..71af79ca6 100644 --- a/crates/fbuild-core/src/platform/linux/process.rs +++ b/crates/fbuild-core/src/platform/linux/process.rs @@ -24,6 +24,10 @@ pub(crate) async fn daemon_terminate_signal() { } } +pub(crate) fn daemon_graceful_termination_budget() -> std::time::Duration { + std::time::Duration::from_secs(crate::daemon_health::TERMINATE_EXIT_BUDGET.as_secs() + 1) +} + pub(crate) fn configure_tokio_owner_death( command: &mut tokio::process::Command, ) -> std::io::Result<()> { diff --git a/crates/fbuild-core/src/platform/macos/process.rs b/crates/fbuild-core/src/platform/macos/process.rs index d61ec9c96..f76896b13 100644 --- a/crates/fbuild-core/src/platform/macos/process.rs +++ b/crates/fbuild-core/src/platform/macos/process.rs @@ -24,6 +24,10 @@ pub(crate) async fn daemon_terminate_signal() { } } +pub(crate) fn daemon_graceful_termination_budget() -> std::time::Duration { + std::time::Duration::from_secs(crate::daemon_health::TERMINATE_EXIT_BUDGET.as_secs() + 1) +} + pub(crate) fn configure_tokio_owner_death( command: &mut tokio::process::Command, ) -> std::io::Result<()> { diff --git a/crates/fbuild-core/src/platform/process.rs b/crates/fbuild-core/src/platform/process.rs index 938d06a08..7396fff63 100644 --- a/crates/fbuild-core/src/platform/process.rs +++ b/crates/fbuild-core/src/platform/process.rs @@ -261,6 +261,12 @@ pub async fn daemon_terminate_signal() { super::selected::process::daemon_terminate_signal().await } +/// How long a caller should wait after a successfully delivered graceful +/// daemon termination request before escalating to a forced kill. +pub fn daemon_graceful_termination_budget() -> Duration { + super::selected::process::daemon_graceful_termination_budget() +} + /// Build the host-correct child environment while preserving caller overlays. pub(crate) fn command_environment( program: &str, diff --git a/crates/fbuild-core/src/platform/windows/process.rs b/crates/fbuild-core/src/platform/windows/process.rs index 0b7bc3fab..868bccd7e 100644 --- a/crates/fbuild-core/src/platform/windows/process.rs +++ b/crates/fbuild-core/src/platform/windows/process.rs @@ -23,6 +23,10 @@ pub(crate) async fn daemon_terminate_signal() { std::future::pending::<()>().await } +pub(crate) fn daemon_graceful_termination_budget() -> std::time::Duration { + std::time::Duration::from_secs(5) +} + pub(crate) fn register_daemon_shutdown_handler( shutdown_tx: tokio::sync::watch::Sender, ) -> std::io::Result<()> { diff --git a/crates/fbuild-daemon/src/context.rs b/crates/fbuild-daemon/src/context.rs index 1255d2158..33b2a8df8 100644 --- a/crates/fbuild-daemon/src/context.rs +++ b/crates/fbuild-daemon/src/context.rs @@ -344,7 +344,10 @@ impl DaemonContext { let active = self .active_operations .load(std::sync::atomic::Ordering::Acquire); - if !force && (admitted != 0 || active != 0) { + let operation_in_progress = self + .operation_in_progress + .load(std::sync::atomic::Ordering::Acquire); + if !force && (admitted != 0 || active != 0 || operation_in_progress) { return None; } self.is_shutting_down From 2161f26a9e2176b25d23ac6cbdd72c2b48039155 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 16:12:28 -0700 Subject: [PATCH 5/8] fix(daemon): test shutdown admission and refresh boundary inventories --- crates/fbuild-daemon/src/handlers/health.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fbuild-daemon/src/handlers/health.rs b/crates/fbuild-daemon/src/handlers/health.rs index fdefc9f0d..15e672721 100644 --- a/crates/fbuild-daemon/src/handlers/health.rs +++ b/crates/fbuild-daemon/src/handlers/health.rs @@ -346,7 +346,7 @@ mod tests { #[tokio::test] async fn shutdown_refuses_non_force_when_operation_in_progress() { let ctx = test_context(); - ctx.operation_in_progress.store(true, Ordering::Relaxed); + let _admission = ctx.begin_operation_admission().unwrap(); *ctx.current_operation.write().unwrap() = Some("Building C:/work/fastled".to_string()); let (status, body) = shutdown( From 300a367e14ea9703227f42fb529d5afe09f86d93 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 16:17:25 -0700 Subject: [PATCH 6/8] fix(paths): keep executable memo identity behind platform boundary --- crates/fbuild-core/src/platform/fs.rs | 5 +++++ crates/fbuild-core/src/platform/linux/fs.rs | 17 ++++++++++++++ crates/fbuild-core/src/platform/macos/fs.rs | 17 ++++++++++++++ crates/fbuild-core/src/platform/windows/fs.rs | 10 +++++++++ crates/fbuild-paths/src/executable_hash.rs | 22 ++----------------- 5 files changed, 51 insertions(+), 20 deletions(-) diff --git a/crates/fbuild-core/src/platform/fs.rs b/crates/fbuild-core/src/platform/fs.rs index 4a3bf2cbc..e28e46ebd 100644 --- a/crates/fbuild-core/src/platform/fs.rs +++ b/crates/fbuild-core/src/platform/fs.rs @@ -46,6 +46,11 @@ pub fn same_file(left: &Path, right: &Path) -> std::io::Result { Ok(file_identity(left)? == file_identity(right)?) } +/// Return a change-sensitive identity for a short-lived executable hash memo. +pub fn executable_memo_identity(metadata: &std::fs::Metadata) -> std::io::Result { + super::selected::fs::executable_memo_identity(metadata) +} + /// Normalize a lexical path into the host's comparison-key representation. #[must_use] pub fn comparison_key(path: &Path) -> String { diff --git a/crates/fbuild-core/src/platform/linux/fs.rs b/crates/fbuild-core/src/platform/linux/fs.rs index 3df813255..6e8eb2ba8 100644 --- a/crates/fbuild-core/src/platform/linux/fs.rs +++ b/crates/fbuild-core/src/platform/linux/fs.rs @@ -9,6 +9,23 @@ pub(crate) fn file_identity(path: &Path) -> std::io::Result { same_file::Handle::from_path(path) } +pub(crate) fn executable_memo_identity(metadata: &std::fs::Metadata) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + let modified = metadata + .modified()? + .duration_since(std::time::UNIX_EPOCH) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error))?; + Ok(format!( + "{} {} {} {} {}.{}", + metadata.len(), + modified.as_nanos(), + metadata.dev(), + metadata.ino(), + metadata.ctime(), + metadata.ctime_nsec() + )) +} + pub(crate) fn comparison_key(path: &Path) -> String { path.as_os_str().to_string_lossy().into_owned() } diff --git a/crates/fbuild-core/src/platform/macos/fs.rs b/crates/fbuild-core/src/platform/macos/fs.rs index 456203cb7..8d1d88761 100644 --- a/crates/fbuild-core/src/platform/macos/fs.rs +++ b/crates/fbuild-core/src/platform/macos/fs.rs @@ -9,6 +9,23 @@ pub(crate) fn file_identity(path: &Path) -> std::io::Result { same_file::Handle::from_path(path) } +pub(crate) fn executable_memo_identity(metadata: &std::fs::Metadata) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + let modified = metadata + .modified()? + .duration_since(std::time::UNIX_EPOCH) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error))?; + Ok(format!( + "{} {} {} {} {}.{}", + metadata.len(), + modified.as_nanos(), + metadata.dev(), + metadata.ino(), + metadata.ctime(), + metadata.ctime_nsec() + )) +} + pub(crate) fn comparison_key(path: &Path) -> String { path.as_os_str().to_string_lossy().to_lowercase() } diff --git a/crates/fbuild-core/src/platform/windows/fs.rs b/crates/fbuild-core/src/platform/windows/fs.rs index 2543ff211..274dedb81 100644 --- a/crates/fbuild-core/src/platform/windows/fs.rs +++ b/crates/fbuild-core/src/platform/windows/fs.rs @@ -11,6 +11,16 @@ pub(crate) fn file_identity(path: &Path) -> std::io::Result { same_file::Handle::from_path(path) } +pub(crate) fn executable_memo_identity(metadata: &std::fs::Metadata) -> std::io::Result { + Ok(format!( + "{} {} {} {}", + metadata.file_size(), + metadata.last_write_time(), + metadata.creation_time(), + metadata.file_attributes() + )) +} + pub(crate) fn comparison_key(path: &Path) -> String { let mut value = display_slash(path); value.make_ascii_lowercase(); diff --git a/crates/fbuild-paths/src/executable_hash.rs b/crates/fbuild-paths/src/executable_hash.rs index cbaf64e8a..90840d71b 100644 --- a/crates/fbuild-paths/src/executable_hash.rs +++ b/crates/fbuild-paths/src/executable_hash.rs @@ -42,7 +42,7 @@ pub fn memoized_blake3_file(path: &Path, memo_dir: &Path) -> io::Result io::Result io::Result { - metadata_identity(&path.metadata()?) -} - -#[cfg(unix)] -fn metadata_identity(metadata: &std::fs::Metadata) -> io::Result { - use std::os::unix::fs::MetadataExt; - let modified = metadata - .modified()? - .duration_since(std::time::UNIX_EPOCH) - .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?; - Ok(format!( - "{} {} {} {} {}.{}", - metadata.len(), - modified.as_nanos(), - metadata.dev(), - metadata.ino(), - metadata.ctime(), - metadata.ctime_nsec() - )) + fbuild_core::platform::fs::executable_memo_identity(&path.metadata()?) } #[cfg(unix)] From 2a8afafcd416156c29f580fa80b94cd40bdde06f Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 26 Sep 2026 22:10:27 -0700 Subject: [PATCH 7/8] Refresh platform boundary inventory after rebase --- ci/platform_boundary_ledger.tsv | 2 -- ci/platform_boundary_research.tsv | 34 +++++++++---------- ci/test_enforce_platform_boundary.py | 18 +++------- .../src/baseline.txt | 2 -- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/ci/platform_boundary_ledger.tsv b/ci/platform_boundary_ledger.tsv index be6b6f271..f47962701 100644 --- a/ci/platform_boundary_ledger.tsv +++ b/ci/platform_boundary_ledger.tsv @@ -32,8 +32,6 @@ crates/fbuild-paths/src/executable_hash.rs attr_cfg #[cfg(unix)] 1 host host_mec crates/fbuild-paths/src/executable_hash.rs attr_cfg #[cfg(unix)] 2 host host_mechanic crates/fbuild-paths/src/executable_hash.rs attr_cfg #[cfg(unix)] 3 host host_mechanic crates/fbuild-paths/src/executable_hash.rs attr_cfg #[cfg(unix)] 4 host host_mechanic -crates/fbuild-paths/src/executable_hash.rs attr_cfg #[cfg(unix)] 5 host host_mechanic -crates/fbuild-paths/src/executable_hash.rs native_path std::os::unix::fs::MetadataExt 0 fs host_mechanic crates/fbuild-python/tests/python_facades.rs native_path std::env::current_exe 0 host_executable host_mechanic crates/fbuild-toolchain/src/toolchain/esp_qemu.rs attr_cfg #[cfg(not(windows))] 0 host_executable host_mechanic crates/fbuild-toolchain/src/toolchain/esp_qemu.rs attr_cfg #[cfg(not(windows))] 1 host_executable host_mechanic diff --git a/ci/platform_boundary_research.tsv b/ci/platform_boundary_research.tsv index 2472463aa..0b18807c8 100644 --- a/ci/platform_boundary_research.tsv +++ b/ci/platform_boundary_research.tsv @@ -10,11 +10,12 @@ crates/fbuild-core/src/platform/linux/device.rs 33 native_path std::os::unix::fs crates/fbuild-core/src/platform/linux/device.rs 41 native_path libc:: process host_mechanic crates/fbuild-core/src/platform/linux/device.rs 41 native_path libc:: process host_mechanic crates/fbuild-core/src/platform/linux/fs.rs 2 native_path std::os::unix::fs::PermissionsExt fs host_mechanic -crates/fbuild-core/src/platform/linux/fs.rs 40 native_path std::os::unix::fs::symlink fs host_mechanic -crates/fbuild-core/src/platform/linux/fs.rs 84 native_path std::os::unix::ffi::OsStrExt process host_mechanic -crates/fbuild-core/src/platform/linux/fs.rs 89 native_path libc:: process host_mechanic -crates/fbuild-core/src/platform/linux/fs.rs 91 native_path libc:: process host_mechanic -crates/fbuild-core/src/platform/linux/fs.rs 101 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 13 native_path std::os::unix::fs::MetadataExt fs host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 57 native_path std::os::unix::fs::symlink fs host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 101 native_path std::os::unix::ffi::OsStrExt process host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 106 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 108 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/linux/fs.rs 118 native_path libc:: process host_mechanic crates/fbuild-core/src/platform/linux/ipc.rs 1 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/linux/ipc.rs 2 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/linux/ipc.rs 3 native_path interprocess::os::unix ipc host_mechanic @@ -23,11 +24,12 @@ crates/fbuild-core/src/platform/linux/ipc.rs 66 native_path std::os::unix::fs::P crates/fbuild-core/src/platform/linux/mod.rs 13 compile_host_fact std::env::consts::ARCH host host_mechanic crates/fbuild-core/src/platform/linux/process.rs 1 native_path std::os::unix::process::ExitStatusExt process host_mechanic crates/fbuild-core/src/platform/macos/fs.rs 2 native_path std::os::unix::fs::PermissionsExt fs host_mechanic -crates/fbuild-core/src/platform/macos/fs.rs 40 native_path std::os::unix::fs::symlink fs host_mechanic -crates/fbuild-core/src/platform/macos/fs.rs 84 native_path std::os::unix::ffi::OsStrExt process host_mechanic -crates/fbuild-core/src/platform/macos/fs.rs 89 native_path libc:: process host_mechanic -crates/fbuild-core/src/platform/macos/fs.rs 91 native_path libc:: process host_mechanic -crates/fbuild-core/src/platform/macos/fs.rs 101 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 13 native_path std::os::unix::fs::MetadataExt fs host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 57 native_path std::os::unix::fs::symlink fs host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 101 native_path std::os::unix::ffi::OsStrExt process host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 106 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 108 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/macos/fs.rs 118 native_path libc:: process host_mechanic crates/fbuild-core/src/platform/macos/ipc.rs 1 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/macos/ipc.rs 2 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/macos/ipc.rs 3 native_path socket2:: ipc host_mechanic @@ -43,10 +45,10 @@ crates/fbuild-core/src/platform/windows/device.rs 35 native_path windows_sys:: p crates/fbuild-core/src/platform/windows/fs.rs 2 native_path std::os::windows::ffi::OsStrExt process host_mechanic crates/fbuild-core/src/platform/windows/fs.rs 3 native_path std::os::windows::fs fs host_mechanic crates/fbuild-core/src/platform/windows/fs.rs 4 native_path std::os::windows::io::AsRawHandle process host_mechanic -crates/fbuild-core/src/platform/windows/fs.rs 50 native_path std::os::windows::fs::symlink_dir fs host_mechanic -crates/fbuild-core/src/platform/windows/fs.rs 78 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/fs.rs 133 native_path windows_sys:: process host_mechanic -crates/fbuild-core/src/platform/windows/fs.rs 177 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/fs.rs 60 native_path std::os::windows::fs::symlink_dir fs host_mechanic +crates/fbuild-core/src/platform/windows/fs.rs 88 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/fs.rs 143 native_path windows_sys:: process host_mechanic +crates/fbuild-core/src/platform/windows/fs.rs 187 native_path windows_sys:: process host_mechanic crates/fbuild-core/src/platform/windows/ipc.rs 1 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/windows/ipc.rs 2 native_path interprocess::local_socket ipc host_mechanic crates/fbuild-core/src/platform/windows/ipc.rs 3 native_path interprocess::os::windows ipc host_mechanic @@ -98,9 +100,7 @@ crates/fbuild-paths/src/executable_hash.rs 23 attr_cfg #[cfg(unix)] host host_me crates/fbuild-paths/src/executable_hash.rs 66 attr_cfg #[cfg(not(unix))] host host_mechanic crates/fbuild-paths/src/executable_hash.rs 71 attr_cfg #[cfg(unix)] host host_mechanic crates/fbuild-paths/src/executable_hash.rs 76 attr_cfg #[cfg(unix)] host host_mechanic -crates/fbuild-paths/src/executable_hash.rs 78 native_path std::os::unix::fs::MetadataExt fs host_mechanic -crates/fbuild-paths/src/executable_hash.rs 94 attr_cfg #[cfg(unix)] host host_mechanic -crates/fbuild-paths/src/executable_hash.rs 103 attr_cfg #[cfg(all(test,unix))] host host_mechanic +crates/fbuild-paths/src/executable_hash.rs 85 attr_cfg #[cfg(all(test,unix))] host host_mechanic crates/fbuild-python/tests/python_facades.rs 317 native_path std::env::current_exe host_executable host_mechanic crates/fbuild-toolchain/src/toolchain/esp_qemu.rs 528 attr_cfg #[cfg(windows)] host_executable host_mechanic crates/fbuild-toolchain/src/toolchain/esp_qemu.rs 534 attr_cfg #[cfg(windows)] host_executable host_mechanic diff --git a/ci/test_enforce_platform_boundary.py b/ci/test_enforce_platform_boundary.py index 8fd68e502..4c3dba961 100644 --- a/ci/test_enforce_platform_boundary.py +++ b/ci/test_enforce_platform_boundary.py @@ -15,10 +15,9 @@ def setUpClass(cls) -> None: def test_committed_exact_occurrence_ledger_matches_whole_tree(self) -> None: # Keep the row count explicit so additions to host mechanics require - # a deliberate inventory update. Serial PTY tests and the merged - # main-branch daemon/executable changes and the Windows file-URL - # resolver fixture and its Windows-only import bring the total to 46. - self.assertEqual(len(self.expected), 46) + # a deliberate inventory update. The executable identity now lives + # behind the platform boundary, leaving 44 occurrences. + self.assertEqual(len(self.expected), 44) self.assertFalse(boundary.validate_ledger(self.expected)) self.assertFalse(boundary.compare(self.expected, self.observed)) @@ -102,17 +101,10 @@ def test_no_raw_host_fact_reads_remain_outside_the_boundary(self) -> None: ) def test_no_filesystem_mechanics_remain_outside_the_boundary(self) -> None: - # The executable-hash implementation added on main uses Unix file - # metadata; keep that one exception exact and reject any new site. + # Filesystem mechanics belong behind the platform boundary. self.assertEqual( [(row.path, row.kind, row.normalized) for row in self.expected if row.capability == "fs"], - [ - ( - "crates/fbuild-paths/src/executable_hash.rs", - "native_path", - "std::os::unix::fs::MetadataExt", - ) - ], + [], ) def test_rp2040_filesystem_mechanics_use_the_neutral_facade(self) -> None: diff --git a/dylints/enforce_platform_boundary/src/baseline.txt b/dylints/enforce_platform_boundary/src/baseline.txt index f1c49f811..b10d9c6d4 100644 --- a/dylints/enforce_platform_boundary/src/baseline.txt +++ b/dylints/enforce_platform_boundary/src/baseline.txt @@ -31,8 +31,6 @@ crates/fbuild-paths/src/executable_hash.rs attr_cfg unix 3 crates/fbuild-paths/src/executable_hash.rs attr_cfg unix 4 crates/fbuild-paths/src/executable_hash.rs attr_cfg unix 5 crates/fbuild-paths/src/executable_hash.rs attr_cfg unix 6 -crates/fbuild-paths/src/executable_hash.rs attr_cfg unix 7 -crates/fbuild-paths/src/executable_hash.rs native_import std::os::unix 0 crates/fbuild-python/tests/python_facades.rs native_import std::env::current_exe 0 crates/fbuild-toolchain/src/toolchain/esp_qemu.rs attr_cfg windows 0 crates/fbuild-toolchain/src/toolchain/esp_qemu.rs attr_cfg windows 1 From 85ad1cd808151e118a2b6e876227488fa59c6baa Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sun, 27 Sep 2026 01:01:40 -0700 Subject: [PATCH 8/8] fix(daemon): keep SIGTERM process exit in binary entry point --- crates/fbuild-daemon/src/main.rs | 3 ++- crates/fbuild-daemon/src/shutdown.rs | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 2dceaa221..fa240481c 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -411,7 +411,8 @@ async fn main() { let ctx = context.clone(); async move { fbuild_core::platform::process::daemon_terminate_signal().await; - fbuild_daemon::shutdown::exit_on_terminate(ctx).await + fbuild_daemon::shutdown::drain_and_persist_on_terminate(ctx).await; + std::process::exit(0); } }); diff --git a/crates/fbuild-daemon/src/shutdown.rs b/crates/fbuild-daemon/src/shutdown.rs index fb0147e1b..9e4c62573 100644 --- a/crates/fbuild-daemon/src/shutdown.rs +++ b/crates/fbuild-daemon/src/shutdown.rs @@ -48,9 +48,9 @@ pub async fn refuse_new_operations_when_shutting_down( next.run(request).await } -/// Controlled exit on SIGTERM: refuse new operations, give in-flight ones -/// [`SHUTDOWN_DRAIN_BUDGET`], persist, exit. -pub async fn exit_on_terminate(ctx: Arc) -> ! { +/// Prepare a controlled SIGTERM exit: refuse new operations, give in-flight +/// ones [`SHUTDOWN_DRAIN_BUDGET`], then persist before the binary exits. +pub async fn drain_and_persist_on_terminate(ctx: Arc) { let in_flight = ctx.begin_shutdown(); tracing::info!( in_flight, @@ -67,8 +67,7 @@ pub async fn exit_on_terminate(ctx: Arc) -> ! { ); } persist_and_clean_up().await; - tracing::info!("daemon exiting (SIGTERM)"); - std::process::exit(0) + tracing::info!("daemon ready to exit (SIGTERM)"); } /// Remove this daemon's pid/port/claim/status records and flush the embedded