fix(cli): exit cleanly when stdin is an open pipe that never closes#1294
Open
artogahr wants to merge 2 commits into
Open
fix(cli): exit cleanly when stdin is an open pipe that never closes#1294artogahr wants to merge 2 commits into
artogahr wants to merge 2 commits into
Conversation
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.
Hi folks, this is something that was bothering me for a while, so I thought I would go ahead and make a PR for it.
What's wrong
When stdin is connected but never closed, which is what you get from anything spawning the CLI with
stdio: ['pipe', ...](e.g. claude code), every command finishes its work, prints all of its output, and then never exits. Reported in #1206, and it's the visible symptom in #1187 as well.Why it happens
src/entrypoints/_shared.tsawaitsreadStdin()before running any command. For socket-type stdin there's a 50ms timeout so the CLI doesn't wait forever, but the abort path returned with the'data'listener still attached and the stream still flowing. That single open handle keeps the event loop from draining. There's noprocess.exit()on the normal path, the process relies on the loop emptying out, so it hangs after the command is done. A stack sample of a hung process shows the main thread idle inuv__io_poll.The fix
readStdinnow removes its listener and pauses the stream in afinallyblock.pause()releases the loop for every stdin type Node hands out, and it's the safest of the options:fs.ReadStreamstdin (from< file) has nounref(), and destroying fd 0 would breakset-value, which streamsprocess.stdinas the upload body.A nice side effect: the leftover flowing listener used to eat any stdin bytes that arrived after the 50ms abort, which could silently truncate
set-valueuploads. Those bytes now stay buffered in the paused stream and upload in full.Regression test
test/e2e/commands/stdin-held-open.test.tsspawns the built CLI with stdin held open and checks that it exits on its own. It passes on this branch in about half a second, and fails with a timeout if you strip the fix back out of the bundle. It needsdist/built, so it lives in the e2e suite and runs withpnpm run test:e2e, nottest:local.What this doesn't fix
These all predate the change and behave the same before and after:
tail -f /dev/null | apify ...) still hangs at startup, since no timeout is armed for FIFOs.'end'never comes.datasets get-itemsfinishes but never exits — leaked keep-alive sockets pin the process (1.6.1 & 1.6.2) #1187 can pin the process on their own.The first two need the lazy stdin reading discussed in #1206.
I've verified this change separately with Fable and Kimi K3.