Skip to content

fix(timeout): close #802 audit family — workspace-wide timeouts on blocking ops#843

Merged
zackees merged 1 commit into
mainfrom
fix/timeout-audit-802
Jun 29, 2026
Merged

fix(timeout): close #802 audit family — workspace-wide timeouts on blocking ops#843
zackees merged 1 commit into
mainfrom
fix/timeout-audit-802

Conversation

@zackees

@zackees zackees commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary

Comprehensive timeout sweep addressing all 9 open sub-issues of the #802 meta audit ("blocking operations with no timeout"). Nine parallel agents worked on disjoint crate scopes; this PR consolidates their changes.

What's in this PR (by sub-issue)

Sub-issue Scope Key changes
#803 fbuild-serial write_to_port + reader close wrapped in 2s tokio::time::timeout; esp_hard_reset 3s cap; preemption.rs switched to std::sync::Mutex (never held across .await)
#804 fbuild-deploy Native espflash entry points capped: write 180s, verify 30s, selective write 180s
#805 fbuild-packages Streaming body chunk reads get per-chunk 60s tokio::time::timeout; install_lock honors FBUILD_INSTALL_LOCK_STALE_SECS env override
#806 tests + benches HIGH/MEDIUM async tests wrapped in tokio::time::timeout helpers; CLI integration tests use Child::try_wait poll deadlines
#807 fbuild-core subprocess::run_command* family resolves a default 15-min cap when caller passes None (overridable via FBUILD_SUBPROCESS_DEFAULT_TIMEOUT_SECS); explicit run_command_no_timeout helpers for legitimately-unbounded sites; publish_install_status wraps subscriber in catch_unwind
#808 fbuild-daemon 14 handler/loop/channel sites bounded — build/deploy/install/monitor caps, WebSocket 10s attach handshake + 300s idle, serialport::available_ports moved to spawn_blocking + 5s cap, broker session adopt/request 10s
#809 fbuild-build 22 subprocess sites bounded — per-TU compile 300s, linkers 180s, archive 120s, objcopy 60s, nm/c++filt 30-60s, version probes 5s, esp32 elf2image/bootloader 30s
#810 fbuild-cli + fbuild-python daemon-client build/shutdown caps; clang-tools subprocess output 120s; serial monitor WS handshake 5s across sync + async surfaces
#812 CI workflows + scripts timeout-minutes across 20 GHA workflows; curl --max-time/--connect-timeout/--retry; pip --timeout 60; subprocess.run timeout= kwargs in 9 ci/ helpers

Test plan

  • soldr cargo check --workspace --all-targets — clean
  • soldr cargo clippy --workspace --all-targets -- -D warnings — clean
  • soldr cargo test --workspace --no-fail-fast — exit 0

Deferred (out of scope, noted in agent reports)

Closes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added execution time limits across build, CI, deployment, monitoring, and CLI workflows to reduce hangs and improve reliability.
    • Introduced default timeout handling for subprocesses, with several commands now failing fast when tools or network requests stall.
  • Bug Fixes

    • Prevented long-running daemon, serial, and emulator operations from blocking indefinitely.
    • Added safer handling for stalled downloads, toolchain checks, and test runs, including clearer timeout failures.

…ocking ops

Comprehensive timeout sweep addressing all 9 open sub-issues of the
#802 meta audit. Nine parallel agents worked on disjoint crate scopes;
this commit consolidates their changes.

#803 fbuild-serial: write_to_port + reader close wrapped in 2s
tokio::time::timeout; esp_hard_reset gets 3s cap; preemption.rs uses
std::sync::Mutex (never held across .await).

#804 fbuild-deploy: native espflash entry points capped — write 180s,
verify 30s, selective write 180s.

#805 fbuild-packages: streaming body chunk reads get per-chunk 60s
tokio::time::timeout; install_lock stale-after honors
FBUILD_INSTALL_LOCK_STALE_SECS env override.

