Skip to content

audit: blocking operations with no timeout in tests + benches (sub-issue of #802) #806

Description

@zackees

Sub-issue of #802 covering the test + bench surface area only. Test hangs cost CI minutes (and burn the runner job's 6 h default cap), not user-perceptible time, so the priority of these findings is lower than the daemon/build crates — but a wedged --ignored daemon test or a hung network download on an acceptance run can still hold a CI job hostage for hours. Most of the integration tests already use #[ignore] gating or local mock servers, so the blast radius is contained.

Findings table

SeverityFile:lineOperationWhy unboundedSuggested fix
HIGHcrates/fbuild-daemon/tests/port_recovery.rs:70,86d1.wait() / d2.wait() on the real fbuild-daemon childIf the daemon spawns but the signal/taskkill doesn't reach it, wait() blocks forever. Test is #[ignore], but CI runs --ignored jobs.Use child.wait_timeout(Duration::from_secs(30)) via wait-timeout crate, then kill() + assert.
HIGHcrates/fbuild-daemon/tests/process_containment.rs:93parent.wait() on the containment harness parent after taskkill /FIf taskkill fails silently on Windows (e.g. permissions, AV interference), wait() blocks. The polling loop below has a 10 s deadline but wait() runs first.Same fix — bounded wait, then force-kill the handle.
HIGHcrates/fbuild-cli/tests/test_emu_exit_code.rs:141-158Command::new(bin).output() invoking the compiled fbuild CLICLI is wired at a mock daemon over loopback; if the mock thread panics before responding, the CLI's reqwest call has no top-level timeout in this path (CLI relies on its own internal timeouts; if those regress, the test wedges).Wrap in wait_timeout(Duration::from_secs(30)) or assert via a tokio::time::timeout around the child process.
HIGHcrates/fbuild-cli/tests/ci_command.rs:14-67 (3 tests)Command::new(bin).output() on fbuild ci --help / argument-validation pathsEach is a one-shot CLI invocation. If the binary hangs (regression in clap parsing → infinite loop), output() blocks the test runner indefinitely. Low practical risk, but trivial to add a cap.wait_timeout(Duration::from_secs(10)) per spawn, or move to a shared run_cli_or_timeout helper.
HIGHcrates/fbuild-cli/tests/lib_select.rs:16-77 (3 tests)Same as ci_command.rs — three more help/usage spawns via .output()SameSame — shared helper would cover both files.
HIGHcrates/fbuild-build/tests/lite_scons_acceptance.rs:43-46Command::new(python).args(["--version"]).output() probeIf a wedged Python interpreter binds the test (e.g. an alias to a hung shim), output() blocks. Tests then re-run lite-SCons with no enforced wall-clock either.Cap the probe with wait_timeout(Duration::from_secs(5)); treat timeout as "python unavailable" and skip.
HIGHcrates/fbuild-packages/tests/lnk_e2e.rs:69-237 (4 #[tokio::test] cases)materialize_alldownload_file against an in-process axum server, with no enclosing tokio::time::timeoutServer is in-process so the network path is loopback only. But if the resolver wedges (e.g. mismatched future cancellation on server_handle.abort()), the test hangs. The 404 + sha-mismatch cases don't even close the server until the assertion passes.Wrap each test body in tokio::time::timeout(Duration::from_secs(15), async { ... }).await.expect("test timeout").
MEDIUMcrates/fbuild-build/tests/avr_build.rs, esp32_build.rs, teensy_build.rs, teensy30_acceptance.rs, teensy41_acceptance.rs, teensylc_acceptance.rs, stm32_acceptance.rs, nxplpc_build_flags.rs, eh_frame_strip_esp32.rs, compile_many_stage2_perf.rsReal toolchain + framework downloads from GitHub / downloads.arduino.cc, then a full buildAll #[ignore], run only on --ignored CI jobs. Downloads currently have no top-level wall-clock; a stuck TCP read or a slow GitHub mirror can wedge the job for the full job-timeout (6 h default).Add per-test #[timeout] via ntest::timeout or wrap the body in std::thread::spawn with a join_timeout. 10–15 min cap is plenty for the slowest of these.
MEDIUMcrates/fbuild-build/tests/zccache_hit_across_workspace_rename.rs:267Command::new(rustc).status() to compile a throwaway fake_zccache helperrustc misbehaving (e.g. a crashed proc-macro server) hangs the test.wait_timeout(Duration::from_secs(120)) is plenty for compiling a 100-line single-file program.
MEDIUMcrates/fbuild-build/tests/compile_many_two_stage.rs:135barrier.wait() inside the MockBuilderThis is a std::sync::Barrier sized to the worker count. If compile_many runs workers serially (the bug the test is designed to catch), the barrier deadlocks. The test already protects against this via the polling deadline at line 277-291 (10 s) — so this one is intentional and not a finding. Listed for transparency.No fix needed; the surrounding deadline catches the deadlock and fails fast.
LOWcrates/fbuild-daemon/tests/build_streaming.rs:75-82reqwest::Client::new().post(...).timeout(Duration::from_secs(10)).send()Already has a per-request timeout. The only remaining risk is the resp.text().await after .send() returns — chunk streaming has no separate cap. In practice the body is short and bounded by the server's handler completion.Optional: wrap entire async block in tokio::time::timeout. Low priority.
LOWcrates/fbuild-daemon/tests/test_emu_endpoint.rs:75-81Same .timeout(Duration::from_secs(5)) pattern as build_streaming.rsAlready capped at the HTTP layer.No change needed.

What was searched

Patterns (via Grep):

  • Command::new, \.output\(\), \.wait\(\), \.wait_with_output\(\), \.spawn\(\)
  • JoinHandle, \.join\(\)
  • \.recv\(\), recv_timeout
  • loop \{, while !
  • std::thread::sleep, tokio::time::sleep
  • connect_timeout, set_read_timeout, set_write_timeout, tokio::time::timeout
  • Duration::from_secs, Duration::from_millis
  • reqwest, download, fetch, http

Directories (via Glob):

  • crates/*/tests/*.rs — 28 files across 5 crates (fbuild-build, fbuild-cli, fbuild-daemon, fbuild-library-select, fbuild-packages)
  • crates/fbuild-test-support/src/**/*.rs — 6 files; no blocking ops found (pure fixture/setup helpers)
  • bench/fastled-examples/src/main.rs — 1 file; no blocking ops found (pure in-process resolver work, uses Instant::now() for measurement only)

Out-of-scope notes

  • compile_many_two_stage.rs:135 barrier: deliberately designed to deadlock if the production code regresses to serial execution — the test's 10 s polling deadline catches it. Documented in the table for transparency but not an actionable finding.
  • #[ignore]-gated acceptance tests (avr_build, esp32_build, the teensy* family, etc.) only run in dedicated CI jobs. They're MEDIUM severity rather than HIGH because the default test run does not touch them — but --ignored CI jobs do, and those have no per-test wall-clock cap today.
  • bench/fastled-examples uses Instant::now() only for timing measurements, and its --max-warm-ms flag is a post-hoc threshold check, not a real-time cap. The bench itself does not spawn long-lived processes — it's safe.
  • fbuild-test-support is pure fixture code (MiniFramework, create_test_project, board/elf helpers). No process spawns, no I/O timeouts to add.
  • Tests using reqwest with .timeout(...) already capped at the HTTP layer (test_emu_endpoint.rs, build_streaming.rs) — LOW priority refinements only.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions