fix: prevent pkill hang when close event never firesFix/pkill hang#25672
fix: prevent pkill hang when close event never firesFix/pkill hang#25672zenoda wants to merge 40 commits into
Conversation
…r fires The orElse path in Effect.timeoutOrElse (used for forceKillAfter escalation from SIGTERM to SIGKILL) was awaiting the exit Deferred after sending SIGKILL. If the Node.js close event never fires (e.g., orphaned children from pkill holding pipe FDs), this Deferred.await hangs indefinitely with no timeout protection — even after SIGKILL reaches the process. Remove the Deferred.await from the orElse path since kill(SIGKILL) is synchronous on Linux — the kernel has already freed the PID and closed FDs before the syscall returns, making the wait redundant.
… hang The exit-signal Deferred was resolved only on the Node.js 'close' event, which requires all pipe file descriptors to be closed. When pkill -f pattern matches the shell process itself (e.g., pkill -f vim where the shell command line contains 'vim'), the shell is killed but orphaned children (from shell init files or the pkill process itself) may still hold pipe FDs, preventing 'close' from ever firing. Resolve the Deferred on 'exit' instead — the exit event fires as soon as the process terminates regardless of pipe state. The 'close' handler is kept as a safety fallback. Output consumption via handle.all is independent and runs in a separate fiber, so resolving earlier does not cause data loss.
github-actions
Bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: I found a potentially related PR: PR #24783: "fix: add exit event fallback for child process close hang on Windows" This appears to be related because it also addresses similar issues with child process lifecycle events (exit vs close events). However, PR #24783 specifically targets Windows, while the current PR #25672 focuses on the The two PRs may be addressing complementary aspects of the same underlying problem with process event handling, so it's worth checking if they have overlapping changes or if one supersedes the other. |
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
7 participants
Issue for this PR
Closes #25664
Type of change
What does this PR do?
Summary
exitevent instead ofcloseto prevent hang whenpkill -forphans children holding pipe FDsDeferred.awaitfrom SIGKILL escalation path sincekill(SIGKILL)is synchronous on LinuxEffect.catchtoexitCodein bash tool to preventEffect.raceAllhang on ErrorChanges
packages/core/src/cross-spawn-spawner.ts— two commits fixing Deferred resolution and SIGKILL escalationpackages/opencode/src/tool/bash.ts— catch exitCode Error in raceHow did you verify your code works?
Observe that the tool call dose not hangs any more.
Checklist