audit: sync code that could be async in fbuild-daemon (sub-issue of #813)

[zackees]

Intro

fbuild-daemon is the tokio runtime owner — the #[tokio::main] entry point that hosts the axum HTTP server, the WebSocket bridges, the broker IPC endpoint, the broadcast log/status hubs, and the background eviction/GC tasks. For the unified tokio-console story (meta #813) async-ness here matters most: any synchronous work executed inside the daemon process is opaque to tokio-console, and any blocking call made on a tokio worker thread starves every other task scheduled to that worker. Blocking inside an axum handler is the worst case because it ties up an HTTP connection while it holds the worker.

This audit found:

• One CRITICAL std::thread::spawn pair in broker/backend.rs that runs the running-process IPC endpoint and per-connection handlers entirely off the tokio runtime — fully invisible to tokio-console.
• Several HIGH-severity axum handlers in handlers/emulator/avr8js_* and handlers/emulator/qemu_deploy.rs that perform std::fs::* and sync ensure_avr8js_npm() (which itself shells out to npm install) directly on tokio worker threads.
• MEDIUM-severity sync serialport::available_ports() enumeration on the worker thread via DeviceManager::refresh_devices() (called from async list_devices, device_lease, device_release, device_preempt, and from deploy/build paths).
• A swarm of std::sync::Mutex / std::sync::RwLock on DaemonContext fields. Most are fine (short critical sections, never held across .await), but they are worth flagging as targets for the unified-runtime ideal.

The tokio::task::spawn_blocking calls in build/deploy/install_deps/reset/cache are correct: they wrap fbuild-build's BuildOrchestrator::build(), fbuild-deploy's Deployer::deploy(), fbuild_packages::DiskCache, and fbuild_deploy::reset::reset_device — all genuinely synchronous APIs. Keep these.

Findings table

Severity	File:line	Current pattern	Proposed async replacement	Notes
CRITICAL	crates/fbuild-daemon/src/broker/backend.rs:34-43	std::thread::Builder::new().name("fbuild-rp-backend").spawn(|| serve_backend_endpoint(...))	tokio::spawn(async move { serve_backend_endpoint_async(...).await }) after porting the body to tokio	Entire running-process broker backend lives outside the tokio runtime. Invisible to tokio-console, no shared shutdown signal, no graceful drain. The accept loop and per-connection handler are both bare threads.
CRITICAL	crates/fbuild-daemon/src/broker/backend.rs:64-77	for stream in listener.incoming() { std::thread::spawn(move || handle_backend_connection(...)) } (sync interprocess::local_socket::Listener::incoming())	tokio async local-socket (interprocess::local_socket::tokio::Listener if the feature is enabled, or tokio::net::UnixListener on unix + tokio::net::windows::named_pipe::NamedPipeServer on windows) with tokio::spawn per accepted stream	Each broker IPC connection currently spawns its own OS thread. Even if the body is short, this is the runtime owner inviting parallel sync work it cannot observe.
CRITICAL	crates/fbuild-daemon/src/broker/backend.rs:81-115	Sync Read::read_exact / Write::write_all via read_frame / write_frame from the running-process crate on a Read + Write stream	Async I/O variant of the framing helpers reading from tokio::io::AsyncRead + AsyncWrite	All broker IPC is currently blocking. Depends on a small upstream change to the running-process crate (cross-repo coordination).
HIGH	crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs:131,142,154,179,224	std::fs::create_dir_all / std::fs::copy / std::fs::write directly inside the pub async fn deploy_avr8js axum handler body	tokio::fs::create_dir_all, tokio::fs::copy, tokio::fs::write (all return futures that schedule the blocking syscall on the tokio blocking pool)	Five separate blocking syscalls on the tokio worker per AVR8js deploy. Compounded by the next finding.
HIGH	crates/fbuild-daemon/src/handlers/emulator/avr8js_deploy.rs:205	ensure_avr8js_npm() called directly (sync — spawns npm install via fbuild_core::subprocess::run_command) on a tokio worker	Wrap in tokio::task::spawn_blocking(|| ensure_avr8js_npm()).await OR port ensure_avr8js_npm to use tokio::process::Command	This is npm install blocking the axum handler. First-run cold path holds the worker for tens of seconds.
HIGH	crates/fbuild-daemon/src/handlers/emulator/qemu_deploy.rs:218	std::fs::create_dir_all in pub async fn deploy_qemu axum handler	tokio::fs::create_dir_all(&session_dir).await	Same pattern as avr8js_deploy.rs.
HIGH	crates/fbuild-daemon/src/handlers/emulator/avr8js_web.rs:172,225	std::fs::read_to_string(&manifest_path) inside async handlers avr8js_page, avr8js_session_json, avr8js_firmware_hex	tokio::fs::read_to_string(&manifest_path).await	Every GET to the emulator web UI synchronously reads session.json / firmware.hex on a tokio worker. Bursty browser refreshes will starve the worker pool.
HIGH	crates/fbuild-daemon/src/handlers/emulator/avr8js_npm.rs:81,98,130	std::fs::remove_dir_all (whole node_modules tree) and std::fs::create_dir_all inside ensure_avr8js_npm_in, reached from async deploy_avr8js without spawn_blocking	Wrap the whole ensure_avr8js_npm_in call site in spawn_blocking (simplest), or port to tokio::fs::*	Removing a populated node_modules is a multi-thousand-syscall blocking operation.
HIGH	crates/fbuild-daemon/src/handlers/operations/build.rs:528-541 (export_artifacts_bundle call after the spawn_blocking returns on the non-streaming build path)	Sync std::fs::create_dir_all + read_dir + copy + write (via common.rs:423-477) directly on the axum worker	Move into the existing spawn_blocking block (line 520) so the artifact export runs alongside the build, or wrap in a fresh spawn_blocking	Reading + copying every artifact in the build dir from the tokio worker on every successful non-streaming build.
HIGH	crates/fbuild-daemon/src/handlers/operations/deploy.rs:283-303	export_artifacts_bundle(...) called directly on the axum handler before the deploy spawn_blocking	Same fix — wrap in spawn_blocking	Same justification as build.rs:528.
MEDIUM	crates/fbuild-daemon/src/device_manager.rs:222-285	DeviceManager::refresh_devices() calls sync serialport::available_ports() (~20-30 ms on Windows). Called from pub async fn handlers via refresh_devices_and_broadcast_serial_moves (context.rs:386).	Either (a) move refresh_devices into spawn_blocking inside refresh_devices_and_broadcast_serial_moves, or (b) introduce async fn available_ports_async() -> Vec<SerialPortInfo> that wraps spawn_blocking once in fbuild-serial	Hits every list_devices / device_lease / device_release / device_preempt GET/POST. The trust-cache deploy path also re-enters here (deploy.rs:415).
MEDIUM	crates/fbuild-daemon/src/device_manager.rs:167-174	DeviceManager::{devices, last_refresh_at, recent_port_moves} all std::sync::Mutex<...>; locks acquired from async paths	tokio::sync::Mutex if any lock is ever held across .await (audit-pass needed); otherwise keep std Mutex (current critical sections look short — pure HashMap mutation, no I/O while holding)	Need to verify nothing holds these across .await. Quick read suggests it is safe today, but no static check.
MEDIUM	crates/fbuild-daemon/src/context.rs:149-153,165	DaemonContext::{daemon_state, current_operation, dependency_install} are std::sync::RwLock<...> and last_activity is std::sync::Mutex<Instant>	tokio::sync::RwLock<...> / tokio::sync::Mutex<Instant> only if held across .await. Today they are read with .read().unwrap_or_else(|e| e.into_inner()) in tight blocks (e.g. broker/backend.rs:149-154, context.rs:354-363). Keep std locks for now.	NOT A FINDING under the current usage. Flagged for unified-runtime ideal: if we ever want tokio-console to see every contention point, these get migrated.
MEDIUM	crates/fbuild-daemon/src/status_manager.rs:177,184,206,211,215,221-224	StatusManager::write_atomic does std::fs::create_dir_all + std::fs::write (temp) + std::fs::rename from update, update_operation, complete, set_idle. These methods are sync and called from the OperationGuard::drop impl and broadcast paths.	If callers are inside spawn_blocking already: keep. Otherwise: move file I/O into tokio::fs::* and make these methods async, or move calls into a background tokio::spawn that drains a channel of updates.	The Drop impl path (common.rs:191-202) is sync and fires from within spawn_blocking bodies and async handler scopes alike. Worth a follow-up pass to confirm whether the rename path ever runs on a worker.
MEDIUM	crates/fbuild-daemon/src/main.rs:186,189	std::fs::write(&pid_file, ...), std::fs::write(&port_file, ...) during startup	Could be tokio::fs::write(...).await for consistency, but startup-only one-shot.	Out-of-scope per audit rules (one-shot startup). Listed for completeness.
LOW	crates/fbuild-daemon/src/main.rs:580	std::thread::sleep(Duration::from_millis(500)) inside bind_listener_with_retry	tokio::time::sleep(...).await (the function is called inside #[tokio::main], so we are on the runtime here)	Blocks the runtime briefly during port retries. Fine in practice but inconsistent.
LOW	crates/fbuild-daemon/src/main.rs:677	std::thread::sleep(Duration::from_millis(3500)) inside Windows ctrl handler	Cannot be async — this is a C-ABI ctrl handler callback running on a system-spawned thread outside tokio. Keep.	Documented in code.
LOW	crates/fbuild-daemon/src/handlers/emulator/tests_process.rs:195-197	tokio::runtime::Runtime::new().unwrap().block_on(...) inside a test	Use #[tokio::test] instead	Test-only. Trivial fix but doesn't affect production runtime.
LOW	crates/fbuild-daemon/src/handlers/operations/build.rs:201	std::sync::mpsc::channel::<String>() for log_sender then bridged to async via spawn_blocking (build.rs:314)	Acceptable: the producer side is fbuild-build's synchronous BuildOrchestrator which can only write to std::sync::mpsc. The bridge is correctly spawn_blocking.	Keep. Documented in the call site.

spawn_blocking audit

Every tokio::task::spawn_blocking call in crates/fbuild-daemon/src/:

File:line	Body	Verdict
handlers/cache.rs:11	fbuild_packages::DiskCache::open() + dc.stats()	CORRECT — DiskCache is sync (rusqlite).
handlers/cache.rs:49	fbuild_packages::DiskCache::open() + dc.run_gc()	CORRECT — same.
handlers/operations/build.rs:314	Sync drain of std::sync::mpsc::Receiver<String> from fbuild-build's BuildLog channel into the NDJSON async_tx	CORRECT — sync_rx.iter() is a blocking iterator; only way to bridge sync mpsc into the async pipeline.
handlers/operations/build.rs:327	fbuild_build::get_orchestrator(platform)?.build(&params) (streaming path)	CORRECT — BuildOrchestrator::build is the canonical synchronous compile call.
handlers/operations/build.rs:520	Same as above (non-streaming path)	CORRECT.
handlers/operations/deploy.rs:237	fbuild_build::get_orchestrator(p)?.build(&params) from the build-then-deploy path	CORRECT.
handlers/operations/deploy.rs:418	Full ESP32 / AVR / Teensy / LPC deployer dispatch, including deployer.deploy(...), esptool verify-flash calls, etc.	CORRECT — Deployer::deploy is sync, and this closure does heavy CPU + subprocess work that genuinely belongs off the runtime.
handlers/operations/deploy.rs:772	deployer.post_deploy_recovery(&port_name)	CORRECT — post_deploy_recovery is sync (DTR/RTS toggles via blocking serialport API).
handlers/operations/install_deps.rs:96	fbuild_build::install_platform_deps(platform, &project_dir)	CORRECT — install_platform_deps is sync (downloads + extracts toolchains via blocking reqwest).
handlers/operations/reset.rs:44	fbuild_deploy::reset::reset_device(&platform_str, &port, verbose)	CORRECT — sync serialport DTR/RTS toggling.
handlers/emulator/select.rs:310	Emulator runner selection / artifact prep (sync fbuild_packages::Package::ensure_installed)	CORRECT.

All eleven spawn_blocking calls in fbuild-daemon are wrapping genuinely-synchronous APIs. None of them would meaningfully migrate to plain tokio::spawn. The fix for the tokio-console story is upstream: when fbuild-build / fbuild-deploy / fbuild-packages grow async APIs, the call sites simplify. Not in scope for this audit.

What was searched

Patterns (via Grep over crates/fbuild-daemon/src/):

• std::thread::spawn / thread::spawn → 1 hit (broker/backend.rs:69)
• std::thread::Builder → 1 hit (broker/backend.rs:34)
• rayon:: → 0 hits (good)
• std::process::Command / std::process::Stdio → 5 hits, all in bin/containment_harness.rs (separate test binary) or as Stdio imports used by tokio::process::Command callers. No sync subprocess invocations in handlers themselves.
• tokio::task::spawn_blocking → 11 hits, all audited above.
• Mutex::new / RwLock::new → 11 production-code hits, audited per finding above.
• std::sync::mpsc → 1 hit (build.rs:201), correctly bridged.
• std::fs:: → 41 hits — narrowed to async handler bodies (HIGH findings) vs spawn_blocking bodies (correct) vs startup/Drop paths (low).
• block_on / Builder::new_current_thread / Runtime::new → 1 hit (test code only — tests_process.rs:195).
• reqwest::blocking → 0 hits (good).
• OnceLock → all uses are for one-shot global state (compile_backend handle, ctrl handler shutdown_tx). None gate async init.

Files inspected in full or in detail:

• crates/fbuild-daemon/src/main.rs — runtime construction, single #[tokio::main], no rogue runtimes.
• crates/fbuild-daemon/src/context.rs — DaemonContext field types.
• crates/fbuild-daemon/src/status_manager.rs — StatusManager sync file writes.
• crates/fbuild-daemon/src/device_manager.rs — DeviceManager sync serialport::available_ports().
• crates/fbuild-daemon/src/log_layer.rs — BroadcastLogLayer (clean: only broadcast::Sender::send, zero blocking).
• crates/fbuild-daemon/src/watch_set_cache.rs — DashMap + atomics (lock-free fast path; clean).
• crates/fbuild-daemon/src/broker/{backend,session,service,protocol,mod}.rs — IPC.
• crates/fbuild-daemon/src/handlers/{cache,devices,health,locks,websockets}.rs.
• crates/fbuild-daemon/src/handlers/operations/{build,deploy,install_deps,reset,common,monitor,tests}.rs.
• crates/fbuild-daemon/src/handlers/emulator/{avr8js_deploy,avr8js_headless,avr8js_npm,avr8js_web,qemu_deploy,runners,shared,select}.rs.

Out-of-scope notes

• fbuild-build / fbuild-deploy / fbuild-packages internals are out of scope. They have their own sub-issues under audit: go fully async — whole-app tokio runtime sharing for tokio-console (meta) #813. The fbuild-daemon side is correct in wrapping them with spawn_blocking.
• tokio::process::Command vs std::process::Command: the daemon already uses tokio::process::Command for QEMU, simavr, and Node.js subprocesses (see handlers/emulator/shared.rs:183, avr8js_headless.rs:58). The remaining std::process::Command usage is in bin/containment_harness.rs, a separate harness binary that doesn't run inside the daemon process — out of scope.
• Drop impls are not flagged. StreamTerminationGuard::drop (build.rs:40), OperationGuard::drop (common.rs:191) both do sync work (channel send, RwLock write, atomic store). Per the audit rules, this is fine — Drop cannot be async without bigger restructuring.
• Tests that use std::sync::Mutex for env-lock serialization (broker/service.rs:200, handlers/operations/tests.rs:77, handlers/emulator/tests_npm_cache.rs:15) and tokio::runtime::Runtime::new() for one-shot test setup are out of scope.
• The single Builder::new_current_thread check found zero hits. The daemon is unambiguously a single multi-thread #[tokio::main] runtime.

Sub-issue of #813.