fix(core): resolve spawn completion on exit, not only close (Windows detached-child hang)#29831
Open
Hona wants to merge 7 commits into
Open
fix(core): resolve spawn completion on exit, not only close (Windows detached-child hang)#29831Hona wants to merge 7 commits into
Hona wants to merge 7 commits into
Conversation
The shell tool hangs on Windows when a command spawns a detached child that inherits stdio (e.g. a backgrounded web server or playwright-cli). The cross-spawn spawner resolved process completion only on the child's `close` event, which fires after *all* stdio streams close. A detached child keeps the inherited pipe's write end open, so `close` is delayed indefinitely even though the main process already exited -- and exitCode (and thus the shell tool) never resolves. Resolve on whichever of `exit`/`close` fires first. `exit` returns promptly once the main process is gone (fixing the hang); `close` is kept as the required backstop for failed spawns, where cross-spawn emits spawn->error->close but never exit, so exit-only would hang that path forever. Closes anomalyco#24731
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Windows hang in the shell tool when a command spawns a detached child that inherits stdio (e.g. playwright-cli). The CrossSpawnSpawner previously completed only on the child's "close" event, which can be delayed indefinitely if a detached child keeps the parent's stdio pipes open. The fix completes the exit Deferred on whichever of "exit" or "close" fires first, while preserving the "close" listener as a backstop for the ENOENT path where cross-spawn never emits "exit".
Changes:
- Resolve the spawn completion Deferred in the
"exit"handler in addition to the existing"close"handler, withend/exitguards so the value is consistent regardless of order. - Added an explanatory comment documenting why both events are needed.
- Added a regression test (using
fx.live) that spawns a detached daemon inheriting stdio and assertsexitCoderesolves within 5s, reaping the daemon afterward.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/cross-spawn-spawner.ts | Complete on first of exit/close; keep close as ENOENT backstop. |
| packages/core/test/effect/cross-spawn-spawner.test.ts | New regression test for detached child stdio inheritance (#24731); trailing blank lines added. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 29, 2026
…ed pipes Replace the fixed 100ms post-exit drain (which truncated output when a slow per-chunk consumer was still working through a backlog) with joining the output reader fiber. For a normal command this returns the instant the stdio streams reach EOF -- full output, no fixed delay, matching upstream's backpressure-synced behavior. A detached child can hold the pipe open forever with no further output, so an idle watcher that resets on every chunk bounds only that case; a command still producing output is never cut off.
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.
Summary
Fix shell commands hanging after their main process exits while a child still holds stdout/stderr open.
Fix
exitedseparately from stream/groupclosed.exitCode/isRunningresolve onexited; kill escalation (forceKillAfter) waits onclosedso resistant descendants are still SIGKILLed.Why no temp file / no fixed drain
closeonly fires once every holder of the stdio write-end closes; a detached grandchild that inherited the pipe keeps it open, soclosenever comes. We cannot control the grandchild's stdio, so completion must key off processexit. "Exited" does not mean "fully read" -- a small produced-but-unconsumed backlog can remain, which is why we join the reader. A fixed-duration drain truncates that backlog for slow consumers; the idle-reset watcher does not.Tests
exitCode(core spawner).Closes #24731
Closes #24784
Closes #20902
Closes #29822
Closes #25038
Closes #28697
Closes #25938
Refs #28654
Refs #22012