fix: destroy piped streams on child exit to prevent grandchild deadlock#137
Merged
Merged
Conversation
When a child process spawns a grandchild that inherits the piped stdout fd (fd 1), the grandchild keeps the pipe open after the child exits. Without the fix, both `await exec()` and the async iterator hang indefinitely because the piped streams never end. Each test runs in a subprocess (spawnSync with a 10s timeout) so the orphaned grandchild doesn't block vitest. A 5s inner race detects whether exec completes or hangs.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an exit-based cleanup to prevent hangs when a grandchild process keeps inherited pipe FDs open, and introduces regression tests that reproduce the scenario in a subprocess.
Changes:
- Add an
exithandler inExecProcessto destroy piped stdout/stderr streams on child exit. - Add Vitest coverage for “grandchild holds stdout open” for both
awaitand async-iterator usage patterns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main.ts | Destroys piped streams on exit to avoid waiting forever for close when grandchildren keep FDs open. |
| src/test/grandchild_test.ts | Adds regression tests that spawn a child which spawns an orphaned grandchild inheriting stdout, ensuring x() completes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mearman
force-pushed
the
fix/grandchild-pipe-deadlock
branch
from
97115db to
df68f2d
Compare
When a child process spawns a grandchild that inherits the piped stdout/stderr file descriptors, the grandchild holds them open after the child exits. Node's close event waits for all fds to be released, so it never fires. readStream and combineStreams hang indefinitely because the streams never end. Listen for the exit event (fires when the process exits, regardless of pipe state) and destroy the piped streams. This forces the PassThrough and readline consumers to complete. Refs: lint-staged/lint-staged#1800
Mearman
force-pushed
the
fix/grandchild-pipe-deadlock
branch
from
df68f2d to
9657bb5
Compare
iiroj
reviewed
May 27, 2026
iiroj
reviewed
May 27, 2026
iiroj
reviewed
May 27, 2026
Static test fixtures that spawn a long-lived grandchild process inheriting the piped stdout fd, simulating tsserver inheriting eslint's piped streams through lint-staged/tinyexec.
Use committed fixture scripts instead of generated inline scripts. Run tests in a subprocess with isolated stdio so orphaned grandchildren don't block vitest's teardown. Clean up grandchildren via pkill with a marker comment in the fixture scripts.
setImmediate is sufficient to let buffered data drain before destroying the streams. Removes the timer tracking in _resetState and _onClose that was needed for the setTimeout approach.
Member
|
the test doesn't really belong in its own file as its still a ill move it myself though as this otherwise looks good 👍 |
Contributor
Member
|
it'll be in v1.2.3 👍 most likely today |
Contributor
|
@43081j nice, I see it's already published as staged, but not public on npmjs.com yet: https://github.com/tinylibs/tinyexec/actions/runs/26569657183/job/78273310463 |
Member
|
yup just hadn't got around to it yet 👍 don't worry, i didn't forget 😄 |
This was referenced May 30, 2026
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.
Fixes #138
When a child process spawns a grandchild that inherits the piped stdout/stderr fds, the grandchild holds them open after the child exits. Node's
closeevent waits for all fds to be released, so it never fires. Bothawait exec()and the async iterator hang.This is what causes lint-staged to deadlock when eslint uses typescript-eslint's
projectService(tsserver inherits the piped fds). See lint-staged/lint-staged#1800.The fix listens for
exitand schedules stream destruction after a 100ms grace period. Ifclosefires normally (no grandchild), the timer is cancelled. This avoids truncating buffered output while still breaking the deadlock when grandchildren hold the pipes.Two tests in
src/test/grandchild_test.ts, both running in a subprocess with a 10s timeout so orphaned grandchildren don't block vitest:await x()completes and resolvesfor await (const line of x())yields the output lines and completesWithout the fix the subprocess hangs and gets killed by the timeout.