#806 tests + benches: HIGH/MEDIUM async tests wrapped in
tokio::time::timeout helpers; CLI integration tests use Child::try_wait
poll deadlines. 15min budget for hardware-toolchain-download tests.

#807 fbuild-core: subprocess::run_command* family resolves a default
15-min cap when caller passes None timeout (overridable via
FBUILD_SUBPROCESS_DEFAULT_TIMEOUT_SECS); explicit run_command_no_timeout
helpers added for legitimately-unbounded sites. publish_install_status
wraps subscriber call in catch_unwind so panicking subscriber can't
unwind into hot path.

#808 fbuild-daemon: 14 handler / loop / channel sites bounded —
build/deploy/install/monitor caps, WebSocket 10s attach handshake +
300s idle deadline, serialport::available_ports moved to
spawn_blocking + 5s cap, broker session adopt/request 10s caps.

#809 fbuild-build: 22 subprocess sites bounded — per-TU compile 300s,
linkers 180s, archive 120s, objcopy 60s, nm/c++filt 30-60s, version
probes 5s, esp32 elf2image / bootloader merges 30s.

#810 fbuild-cli + fbuild-python: daemon-client build/shutdown caps;
clang-tools subprocess output 120s; serial monitor WS handshake 5s
across both sync + async surfaces.

#812 CI scripts + workflows: timeout-minutes added across 20 GHA
workflows; curl --max-time / --connect-timeout / --retry on uncapped
download sites; pip install --timeout 60; subprocess.run timeout=
kwargs added to 9 ci/ Python helpers.

Also: const _: () = assert!(...) for compile-time bounds in
subprocess.rs so clippy's assertions_on_constants doesn't fire.

Closes #803, #804, #805, #806, #807, #808, #809, #810, #812.
Closes #802 (meta — all sub-issues addressed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zackees
zackees merged commit ba1ed30 into main Jun 29, 2026
79 of 97 checks passed
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

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: a03bd31c-10df-402b-b1d6-5699da36e20b

📥 Commits

Reviewing files that changed from the base of the PR and between 596ae19 and bb75a64.

