Repository navigation
Conversation
|
Updated 12:23 AM PT - Oct 26th, 2026
❌ @robobun, your commit dfbce35 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 46954That installs a local version of the PR into your bun-46954 --bun |
|
Status How I reproduced it, on 9cd5284 (the head of #46528), Linux x64:
CI on dfbce35 (build 132961, finished): 181 of 182 jobs passed. Both test files pass on Linux, Windows and macOS (aarch64 and x64). The one red job ran on the macOS agent |
A reader release only dropped the event-loop ref of the native file source. The source of Bun.stdin.stream() kept its poll on fd 0 and took input that arrived while no reader was attached: one parked pull, then one more chunk. A dialog, fs.readSync(0) or a child process with inherited stdin did not get that input. After a break out of `for await (const line of console)` on a terminal, a child that reads stdin got 1 to 5 of 8 typed lines. The release hook now calls Bun__FileReader__onReaderRelease. A source that reads stdin, stdout or stderr as a pipe, socket or terminal stops, and the next reader lock resumes it through resumeNativeSource. A stopped source keeps nothing alive, so it keeps its ref for that next reader: a second console loop in an async function now waits for its input. Every other source reads on and drops its ref as before: a regular file, a path, another descriptor, a Bun.spawn pipe, and on Windows a console. A consumer that stops the source inside the delivery of a chunk ends the read loop. The loop read once more after a chunk of 131,072 bytes or more. set_flowing(true) reads only when the buffer of the source is below the highwater mark. Without that limit each release and lock adds one chunk to the buffer. The console iterator keeps the unfinished line at the end of the chunk that it stopped in. The next loop lost that start of a line. The rule "a reader release stops the native source, a reader lock resumes it" is from d227cc7 in #23341 and from 1d2ebbb in #43747. Both had no scope. The first stopped the file readers of Windows and the stdout of a child. The second leaked the descriptor of an abandoned Bun.file().stream() reader.
64fba78 to
dfbce35
Compare
|
Order note: #48209 deletes the resume branch of Nothing changes here until #48209 is on main. After that, this PR can drop that line when it is cut again on main. Its tests for the split line stay valid. |
Draft until #46528 merges: it needs
resumeNativeSourceof that branch. Then I cut it again on main.Problem
reader.releaseLock()or abreakout offor await (const line of console), the stdin stream still reads fd 0: 131,072 bytes of a pipe, up to 200,000 of 200,000 on a socket. A child that inherits a terminal gets 1 to 5 of 8 typed lines.releaseNativeSourceRef(ReadableStreamOperations.cpp:511). It only callsupdateRef(false). Fix: after pausing stdin, a subprocess should be able to read from stdin #23341 fixedprocess.stdin.pause()only ([Node Incompatibility]pausecontinues reading data on a tty #23333).Fix
Bun__FileReader__onReaderRelease. A stdio source on a pipe, socket or terminal stops and keeps its ref. The next reader lock resumes it.process-stdin.test.ts,console-iterator.test.ts: 51 new tests, 41 fail on the base.Background
FileReadersource polls the descriptor and reads ahead. Its ref keeps the process alive.process.stdinand the console iterator share one stream.break,releaseLock()andpreventCancelrelease a reader with no cancel.Downsides
Notes
How this was found. No user reported these forms. A check of what
prompt()andconfirm()read after a stream reader of stdin found the defect, and #41578 already lists it as a known gap. The reports that exist are for theprocess.stdinform (#23333, #13978), which #23341 fixed. That PR also tried this rule: d227cc7 setflowingingetReader()and cleared it inreleaseLock(), for every file reader. It was taken out because it stopped the file readers of Windows and the stdout of a child. 1d2ebbb in #43747 tried it again and leaked the descriptor of an abandonedBun.file().stream()reader. The scope here (fd 0, 1 or 2 on a pipe, socket or terminal) leaves out both of those sources.Repro (base is the head of #46528, 9cd5284)
On a terminal: a console loop reads
goand breaks. ThenBun.spawnstarts a child with stdin"inherit"that readsprocess.stdin'data'events, and eight lines are typed. The parent and the child both poll the terminal, and each line goes to one of them. A child that blocks inread(2)wins every line, so only a child that reads through an event loop (an editor, a REPL, ssh) loses input.Reference. Node v26.3.0 on a terminal, with the same child:
Readable.toWeb(process.stdin).getReader(),read(),releaseLock()rl.once("line"), thenrl.close()for await (const line of rl) { break }Node has this on a terminal only, where it does not read ahead (
process.stdin.readableHighWaterMarkis 0 there and 65,536 on a pipe). On a pipe Node reads ahead: afterpause()and after a readlinebreak, a laterfs.readSync(0)finds nothing. Its docs say that iterating a readline interface "will always consume the input stream fully". So the pipe rows follow the rule that #23341 set forprocess.stdin.pause(), not Node. Bun has synchronous readers of fd 0 that Node has not (prompt,confirm,alert). The console iterator of Bun also keeps the lines that it did not yield for the next loop (#5175), so itsbreakis a pause and not an end.Second and third fix in this PR
resolve_pending_read). The loop read once more after a chunk of 131,072 bytes or more. With a second write of 131,072 bytes queued, the base and the first form of this PR left 0 of those bytes on fd 0. Now 131,072 stay, also forprocess.stdin.pause()inside a'data'listener.ConsoleObject.ts, the loop that runs when a stopped iterator starts again).printf 'a\nb\nc' | bun x.jswith abreakafter the first line lostc. One line per loop over 3.2 MB of piped input cut 48 of 200,000 lines on the base. The stop makes chunks smaller, so it cut 78 before this line was added, and 0 now. The line is the one of hot: let the newest console iterator take stdin after a reload #43951.Measurements (linux x64, release builds unless noted, base 9cd5284 against this PR)
break, of 8 (5 runs)getReader(),read(),releaseLock()(5 runs)fs.readSync(0)gets the late line, of 4breakthat get the late input, of 25 formsbreakafter one line (3 runs each)getReader()andreleaseLock()cycle: JS sourceBun.file(path)streaminstructions:u, median of 5.text,.rodata,.bun_builtinssize_of::<FileReader>()Bun.file(0).stream()readers on a terminal, after one EOF and GC (at 3fd0627)set_flowing(true), 300 loops take all of an 8 MiB feed into the buffer of the source.disown()repair for abreak, thenprocess.stdin.pause(), with its own test. Every row is measured again on 9cd5284, except the last two.perf_event_opencounter forinstructions:u,size -A, and the debugSYSandFileReaderlogs.strace,perfandbloatyare not installed here.Scope. The source stops when its Blob names fd 0, 1 or 2 and the descriptor is a pipe, a socket or a terminal. On Windows only a pipe stops. These stay out on purpose:
Bun.spawnpipe. The child must not block, and the parent has to see the pipe end.Bun.file(path). A stopped reader that is abandoned never reaches its end.Bun.file(fd)for another descriptor.uv__cancel_read_console), so a stop in the middle of a typed line drops that line. I did not run a Windows console, so that source does not stop here.The scope is a flag because a terminal is read through a second descriptor, so the descriptor of the reader does not say that the source is stdin. The release is its own extern function because
updateRef(false)is also whatprocess.stdin.unref()calls, and that source has to keep reading.shared_stdiois about the descriptor. Thesharedflag of #45431 is about the cached stream of a Blob.Not fixed here. No user reported these forms, so I did not open an issue for them. This list is the record.
Readable.toWeb(process.stdin), then a reader release: 65,536 to 131,072 of 200,000 late bytes are taken, and a child on a terminal gets 2 to 5 of 8 lines (Node 8).Bun.file("/dev/stdin").stream(), then a reader release. 131,072 of 200,000 late bytes are taken.printf 'p\nx\ny\n' | bun x.jsall three lines are in the first chunk. Node does the same like for like.reader.cancel(), abreakout offor await (const chunk of Bun.stdin.stream()), a defaultpipeTo) ends the one cached stream for the rest of the process. It takes no late input, on the base and here.Bun.file(0).stream()besideBun.stdin.stream()) collide in epoll on Linux (Bun.file().stream(): poll a dup of a stdio pipe, socket or terminal #46807). On the base the second source fails withEEXIST. Here it reads, and the shared stream fails at its next lock if the second source still holds the descriptor.prompt()reads through its own 4096-byte buffer (Delete BufferedStdin: prompt(), bun init and the publish OTP question take one line of stdin #36369). The tests useconfirm()and a one-bytefs.readSyncloop.disown()ofprocess.stdinis not changed. ItssetFlowing(false)andupdateRef()calls do nothing more for a pipe or a terminal, and a file stdin still needs the first one.Tests. 49 new tests in
process-stdin.test.tsand 2 inconsole-iterator.test.ts. The child asks throughprocess.send(), the test writes the next input and answers, so no new test waits for time.confirm(), 200,000 late bytes, a release inside a large delivery,finished()on a stopped stream, a FIFO row, 2 terminal rows, and the 2 console iterator tests.process-stdin.test.tsflowingtest in the deliveryset_flowing(true)is_stdio &&in the scopeBun.file(fd)over a FIFO testConsoleObject.tsbun-serve-file.test.ts:1891andFdPipe.rs:79note it), and the first of the two timed out on macOS.test/js/web/streams,test/js/bun/spawn,test/js/bun/console,test/js/node/readline,test/js/node/process, and 55 more files that usereleaseLock(),preventCancelor stdin, 141 files in all. 6 tests fail. 3 fail on the base in every run. The other 3 are timing tests that fail on the base in some runs (4 of 8, 5 of 8 and 25 of 60).Self-review. These recommendations are not followed, or only in part:
disown()that the hook makes redundant. That function belongs to http: consolidate the open fetch, Bun.serve, node:http, node:http2 and HTTP/3 pull requests #46528, which changed it again in 9cd5284. I left it to that PR.test.todorows for the forms that are not fixed. No user ran into them, so the list above is the record.Bun.file(fd)test needs a FIFO.SourceContext::set_ref_unref. The release no longer goes through it.Platforms. Linux x64 ran here. Windows and macOS ran in CI only.
cargo checkpasses for x86_64-pc-windows-msvc, aarch64-pc-windows-msvc and aarch64-apple-darwin.no test proof · iteration 0 · 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