Skip to content

fix: keep accept loops alive when a connection thread cannot be spawned - #9

Closed
hojinyoo wants to merge 1 commit into
masterfrom
hojinyoo/api-accept-resilience
Closed

hojinyoo wants to merge 1 commit into
masterfrom
hojinyoo/api-accept-resilience

Conversation

@hojinyoo

@hojinyoo hojinyoo commented Sep 9, 2026 •

Copy link
Copy Markdown
Owner

Incident

The server stayed up but stopped answering herdr CLI/API calls. The API accept
loop spawns a handler thread per connection with std::thread::spawn, which
panics when the OS refuses a new thread (EAGAIN under thread/fd pressure). That
panic unwound the listener thread and dropped the listener, so the process kept
running with nothing behind the API socket, and only a restart brought it back.

Changed sites

Both accept loops now spawn through std::thread::Builder, log the failure and
keep accepting, so a refused thread costs one connection instead of the loop:

  • src/api/server.rs — the per-connection handler in the API accept loop.
    This is the site from the incident.
  • src/server/client_accept.rs — the per-client handshake in the thin-client
    accept loop. Same failure mode, worse blast radius: that loop runs on the
    headless server's main event loop, so the panic would take the whole server
    down along with every pane it owns.

Each site has a regression test that forces the spawn to fail through a
#[cfg(test)] hook and asserts the loop still accepts and serves afterwards.

Cherry-pick

Commit for build/v0.9.0: 3c641a5f30045cddd6f0875cf508645dc271b7dc

Branched from b99002ac (release: v0.9.0), so it applies cleanly.

Independent review

Codex was unavailable (usage limit), so the review pass ran on Grok. It found
that both regression tests would have stayed green if the production fix were
reverted
, and it was right on both counts:

  • The API test's empty read is also what a live handler produces after the 5s
    INITIAL_REQUEST_TIMEOUT, so the spawn hook was never observed. The dropped
    connection now sends a real request and the test asserts it never reaches the
    app channel.
  • next_client_id increments before the spawn, so == 3 held either way. The
    handshake test now probes local_stream_peer_closed, proving the accepted
    stream was dropped rather than handed to a handshake thread.

Both fixed, and verified by mutation: reverting either call site to
std::thread::spawn now fails both tests in ~0.02s.

Not addressed, by design: no test exercises Builder::spawn's real Err path.
Forcing a genuine EAGAIN needs a process-wide setrlimit that would break the
parallel test process, so the hook stands in for it.

Validation

Run on the repo-pinned 1.96.1 toolchain (the Homebrew 1.98 on this machine
shadows it and reports unrelated new lints):

  • cargo fmt --check — clean
  • cargo clippy --all-targets --locked -- -D warnings — clean
  • cargo clippy --bin herdr --locked --target x86_64-pc-windows-msvc -- -D warnings — clean
  • cargo nextest run --locked --no-fail-fast — 3105 tests: 3103 passed, 2 failed, 2 skipped
  • just maintenance-test (101), just ui-hot-path-architecture-test (6),
    just integration-assets-test (5), just plugin-marketplace-test (31),
    just docs-contract-test (7) — all pass

The two failures are live_handoff_keeps_unmanaged_agent_name_bound_to_saved_session
and live_handoff_preserves_pane_process_io. Both fail identically on a detached
b99002ac without this commit, so they are pre-existing local failures, not a
regression from this change.

https://claude.ai/code/session_018yr6GvM4fkHH4HVEzwLhHM

@hojinyoo hojinyoo changed the title api: keep the accept loop alive when a thread cannot be spawned fix: keep accept loops alive when a connection thread cannot be spawned Sep 9, 2026
@hojinyoo hojinyoo closed this Sep 9, 2026
@hojinyoo hojinyoo reopened this Sep 9, 2026
std::thread::spawn panics when the OS refuses a new thread, and the panic
unwound the accept loop that raised it, leaving the process running with no
listener behind its socket. Spawn through thread::Builder at both accept
sites, log the failure, drop that one connection and keep accepting.

Claude-Session: https://claude.ai/code/session_018yr6GvM4fkHH4HVEzwLhHM
@hojinyoo
hojinyoo force-pushed the hojinyoo/api-accept-resilience branch from 9cd0ae1 to 3c641a5 Compare September 9, 2026 13:35
@hojinyoo

Copy link
Copy Markdown
Owner Author

Superseded upstream: v0.9.4 herdrdev#4958 guards thread spawns in both the API accept loop and src/server/client_accept.rs.

@hojinyoo hojinyoo closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant