Repository navigation
console: output goes behind what process.stdout/stderr have buffered, as in Node #37128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
90e0621
cde99d8
584ddb3
40db416
b704f36
342d214
e6f489e
8566d73
70e70ac
7bd8448
b7a141f
cec5076
0e78647
7eaf6ab
f35776a
4b248ac
1e52b84
e63575b
c167a8b
b5a2451
7d7d6dc
67b116c
46f1c79
ab797ac
f07b71e
a41ced3
9033ecd
3464a31
83e1363
8658131
75e6511
1954570
7f91520
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,6 +270,13 @@ unsafe fn vm_console_mut<'a>(global: &JSGlobalObject) -> &'a mut ConsoleObject { | |
| unsafe { &mut *vm_console(global) } | ||
| } | ||
|
|
||
| unsafe extern "Rust" { | ||
| /// Defined in `bun_runtime::webcore::file_sink`: the sink behind `process.stdout` (1) / `process.stderr` (2) holds bytes a full pipe or socket refused. | ||
| safe fn __bun_stdio_sink_is_backed_up(global: &JSGlobalObject, fd: i32) -> bool; | ||
| /// Appends `message` behind those bytes; `false`, with nothing appended, if there are none any more. | ||
| safe fn __bun_stdio_sink_append(global: &JSGlobalObject, fd: i32, message: &[u8]) -> bool; | ||
| } | ||
|
|
||
| static STDERR_MUTEX: Mutex = Mutex::new(); | ||
| static STDOUT_MUTEX: Mutex = Mutex::new(); | ||
|
|
||
|
|
@@ -445,19 +452,27 @@ fn message_with_type_and_level_( | |
| let default_indent = unsafe { vm_console_mut(global) }.default_indent; | ||
|
|
||
| // SAFETY: see [`vm_console`] — `console` points at the live boxed | ||
| // `ConsoleObject` for this VM; JS-thread-only. Kept as a raw deref (not | ||
| // `vm_console_mut`) so the resulting `writer` borrow does not pin a | ||
| // long-lived `&mut ConsoleObject` across the re-derive in the empty-`Log` | ||
| // arm below. | ||
| let raw_writer: &mut bun_core::io::Writer = unsafe { | ||
| // `ConsoleObject` for this VM; JS-thread-only. | ||
| let (raw_writer, fd): (&mut bun_core::io::Writer, i32) = unsafe { | ||
| if matches!(level, MessageLevel::Warning | MessageLevel::Error) { | ||
| (*console).error_writer() | ||
| ((*console).error_writer(), 2) | ||
| } else { | ||
| (*console).writer() | ||
| ((*console).writer(), 1) | ||
| } | ||
| }; | ||
| // As in Node, a message for a stream that is backed up is formatted first and then goes behind what the stream buffered. | ||
| let mut behind_sink = None; | ||
| // `bun_core::io::Writer: bun_io::Write` — `&mut Writer` unsize-coerces directly. | ||
| let writer: &mut dyn bun_io::Write = raw_writer; | ||
| let writer: &mut dyn bun_io::Write = if __bun_stdio_sink_is_backed_up(global, fd) { | ||
| &mut **behind_sink.insert(scopeguard::guard(Vec::new(), move |message: Vec<u8>| { | ||
| if !__bun_stdio_sink_append(global, fd, &message) { | ||
| let _ = raw_writer.write_all(&message); | ||
| let _ = raw_writer.flush(); | ||
| } | ||
| })) | ||
|
Comment on lines
+466
to
+472
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Users piping to a slow consumer now lose a console line entirely when Why this was flaggedTrigger: fd 1 is a pipe or socket to a slow reader ( Verification: src/jsc/ConsoleObject.rs:466-472 — when |
||
| } else { | ||
| raw_writer | ||
| }; | ||
|
|
||
| // LAYERING: `Jest::runner()` lives in `bun_runtime::test_runner` (forward | ||
| // dep on the high tier). Dispatch through `RuntimeHooks` instead — the | ||
|
|
@@ -541,12 +556,8 @@ fn message_with_type_and_level_( | |
| print_options, | ||
| )?; | ||
| } else if message_type == MessageType::Log { | ||
| // SAFETY: see [`vm_console`]. `writer` (above) is dead in this arm — | ||
| // the only later uses are in the mutually-exclusive `Trace` block, and | ||
| // `message_type == Log` here. | ||
| let w = unsafe { (*console).writer() }; | ||
| let _ = w.write_all(b"\n"); | ||
| let _ = w.flush(); | ||
| let _ = writer.write_all(b"\n"); | ||
| let _ = writer.flush(); | ||
| } else if message_type != MessageType::Trace { | ||
| let _ = writer.write_all(b"undefined\n"); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -210,6 +210,34 @@ impl FileSink { | |
| } | ||
| } | ||
|
|
||
| unsafe extern "C" { | ||
| /// The `FileSink` wrapper behind `process.stdout` (1) / `process.stderr` (2), or `undefined` before that stream exists. | ||
| safe fn Bun__Process__getStdioSink(global: &JSGlobalObject, fd: i32) -> JSValue; | ||
| } | ||
|
|
||
| /// The writer of that sink, while it holds bytes a full pipe or socket refused. | ||
| fn backed_up_stdio_writer(global: &JSGlobalObject, fd: i32) -> Option<&JsCell<IOWriter>> { | ||
| let this_ptr = JSSink::from_js(Bun__Process__getStdioSink(global, fd))?; | ||
| // SAFETY: `from_js` returned the wrapper's live `m_ctx`; `global`'s `Process` keeps the wrapper alive. | ||
| let writer = unsafe { &(*this_ptr).sink.writer }; | ||
| // A writer whose poll failed to register is closed with bytes left, and nothing writes them. | ||
| #[cfg(unix)] | ||
| writer.get().get_poll()?; | ||
| writer.get().has_pending_data().then_some(writer) | ||
| } | ||
|
|
||
| #[unsafe(no_mangle)] | ||
| fn __bun_stdio_sink_is_backed_up(global: &JSGlobalObject, fd: i32) -> bool { | ||
| backed_up_stdio_writer(global, fd).is_some() | ||
| } | ||
|
|
||
| /// The poll that is armed for the bytes already there writes `message` after them. | ||
| #[unsafe(no_mangle)] | ||
| fn __bun_stdio_sink_append(global: &JSGlobalObject, fd: i32, message: &[u8]) -> bool { | ||
| backed_up_stdio_writer(global, fd) | ||
| .is_some_and(|writer| writer.with_mut(|w| w.outgoing.write(message)).is_ok()) | ||
|
Comment on lines
+236
to
+238
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Programs that log in a synchronous loop after one backed-up process.stdout.write now hold their whole output in memory and print nothing until the loop ends. Once the sink's outgoing is non-empty, every console.* call reaches __bun_stdio_sink_append at src/runtime/webcore/FileSink.rs:238, which only extends outgoing and never tries write(2) or waits for the reader, so the backlog grows by one message per call. Fix: bound the buffered bytes on this path, e.g. when outgoing is above a cap, flush() the writer and poll(POLLOUT) until it drops below it before appending, so order is kept but memory and latency stay bounded as on the base. Why this was flaggedTrigger: fd 1 is a pipe or socket whose reader has not yet drained it, user code runs process.stdout.write(chunk) that returns false, then a synchronous loop of console.log. message_with_type_and_level_ at src/jsc/ConsoleObject.rs:466 sees __bun_stdio_sink_is_backed_up and formats into a Vec; its drop guard at ConsoleObject.rs:468 calls __bun_stdio_sink_append, which at FileSink.rs:238 does w.outgoing.write(message) and returns. No write(2), flush or poll happens on this path, and nothing else drains outgoing while JS runs synchronously, so every message of the loop accumulates in outgoing until the loop returns to the event loop. On the base branch console.log wrote straight to fd 1 and fd_write_all_quiet at src/sys/lib.rs:9235 waited on poll(POLLOUT) on EAGAIN, so memory stayed flat and output streamed at the reader's pace; a multi-gigabyte dump that streamed on the base now needs that much RAM and shows nothing until the loop ends. Node buffers similarly, but the base did not. Verification: src/jsc/ConsoleObject.rs:466 picks the Vec path whenever |
||
| } | ||
|
|
||
| #[unsafe(no_mangle)] | ||
| pub(crate) extern "C" fn Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio( | ||
| _global: *mut JSGlobalObject, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Programs with Web Workers can now get a worker's console line spliced into the middle of a main-thread console message when stdout is backed up; on the base every console message reached the pipe whole. The message is appended to the sink at src/jsc/ConsoleObject.rs:468 and later written by the poll's drain in as many write(2) calls as the pipe allows, outside the console lock taken at src/jsc/ConsoleObject.rs:419, so a worker's direct console write can land between two parts. Fix: keep a console message atomic across threads once it is queued, e.g. drain the sink's tail under the per-fd console lock or write each queued message in one write(2) that is not split across rounds.
Why this was flagged
Trigger: fd 1 is a pipe or socket to a slow reader; the main thread runs process.stdout.write(Buffer.alloc(1 << 20)) (returns false), then several console.log lines, while a Web Worker (its own ConsoleObject, same fd 1, same STDOUT_MUTEX) also logs. The bytes are written later by on_poll at src/io/PipeWriter.rs:180-188, where drain_buffered_data (src/io/PipeWriter.rs:226-247) writes as much as the kernel takes and returns Pending at an arbitrary byte offset, which can fall inside a queued console message; the rest goes out on a later round after register_poll. No lock is held during those rounds, so the worker's console.log, which writes under the lock straight to fd 1 through its raw writer, lands between the two halves. On the base branch the main thread wrote the whole message through raw_writer and its flush while holding ConsoleStreamLock (src/jsc/ConsoleObject.rs:419), polling on EAGAIN under the lock, so other threads' console output could never split a message.
Verification: On this branch, when
__bun_stdio_sink_is_backed_upis true (src/jsc/ConsoleObject.rs:466), the message is formatted into aVecand, at the scopeguard drop (:467-472), handed to__bun_stdio_sink_append(src/runtime/webcore/FileSink.rs:236-239), which only doesw.outgoing.write(message). The lock is released when_stream_lockdrops at scope exit; nothing has been written to the fd yet.