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

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

spawn_blocking audit

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

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.