fix(daemon): cancel in-flight build when CLI disconnects (#853)

[zackees]

Summary

Fixes #853 — force-killing the fbuild CLI mid-build no longer leaves zombie compiler subprocesses or a wedged project lock.

Two coordinated changes:

1. crates/fbuild-core/src/containment.rs — set kill_on_drop(true) on every tokio::process::Command routed through tokio_spawn::spawn_contained. Daemon-death containment (Job Object on Windows, PR_SET_PDEATHSIG on Linux) only fires when the daemon itself exits; for in-daemon cancellation (CLI disconnect → body drop → build-task abort), we need tokio-level drop-kill so aborting the orchestrator's JoinHandle actually terminates the in-flight subprocesses it was awaiting.

2. crates/fbuild-daemon/src/handlers/operations/build.rs — wire a tokio::sync::Notify between the streaming response body and the build worker. A CancelOnDrop guard bundled into the stream::unfold body state fires notify_waiters() when hyper drops the body (the canonical client-disconnect signal in axum — there's no built-in disconnect callback). The build worker's loop changes from tokio::time::timeout(&mut build_task) to a biased tokio::select! that races the cancel signal against build-task completion and the heartbeat tick. On cancel it build_task.abort()s, drains the JoinHandle so all child Drops run (and kill_on_drop fires), then returns a clean cancellation error. A fired_normal_terminal atomic flag suppresses the guard on normal completion so a clean client read-then-disconnect isn't misread as a cancel.

Why these two together

Either change alone is insufficient:

• Cancel signal without kill_on_drop → orchestrator's JoinHandle aborts and drops the in-flight Child handle, but the OS process keeps running (only daemon death kills it).
• kill_on_drop without a cancel signal → nothing ever drops the Child because the build worker is detached from the HTTP request lifetime; the only abort path was the 60-min hard deadline.

Test plan

• soldr cargo check -p fbuild-daemon -p fbuild-core --all-targets — clean
• soldr cargo clippy -p fbuild-daemon -p fbuild-core --all-targets -- -D warnings — clean
• cancel_on_drop_fires_when_not_completed — guard fires notify_waiters when body drops mid-build (deferred to CI — local Windows MSVC linker env was wedged at the time of authorship)
• cancel_on_drop_silent_when_completed — guard suppressed after fired_normal_terminal set (same)
• Manual: start fbuild build <large-project>, kill -9 the CLI, verify compiler subprocesses gone within ~500ms and next fbuild build for same project starts immediately (acceptance criteria from Daemon doesn't cancel in-flight builds when CLI is force-killed (zombie build processes) #853)
• Manual: verify normal build completion still works end-to-end (the fired_normal_terminal arm)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Build streaming now stops when the client disconnects, helping avoid unnecessary work and improving responsiveness.
  • Background subprocesses are now cleaned up more reliably if the owning task is dropped or cancelled.

• Bug Fixes

  • Prevented builds from continuing after a dropped response stream.
  • Reduced the chance of stray processes remaining active after cancellation.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b64dba36-c340-4c40-8f05-c5aae573e700

📥 Commits

Reviewing files that changed from the base of the PR and between b3487ed and a701db2.

📒 Files selected for processing (2)

• crates/fbuild-core/src/containment.rs
• crates/fbuild-daemon/src/handlers/operations/build.rs

---

📝 Walkthrough

Walkthrough

Adds subprocess kill_on_drop(true) in tokio_spawn::spawn_contained and a client-disconnect cancellation path in the streaming build handler via a CancelOnDrop drop-guard, StreamBodyState, a fired_normal_terminal flag, and a biased tokio::select! replacing the previous timeout loop.

Client-Disconnect Build Cancellation

Layer / File(s)	Summary
Subprocess kill-on-drop crates/fbuild-core/src/containment.rs	tokio_spawn::spawn_contained now calls command.kill_on_drop(true) before spawning, so dropping the Child handle terminates the OS process.
CancelOnDrop guard and StreamBodyState crates/fbuild-daemon/src/handlers/operations/build.rs	Introduces CancelOnDrop (drop-guard that calls Notify::notify_waiters unless fired_normal_terminal is set) and StreamBodyState (bundles NDJSON receiver with the guard). Initializes shared Notify and Arc<AtomicBool> cancel primitives and clones them into the worker task.
Biased select! loop and terminal-event wiring crates/fbuild-daemon/src/handlers/operations/build.rs	Replaces timeout(STREAM_STATUS_INTERVAL, &mut build_task) with a biased tokio::select! racing build completion, worker_cancel.notified() (aborts build task and drains its JoinHandle), and sleep(STREAM_STATUS_INTERVAL) for heartbeats. After the terminal event, sets fired_normal_terminal and constructs the response body as unfold over StreamBodyState.
CancelOnDrop unit tests crates/fbuild-daemon/src/handlers/operations/build.rs	Async tests verify the guard notifies waiters on drop when fired_normal_terminal is false, and stays silent when it is already true.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant Hyper
  participant StreamBodyState
  participant Worker
  participant BuildTask
  participant Subprocess

  CLI->>Hyper: TCP disconnect / force-kill
  Hyper->>StreamBodyState: drop response body
  StreamBodyState->>Worker: CancelOnDrop::drop → Notify::notify_waiters
  Worker->>BuildTask: abort()
  BuildTask->>Subprocess: Child dropped → kill_on_drop kills OS process
  BuildTask-->>Worker: JoinHandle resolved
  Worker-->>Worker: release project_lock

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• FastLED/fbuild#254: Modifies the same tokio_spawn::spawn_contained code path in containment.rs.

Poem

🐇 A build once ran on, though the client had fled,
Now kill_on_drop ensures subprocesses are dead.
The CancelOnDrop guard watches the stream,
A select! loop makes cancellation supreme.
No zombie compilers haunting the night —
The daemon now tidies each orphaned plight! ✨

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/853-cancel-build-on-cli-disconnect

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}