fix(core): resolve spawn on exit not close to avoid bg-child hang#29108
Open
divitkashyap wants to merge 1 commit into
Open
fix(core): resolve spawn on exit not close to avoid bg-child hang#29108divitkashyap wants to merge 1 commit into
divitkashyap wants to merge 1 commit into
Conversation
The cross-spawn spawner only resolved its exit Deferred on the Node `close` event, which waits for all stdio descriptors to drain. When a spawned command forks a background grandchild that inherits those descriptors (e.g. `cmd &`, daemonized servers, gradle daemon, MCP stdio servers), `close` never fires and the shell tool hangs until the configured timeout (~2 minutes). Settle the exit Deferred on whichever of `exit` / `close` fires first. `exit` fires when the direct child process exits regardless of inherited-fd lifetime, which is what callers actually care about. Closes anomalyco#20902
Author
|
@thdxr would appreciate a look when you have a moment — small, contained fix with a regression test. Happy to iterate on the approach if you'd prefer something different. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #20902
Type of change
What does this PR do?
The cross-spawn spawner (
packages/core/src/cross-spawn-spawner.ts) only settled its exit Deferred on the Nodecloseevent.closewaits for all stdio descriptors to drain — so if the spawned command forks a background grandchild that inherits stdout/stderr (e.g.cmd &, daemonized servers, the Gradle daemon, long-lived stdio MCP servers),closenever fires and the shell tool hangs until the configured timeout (~2 minutes).The fix is to settle on whichever of
exitorclosefires first.exitfires when the direct child exits regardless of inherited-fd lifetime — which is what callers ofhandle.exitCodeactually want.closeis kept as a fallback in caseexitnever fires for some reason. The exit-code/signal payload comes from whichever event ran first, which matches what Node passes to both.How did you verify your code works?
packages/opencode/test/tool/shell.test.tsthat runssleep 30 & disown; echo backgroundedthrough the shell tool and asserts it returns in well under the timeout. Without the fix it hangs the full 15s test timeout; with the fix it returns in ~1s.packages/opencode/test/tool/shell.test.tssuite (24 tests pass) andpackages/core/test/effect/cross-spawn-spawner.test.ts(24 tests pass) locally.Checklist