process.stdin: leave the ref of the paused stdin source for its next reader
Stacked on #46528. It needs the resume that #46528 adds when a reader attaches.
Problem
- After
process.stdin.pause()orrl.close(),for await (const line of console)reads nothing and the process exits with code 0. ABun.stdin.stream()reader andReadable.fromWeb(Bun.stdin.stream())do the same. - #46528 fixes these only under a top-level await.
- The cause is the last line of
disown()(src/js/builtins/ProcessObjectInternals.ts:154),source?.updateRef?.(false). The next reader resumes the source, but its poll keeps nothing alive.
Fix
disown()ends insource?.updateRef?.(!forceUnref). That is the whole source change.- Correct because a paused source has no armed poll. The ref keeps nothing alive until a reader resumes the source, and that reader's release drops it.
- Verified: 32 new tests in
test/js/node/process/process-stdin.test.ts. 27 fail without the line. The other 5 check that the process still exits. - Self-reviewed: 8 concerns raised, 7 addressed (Notes).
Background
process.stdinreads through a reader onBun.stdin.stream().disown()runs when it lets go: one tick after'pause', and on'close'.setFlowing(false)unregisters the poll of the native source on fd 0.updateRef()sets whether that poll keeps the process alive when armed.forceUnrefis true afterprocess.stdin.unref().- Considered a ref where #46528 resumes the source. It changes every file-backed stream (#41578).
Downsides
- A reader taken after the pause and never read keeps the process alive until stdin closes, as an idle reader does when nothing paused.
- Still broken: a reader that holds the lock when
pause()runs reads nothing. - No new call, allocation or syscall. The builtin text grows by 9 bytes.
Notes
What is left of this PR. Until b3356db46a this PR had three parts: a resume when a reader locks the stream (C++), a started check in FileReader::set_flowing (Rust), and two lines in disown(). #46528 has the first two since 44e485859c, as resumeNativeSource() in setUpReadableStreamDefaultReader and if !self.started.get() || ... in set_flowing. They are gone from here, and so is the segfault of pause() followed by Readable.fromWeb(Bun.stdin.stream()), which #46528 fixes. One line of disown() is left.
Measured (Linux x64, debug builds of 04af03e149 with and without the line). The child runs the first column and then reads. The parent writes the input 500 ms after the child prints a marker, and then closes the pipe.
| The child runs | main 7d240c1dd5 (release) | #46528 | #46528 with the line |
|---|---|---|---|
pause(), one setImmediate, the console iterator | nothing, exit 0 | nothing, exit 0 | reads all |
the same with a getReader() loop, for await, pipeTo() or .text() | nothing, exit 0 | nothing, exit 0 | reads all |
the same with Readable.fromWeb(Bun.stdin.stream()) | SIGSEGV | nothing, exit 0 | reads all |
rl.close(), resume() then pause(), or a 'data' listener that pauses, then a reader | nothing more, exit 0 | nothing more, exit 0 | reads all |
top-level await, pause(), a reader that waits 1 s: CPU time | never reads | 979 to 1004 ms | 10 to 11 ms |
The last row: with nothing ref'd, a pending top-level await spins in EventLoop::wait_for_promise until the input arrives. await new Promise(() => {}) alone does the same on 1.3.13 (#14951), so that part is older and not changed here.
History: the pause() rows read everything on 1.3.13 and nothing on 1.4.2 (SIGSEGV for Readable.fromWeb). Since #34971 (1.4.0) a reader remembers an unref that comes before its first poll registration, and disown() unrefs a source that never started. The rl.close() rows read nothing on 1.3.13 too.
The tests. One describe block, four ways for process.stdin to let go (pause(), rl.close(), destroy(), a break out of for await (const chunk of process.stdin)) and three readers (the console iterator, a Bun.stdin.stream() reader, Readable.fromWeb). No child has a top-level await.
- 12 rows: the reader reads the rest of stdin.
- 12 rows: the reader reads one chunk and lets go. The parent leaves stdin open, and the child has to exit by itself.
- 4 rows: nothing reads stdin again, stdin stays open, the child has to exit.
- 1 row each: the same over a shell pipe (
bun | bun),process.stdintakes stdin back after the reader, a readline prompt on a pty followed by the console iterator, andprocess.stdin.unref()before the pause. The writer of the pipe is notcat: busyboxcatcopies withsendfile(), and asendfile()that waits for input keeps the pipe locked, so the reader blocks inread().
The child prints its marker from an unref'd 50 ms timer. That timer fires only while stdin keeps the process alive, so without the line the marker never comes and the parent never writes. On debug builds of 04af03e149, 26 rows failed in 5 of 5 runs without the line and all rows passed in 5 of 5 runs with it. The shell pipe row failed in 3 of 3 runs without the line and passed in 3 of 3 with it (release builds). The 5 rows that pass both ways are the 4 exit rows and the unref() row. With updateRef(true) in place of the line, the unref() row fails.
Self-review. Concerns and what happened to each:
- The resume at lock time can run user JS (
getReader()in abeforeExitlistener runs a queuedprocess.nextTickcallback before it returns). This PR no longer has a resume. The same holds forresumeNativeSourcein #46528, reported there. - The lock check made
pause()a no-op under every lock holder. Removed, see below. - The rows raced the exit of the child. Now the marker comes from the unref'd timer.
- No row could fail on a leaked keep-alive. Now 17 rows leave stdin open.
- The
forceUnrefhalf had no test. Now theunref()row. - The
native.lockedhalf had no test. Removed with the lock check. - The
'close'path had no test. Now two of the four doors. - Only socketpair and pty stdin. Added the shell pipe row. Not added: rows with stdin from a file. They pass without the line, because a regular file has no poll.
The lock check left this PR. if (!reader && native.locked) return; made process.stdin leave a source alone while another reader holds the lock. It is a new ownership rule with a cost: after pause() under a lock holder, a child with stdin: "inherit" got nothing in 5 of 10 runs (0 of 10 without the check), because the holder still reads fd 0. That is the contract of #13978 and #23333. It needs its own decision. #43951 points at the same case.
Not changed.
- A reader that holds the lock when
pause()ordestroy()runs reads nothing more (Readable.fromWebin the same tick aspause()too). Under a top-level await it hangs. process.stdin.unref()still applies to the shared source, so the next reader does not keep the process alive.- While another reader holds the lock,
process.stdin.resume()throwsReadableStream is locked(#45068, #7541). - A second reader after another reader's release (not after
process.stdin) still exits early (#41578). pause()still frees fd 0 for a child:test/js/node/readline/stdin-pause-pty.test.tspasses on release builds with and without the line.
Suites. Debug build with the line, 0 failures: process-stdin.test.ts (63), console-iterator.test.ts (17), readline.node.test.ts (81), readline_promises.node.test.ts, pause_stdin_should_exit, readline_never_unrefs, stdin_fell_asleep, process-stdio.test.ts, process-stdin-stale-hup, terminal.test.ts (101), terminal-spawn.test.ts, test/js/node/tty, spawn-stdin-readable-stream.test.ts (54), spawn-socketpair-shutdown, spawn-streaming-stdin, node-stream.test.js (162), and the 33 Node test-stdin-*, test-tty-stdin-*, test-readline-* files. On this machine stdin-fixtures.test.ts (a 1 s kill timer) and stdin-pause-pty.test.ts fail on debug builds with and without the line, and pass 3 of 3 on release builds both ways. Nothing ran on macOS or Windows here.
no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/process/process-stdin.test.ts