📒 Files selected for processing (91)
  • .github/workflows/add-to-project.yml
  • .github/workflows/build.yml
  • .github/workflows/check-macos.yml
  • .github/workflows/check-ubuntu.yml
  • .github/workflows/check-windows.yml
  • .github/workflows/ci-workflow-drift.yml
  • .github/workflows/crate-gate.yml
  • .github/workflows/docs.yml
  • .github/workflows/dylint.yml
  • .github/workflows/fmt.yml
  • .github/workflows/hw-ci.yml
  • .github/workflows/lint-subprocess.yml
  • .github/workflows/loc-gate.yml
  • .github/workflows/msrv.yml
  • .github/workflows/nightly-platforms.yml
  • .github/workflows/release-auto.yml
  • .github/workflows/template_build.yml
  • .github/workflows/template_native_build.yml
  • .github/workflows/update-data.yml
  • .github/workflows/validate-boards.yml
  • ci/bench_uv_run.py
  • ci/build_dist.py
  • ci/build_dylint_driver.py
  • ci/hooks/check-on-start.py
  • ci/hooks/check-on-stop.py
  • ci/hooks/code-review-on-stop.py
  • ci/hooks/lint.py
  • ci/lint.py
  • ci/test.py
  • crates/fbuild-build/src/avr/avr_linker.rs
  • crates/fbuild-build/src/ch32v/ch32v_linker.rs
  • crates/fbuild-build/src/compiler.rs
  • crates/fbuild-build/src/esp32/esp32_linker.rs
  • crates/fbuild-build/src/esp32/orchestrator/embed.rs
  • crates/fbuild-build/src/esp8266/esp8266_linker.rs
  • crates/fbuild-build/src/generic_arm/arm_linker.rs
  • crates/fbuild-build/src/linker.rs
  • crates/fbuild-build/src/nrf52/nrf52_linker.rs
  • crates/fbuild-build/src/pipeline/compile.rs
  • crates/fbuild-build/src/renesas/renesas_linker.rs
  • crates/fbuild-build/src/sam/sam_linker.rs
  • crates/fbuild-build/src/script_runtime.rs
  • crates/fbuild-build/src/shrink/probe.rs
  • crates/fbuild-build/src/silabs/silabs_linker.rs
  • crates/fbuild-build/src/stm32/orchestrator/arduino_mbed.rs
  • crates/fbuild-build/src/symbol_analyzer/mod.rs
  • crates/fbuild-build/src/teensy/teensy_linker.rs
  • crates/fbuild-build/tests/avr_build.rs
  • crates/fbuild-build/tests/compile_many_stage2_perf.rs
  • crates/fbuild-build/tests/eh_frame_strip_esp32.rs
  • crates/fbuild-build/tests/esp32_build.rs
  • crates/fbuild-build/tests/lite_scons_acceptance.rs
  • crates/fbuild-build/tests/nxplpc_build_flags.rs
  • crates/fbuild-build/tests/nxplpc_core_compile_commands.rs
  • crates/fbuild-build/tests/stm32_acceptance.rs
  • crates/fbuild-build/tests/teensy30_acceptance.rs
  • crates/fbuild-build/tests/teensy41_acceptance.rs
  • crates/fbuild-build/tests/teensy_build.rs
  • crates/fbuild-build/tests/teensylc_acceptance.rs
  • crates/fbuild-cli/src/cli/build.rs
  • crates/fbuild-cli/src/cli/clang_tools.rs
  • crates/fbuild-cli/src/cli/daemon_cmd.rs
  • crates/fbuild-cli/src/cli/pio.rs
  • crates/fbuild-cli/src/daemon_client.rs
  • crates/fbuild-cli/tests/ci_command.rs
  • crates/fbuild-cli/tests/lib_select.rs
  • crates/fbuild-cli/tests/test_emu_exit_code.rs
  • crates/fbuild-core/src/install_status.rs
  • crates/fbuild-core/src/subprocess.rs
  • crates/fbuild-daemon/src/broker/session.rs
  • crates/fbuild-daemon/src/context.rs
  • crates/fbuild-daemon/src/handlers/emulator/avr8js_headless.rs
  • crates/fbuild-daemon/src/handlers/emulator/select.rs
  • crates/fbuild-daemon/src/handlers/emulator/shared.rs
  • crates/fbuild-daemon/src/handlers/operations/build.rs
  • crates/fbuild-daemon/src/handlers/operations/deploy.rs
  • crates/fbuild-daemon/src/handlers/operations/install_deps.rs
  • crates/fbuild-daemon/src/handlers/operations/monitor.rs
  • crates/fbuild-daemon/src/handlers/operations/reset.rs
  • crates/fbuild-daemon/src/handlers/websockets.rs
  • crates/fbuild-daemon/src/main.rs
  • crates/fbuild-daemon/tests/port_recovery.rs
  • crates/fbuild-daemon/tests/process_containment.rs
  • crates/fbuild-deploy/src/esp32/deployer.rs
  • crates/fbuild-packages/src/downloader.rs
  • crates/fbuild-packages/src/install_lock.rs
  • crates/fbuild-packages/tests/lnk_e2e.rs
  • crates/fbuild-python/src/async_serial_monitor.rs
  • crates/fbuild-python/src/serial_monitor.rs
  • crates/fbuild-serial/src/manager.rs
  • crates/fbuild-serial/src/preemption.rs

📝 Walkthrough

Walkthrough

Adds explicit timeout/deadline enforcement across the entire fbuild system: a default subprocess timeout policy in fbuild-core, job/step timeout-minutes and network flags in all GitHub Actions workflows, per-subprocess duration arguments to every build linker and tool invocation, hard deadline wrappers (tokio::time::timeout) in all daemon HTTP handlers, serial I/O spawn_blocking timeouts, package download chunk timeouts, CLI/Python client request caps, and wall-clock test guards in every integration test.

Changes

System-wide timeout and deadline hardening

Layer / File(s) Summary
Default subprocess timeout policy
crates/fbuild-core/src/subprocess.rs
Introduces DEFAULT_SUBPROCESS_TIMEOUT_SECS (900s), FBUILD_SUBPROCESS_DEFAULT_TIMEOUT_SECS env-var override, resolve_default_timeout/default_subprocess_timeout cached via OnceLock, and applies the policy to run_command, run_command_with_stdin, and run_command_passthrough so None resolves to the cap. Adds tests for policy resolution and timeout-firing behavior.
CI workflow job/step timeouts and network flags
.github/workflows/*, ci/*.py, ci/hooks/*.py
Adds timeout-minutes to every workflow job and selected steps; adds apt-get/pip/curl HTTP timeout and retry flags in template workflows; adds GIT_HTTP_LOW_SPEED_* env vars to update-data; adds timeout= arguments to subprocess.run/check_output calls in all ci/ Python scripts.
Build tool subprocess timeouts
crates/fbuild-build/src/*/...linker.rs, crates/fbuild-build/src/compiler.rs, crates/fbuild-build/src/linker.rs, crates/fbuild-build/src/symbol_analyzer/mod.rs, crates/fbuild-build/src/script_runtime.rs, crates/fbuild-build/src/shrink/probe.rs, crates/fbuild-build/src/pipeline/compile.rs, crates/fbuild-build/src/esp32/orchestrator/embed.rs
Passes explicit Some(Duration::from_secs(...)) to every run_command/run_command_with_stdin call across all platform linkers (AVR, CH32V, ESP32/8266, generic ARM, NRF52, Renesas, SAM, Silabs, STM32, Teensy), LinkerBase archive/size/nm/objcopy, compiler_version, zccache compile_source (tokio timeout), embed objcopy, shrink probe, symbol analyzer demangle/nm/objdump, and script/Python probing.
Daemon handler hard deadlines
crates/fbuild-daemon/src/handlers/operations/build.rs, deploy.rs, monitor.rs, reset.rs, install_deps.rs, crates/fbuild-daemon/src/handlers/emulator/select.rs, avr8js_headless.rs, shared.rs, crates/fbuild-daemon/src/handlers/websockets.rs
Introduces hard deadline constants and tokio::time::timeout wrappers for lock acquisition (503 on timeout) and build/deploy/monitor execution (mapped to FbuildError or HTTP error codes) in every daemon handler; adds child-reap timeouts after emulator kill; adds WebSocket attach handshake timeout (10s) and monitor idle timeout (300s).
Serial manager and ESP32 deploy timeouts
crates/fbuild-serial/src/manager.rs, crates/fbuild-serial/src/preemption.rs, crates/fbuild-deploy/src/esp32/deployer.rs
Moves write_to_port to spawn_blocking with 2s timeout; adds 3s to esp_hard_reset, 2s to close_port/rebind reader joins; switches PreemptionTracker from tokio::sync::Mutex to std::sync::Mutex; adds 30s/180s timeouts to ESP32 verify/write/region-flash blocking tasks.
Package download chunk timeout and install lock env override
crates/fbuild-packages/src/downloader.rs, crates/fbuild-packages/src/install_lock.rs, crates/fbuild-packages/tests/lnk_e2e.rs
Wraps each stream.chunk() in tokio::time::timeout (60s); adds FBUILD_INSTALL_LOCK_STALE_SECS env-var override to install_lock_stale_after with unit tests; adds LNK_TEST_TIMEOUT/with_timeout wrappers to all lnk e2e tests.
CLI and Python client timeouts
crates/fbuild-cli/src/cli/build.rs, clang_tools.rs, daemon_cmd.rs, pio.rs, crates/fbuild-cli/src/daemon_client.rs, crates/fbuild-python/src/async_serial_monitor.rs, serial_monitor.rs
Adds 10s to browser open, 120s tokio::time::timeout to run_iwyu/run_clang_tool, 10s to kill_process/find_daemon_pids, 5s to find_pio, 30min cap to DaemonClient::build, 10s to DaemonClient::shutdown, 5s HANDSHAKE_TIMEOUT to async and sync WebSocket connect+attach flows.
Daemon infrastructure: broker, device manager, async bind/GC
crates/fbuild-daemon/src/broker/session.rs, crates/fbuild-daemon/src/context.rs, crates/fbuild-daemon/src/main.rs
Adds BROKER_ADOPT_TIMEOUT/BROKER_REQUEST_TIMEOUT to broker session; changes DaemonContext.device_manager to Arc<DeviceManager> and moves refresh calls to spawn_blocking with 5s timeout; converts bind_listener_with_retry to async fn using tokio::time::sleep; moves disk cache GC to spawn_blocking.
install_status subscriber panic containment
crates/fbuild-core/src/install_status.rs
Wraps subscriber invocation in catch_unwind(AssertUnwindSafe(...)) in publish_install_status; expands docs; adds regression test confirming a panicking subscriber does not poison the registry lock.
Integration test wall-clock timeout wrappers
crates/fbuild-build/tests/*, crates/fbuild-cli/tests/*, crates/fbuild-daemon/tests/*, crates/fbuild-packages/tests/lnk_e2e.rs
Adds REAL_BUILD_TIMEOUT/under_test_timeout helpers to all ignored real-toolchain build tests (AVR, ESP32, eh_frame, compile_many, nxplpc, STM32, Teensy variants); adds run_cli_or_timeout/run_cli_with_timeout helpers to CLI integration tests; adds wait_with_timeout to daemon process tests; replaces blocking Python availability probe with a polled 5s deadline.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

  • #802 (audit: blocking operations with no timeout): This PR directly implements the fixes identified in the meta audit across all sub-areas (fbuild-build, fbuild-daemon, fbuild-serial, fbuild-deploy, fbuild-packages, fbuild-cli, fbuild-core, ci/).
  • #803 (audit: fbuild-serial): The write_to_port spawn_blocking migration, close_port/rebind reader join timeouts, and PreemptionTracker switch to std::sync::Mutex directly address the HIGH/MEDIUM findings in this audit.

Possibly related PRs

  • FastLED/fbuild#262: Both PRs modify the same ARM/NRF52/SAM linker run_command invocations and crates/fbuild-core/src/subprocess.rs; this PR adds execution timeouts on top of the env overlay added there.
  • FastLED/fbuild#604: Both PRs change fbuild-core/src/install_status.rs's subscriber/publishing mechanism (retrieved PR added the plumbing; this PR adds panic containment via catch_unwind).
  • FastLED/fbuild#750: Both PRs modify handle_serial_ws in crates/fbuild-daemon/src/handlers/websockets.rs; this PR adds the attach handshake timeout on top of the serial-WebSocket refactor from that PR.

Poem

🐇 Hoppity-hop, no more infinite wait,
Every subprocess now meets its fate!
Sixty seconds, three-hundred, or more—
The rabbit won't hang by an open door.
tokio::time::timeout seals the deal,
No wedged build shall the CI conceal! 🕐

✨ 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/timeout-audit-802

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

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

@zackees
zackees deleted the fix/timeout-audit-802 branch June 29, 2026 20:11
zackees added a commit that referenced this pull request Jun 29, 2026
Releases the post-#813 / #802 / #826 / #844 work landed across:
- PR #843 — workspace-wide timeout sweep (#802 family)
- PR #837, #849, #850, #851 — internal-bridges + dylint sweep (#826 + #844)
- PR #842 — async migration follow-ups (#813)

15 dylints now ship in dylints/ (4 from earlier, 10 from #826/#844,
plus the renamed ban_unwrap_in_production). 7 internal bridge APIs:
fbuild_core::{http, fs, time, channel, path::canonicalize_existing},
fbuild_paths::temp_subdir, fbuild_cli::output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

audit: blocking operations with no timeout in fbuild-serial (sub-issue of #802) audit: blocking operations with no timeout (meta)

1 participant