Skip to content

fix(daemon): survive thread spawn failures - #29

Closed
jlongster wants to merge 1 commit into
subscription-pollfrom
handler-spawn
Closed

jlongster wants to merge 1 commit into
subscription-pollfrom
handler-spawn

Conversation

@jlongster

Copy link
Copy Markdown
Collaborator

Summary

  • Part 2 of the stack for Idle daemon wakes the CPU ~100 times/s (10 ms sleep in the accept loop) #25, stacked on fix(daemon): block subscriptions instead of polling #28. Please merge fix(daemon): block subscriptions instead of polling #28 first.
  • thread::spawn panics when the OS can't create a thread. In the daemon that panic skipped orderly shutdown and cleanup, or failed in confusing ways. I found this while planning the readiness-driven loop. All daemon threads now start through thread::Builder, are named opencode-pty-*, and handle spawn failure explicitly:
    • Connection handler: if the spawn fails, that connection is dropped (the peer sees EOF) and the daemon keeps serving.
    • Shutdown watchdog: if the spawn fails, the daemon logs it and continues orderly shutdown without the backstop.
    • Subscription disconnect monitor: if the spawn fails, the existing io error is returned.
    • Stale-runtime sweeper: optional. If the spawn fails, the daemon logs it and continues.
  • Terminal startup in service.rs now cleans up fully after any partial failure. It kills and reaps the child, and joins any threads that already started through the existing TerminalHandle::shutdown path. The child is handed to the waiter only once the waiter is running.
  • Threads now start in the order reader → waiter → actor → writer. If actor init fails on Windows, the pseudoconsole closes through the normal close path while the reader drains it. This fixes a pre-existing case where ClosePseudoConsole could hang on older Windows.
  • Known remaining edge: if the reader thread itself fails to spawn right after the child starts, ClosePseudoConsole can still block on older Windows when the child has already written more than the pipe buffer holds. Fixing that means starting the reader before spawning the child, which is deliberately out of scope here.

Validation

  • No tests added: there's no way to force thread creation to fail without process-wide limits or mocks.
  • cargo fmt --check and cargo clippy --locked --all-targets --all-features -- -D warnings pass.
  • The macOS CI test selection passes: cargo test --locked --lib --test service --test runtime --test ownership --test rows --test ghostty-effects (lib 40, ghostty-effects 5, ownership 8, rows 1, runtime 8 + 1 ignored, service 9).
  • The Windows code was not compiled locally; this relies on Windows CI.

@jlongster

Copy link
Copy Markdown
Collaborator Author

This moved to anomalyco/opencode#59699, which was closed without merging. We decided to accept a thread::spawn panic when thread creation fails.

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