From 2d2428f6661f16a26e3529cc6d968574681441d3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:46:57 +0000 Subject: [PATCH 1/3] crash_handler: wait for a full stderr instead of dropping the report StderrWriter::write_all issued one write(2) per piece of the report and ignored the result. When fd 2 is a non-blocking pipe whose buffer is full (Bun sets O_NONBLOCK on a piped stderr as soon as process.stderr is used, and the flag is inherited through the open file description), every piece failed with EAGAIN and the whole report, including the trace string, was lost. Short writes and EINTR dropped bytes the same way. Loop until the buffer is written: continue after a short write, retry on EINTR, and on EAGAIN poll(2) for POLLOUT on fd 2 like a blocking stderr would wait. Any other error still drops the bytes and returns Ok, so the callers' abort()-on-error paths, the report upload and the re-raise of the original signal are unaffected. The Windows WriteFile branch loops on partial writes the same way and uses the kernel32 binding bun_sys exports. --- src/crash_handler/lib.rs | 80 +++++++++++++++--------- test/cli/run/run-crash-handler.test.ts | 85 +++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 30 deletions(-) diff --git a/src/crash_handler/lib.rs b/src/crash_handler/lib.rs index a5cb8c8966ea..57549eee7ebf 100644 --- a/src/crash_handler/lib.rs +++ b/src/crash_handler/lib.rs @@ -399,12 +399,16 @@ pub use bun_io::{FmtAdapter, Write}; /// `bun_sys::stderr_writer()` (not yet exposed by T1). /// Only impls `bun_io::Write` — `write!` resolves to `bun_io::Write::write_fmt` /// (alloc-free stack `Bridge`, async-signal-safe). +/// +/// Never returns `Err`: callers `abort()` on error, which would also skip the +/// report upload and the re-raise of the original signal, so a closed stderr +/// or a vanished reader silently drops the bytes. A full stderr is waited for. pub(crate) struct StderrWriter; pub(crate) fn stderr_writer() -> StderrWriter { StderrWriter } impl Write for StderrWriter { - fn write_all(&mut self, bytes: &[u8]) -> bun_io::Result<()> { + fn write_all(&mut self, mut bytes: &[u8]) -> bun_io::Result<()> { #[cfg(windows)] { // On Windows this is `GetStdHandle(STD_ERROR_HANDLE)` + kernel32 @@ -413,39 +417,57 @@ impl Write for StderrWriter { // per-fd lock, which can self-deadlock when the VEH crash handler // fires on a thread that faulted *inside* CRT stdio. WriteFile is // lock-free at the kernel32 layer. - // `WriteFile` is declared locally because `bun_windows_sys:: - // kernel32` does not (yet) export it (cf. src/sys/lib.rs). - #[link(name = "kernel32")] - unsafe extern "system" { - fn WriteFile( - hFile: bun_sys::windows::HANDLE, - lpBuffer: *const u8, - nNumberOfBytesToWrite: u32, - lpNumberOfBytesWritten: *mut u32, - lpOverlapped: *mut core::ffi::c_void, - ) -> i32; - } let h = bun_sys::windows::kernel32::GetStdHandle(bun_sys::windows::STD_ERROR_HANDLE); - let mut written: u32 = 0; - // SAFETY: `h` is the cached stderr HANDLE (or INVALID_HANDLE_VALUE, - // in which case WriteFile fails harmlessly); `bytes` is valid for - // reads of `len`; `written` is a valid out-pointer; lpOverlapped - // is null for synchronous I/O. - unsafe { - WriteFile( - h, - bytes.as_ptr(), - bytes.len() as u32, - &mut written, - core::ptr::null_mut(), - ); + while !bytes.is_empty() { + let mut written: u32 = 0; + // SAFETY: `h` is the cached stderr HANDLE (or INVALID_HANDLE_VALUE, + // in which case WriteFile fails harmlessly); `bytes` is valid for + // reads of `len`; `written` is a valid out-pointer; lpOverlapped + // is null for synchronous I/O. + let ok = unsafe { + bun_sys::windows::kernel32::WriteFile( + h, + bytes.as_ptr(), + u32::try_from(bytes.len()).unwrap_or(u32::MAX), + &mut written, + core::ptr::null_mut(), + ) + }; + if ok == 0 || written == 0 { + break; + } + bytes = &bytes[written as usize..]; } } #[cfg(not(windows))] { - // SAFETY: fd 2 is always open; libc::write is async-signal-safe. - unsafe { - libc::write(2, bytes.as_ptr().cast(), bytes.len() as _); + // Everything here is async-signal-safe: write(2) and poll(2), and + // `bun_sys::Error` carries no allocation. + let stderr = bun_sys::Fd::stderr(); + while !bytes.is_empty() { + match bun_sys::write(stderr, bytes) { + Ok(0) => break, + Ok(n) => bytes = &bytes[n..], + Err(err) => match err.get_errno() { + bun_sys::E::EINTR => {} + // fd 2 is O_NONBLOCK once `process.stderr` has touched a + // pipe (the flag is on the open file description, so it is + // inherited too). A full pipe only means the reader is + // behind: wait for it, as a blocking stderr would, instead + // of dropping the rest of the report. + bun_sys::E::EAGAIN => { + let mut pfd = [bun_sys::posix::PollFd { + fd: stderr.native(), + events: bun_sys::posix::POLL_OUT, + revents: 0, + }]; + if bun_sys::posix::poll(&mut pfd, -1).is_err() { + break; + } + } + _ => break, + }, + } } } Ok(()) diff --git a/test/cli/run/run-crash-handler.test.ts b/test/cli/run/run-crash-handler.test.ts index 4d436bbedc7b..ca596d54a447 100644 --- a/test/cli/run/run-crash-handler.test.ts +++ b/test/cli/run/run-crash-handler.test.ts @@ -1,7 +1,8 @@ import { crash_handler } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isASAN, isDebug, isLinux, isPosix, isWindows, mergeWindowEnvs, tempDir } from "harness"; -import { rmSync } from "node:fs"; +import { mkfifo } from "mkfifo"; +import { closeSync, constants, openSync, rmSync, writeSync } from "node:fs"; import path from "path"; const { getMachOImageZeroOffset } = crash_handler; @@ -129,6 +130,88 @@ describe.if(isPosix)("terminal signal reflects the crash cause", () => { }); }); +// The report is written to fd 2 with many small raw write(2) calls. fd 2 is +// often O_NONBLOCK: Bun puts the flag on a piped stderr as soon as +// process.stderr is used, and since it lives on the open file description a +// parent can hand down an fd that already has it. If the reader is behind at +// that moment every write fails with EAGAIN, and the handler used to drop the +// bytes and carry on, so the process died without printing anything. It has +// to wait for the reader instead, like a blocking stderr would. +describe.if(isPosix)("crash report reaches a full non-blocking stderr", () => { + const reportHeader = Buffer.alloc(60, "=").toString() + "\n"; + + test.concurrent.each([ + ["panic", "invoked crashByPanic() handler", "SIGABRT"], + ["segfault", "Segmentation fault at address 0xDEADBEEF", "SIGSEGV"], + ] as const)("%s", async (approach, expectedReason, expectedSignal) => { + using dir = tempDir("crash-stderr-full", {}); + // A fifo is a pipe whose read end this process holds without Bun.spawn + // draining it: the child's stderr has to stay full until the crash handler + // has tried to write to it. + const fifo = path.join(String(dir), "stderr"); + mkfifo(fifo); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + let writeEnd: number | undefined = openSync(fifo, constants.O_WRONLY | constants.O_NONBLOCK); + try { + const fill = Buffer.alloc(64 * 1024, "f"); + let filled = 0; + let fillStoppedBy: string | undefined; + while (fillStoppedBy === undefined) { + try { + filled += writeSync(writeEnd, fill); + } catch (e) { + fillStoppedBy = (e as NodeJS.ErrnoException).code; + } + } + expect({ fillStoppedBy, filled: filled > 0 }).toEqual({ fillStoppedBy: "EAGAIN", filled: true }); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "--debug-crash-handler-use-trace-string", + "-e", + // Both requires come first: loading bun:internal-for-testing takes + // over a second in debug builds, and the marker has to be as close + // as possible to the crash itself. + `const { crash_handler } = require("bun:internal-for-testing"); + const { writeSync } = require("node:fs"); + writeSync(1, "crashing\\n"); + crash_handler.${approach}();`, + ], + env: noReportEnv, + stdio: ["ignore", "pipe", writeEnd], + }); + // The child now holds the only write end, so EOF on the fifo means it died. + closeSync(writeEnd); + writeEnd = undefined; + + const { value: marker } = await proc.stdout.getReader().read(); + expect(new TextDecoder().decode(marker)).toBe("crashing\n"); + + // The child crashes right after the marker and has its whole report + // written (or, before the fix, dropped) within a millisecond or so. + // Nothing observable separates "still formatting" from "waiting in + // poll(2) for us", so give it a moment before draining the fifo: without + // the fix the child exits on its own, with it the race times out. + await Promise.race([proc.exited, Bun.sleep(1_000)]); + + // Draining the fill unblocks the child; whatever follows it is the report. + const report = (await Bun.file(readEnd).text()).slice(filled); + await proc.exited; + + expect(report).toStartWith(reportHeader); + expect(report).toContain(`panic(main thread): ${expectedReason}\n`); + expect(report).toContain("oh no: Bun has crashed. This indicates a bug in Bun, not your code.\n"); + expect(report.trimEnd().split("\n").at(-1)).toMatch(/^ \/\d+\.\d+\.\d+\/\S+$/); + expect(report).toEndWith("\n\n"); + expect(proc.signalCode).toBe(expectedSignal); + } finally { + if (writeEnd !== undefined) closeSync(writeEnd); + closeSync(readEnd); + } + }); +}); + // POSIX-only: Windows refuses to remove a directory that is any process's cwd. describe.if(isPosix)("cwd deleted before startup", () => { test.concurrent.each(["install", "test"])("bun %s prints the cwd-deleted hint", async cmd => { From 20211bf16a7386217ca2b09a19adb316d6a4b056 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 14:37:59 +0000 Subject: [PATCH 2/3] crash_handler: write through bun_sys::write on Windows too One loop for both platforms; Windows only differs in how the stderr handle is obtained. The live GetStdHandle lookup stays because Fd::stderr() is only populated once Output's stdio init has run, which is after the crash handler is installed. --- src/crash_handler/lib.rs | 100 +++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 57 deletions(-) diff --git a/src/crash_handler/lib.rs b/src/crash_handler/lib.rs index 57549eee7ebf..b3b3ad8e6628 100644 --- a/src/crash_handler/lib.rs +++ b/src/crash_handler/lib.rs @@ -403,71 +403,57 @@ pub use bun_io::{FmtAdapter, Write}; /// Never returns `Err`: callers `abort()` on error, which would also skip the /// report upload and the re-raise of the original signal, so a closed stderr /// or a vanished reader silently drops the bytes. A full stderr is waited for. +/// +/// Everything below is async-signal-safe: `bun_sys::write` is a bare +/// `write(2)` (a bare kernel32 `WriteFile` on Windows, which matters there +/// too: the CRT's `_write` takes a per-fd lock that the VEH handler would +/// self-deadlock on when the faulting thread was inside CRT stdio), +/// `bun_sys::posix::poll` is a bare `poll(2)`, and `bun_sys::Error` does not +/// allocate. pub(crate) struct StderrWriter; pub(crate) fn stderr_writer() -> StderrWriter { StderrWriter } impl Write for StderrWriter { fn write_all(&mut self, mut bytes: &[u8]) -> bun_io::Result<()> { - #[cfg(windows)] - { - // On Windows this is `GetStdHandle(STD_ERROR_HANDLE)` + kernel32 - // `WriteFile`, NOT the CRT. Routing through MSVCRT `_write(2,…)` - // would (1) text-mode-translate `\n`→`\r\n` and (2) take the CRT - // per-fd lock, which can self-deadlock when the VEH crash handler - // fires on a thread that faulted *inside* CRT stdio. WriteFile is - // lock-free at the kernel32 layer. - let h = bun_sys::windows::kernel32::GetStdHandle(bun_sys::windows::STD_ERROR_HANDLE); - while !bytes.is_empty() { - let mut written: u32 = 0; - // SAFETY: `h` is the cached stderr HANDLE (or INVALID_HANDLE_VALUE, - // in which case WriteFile fails harmlessly); `bytes` is valid for - // reads of `len`; `written` is a valid out-pointer; lpOverlapped - // is null for synchronous I/O. - let ok = unsafe { - bun_sys::windows::kernel32::WriteFile( - h, - bytes.as_ptr(), - u32::try_from(bytes.len()).unwrap_or(u32::MAX), - &mut written, - core::ptr::null_mut(), - ) - }; - if ok == 0 || written == 0 { - break; - } - bytes = &bytes[written as usize..]; - } - } #[cfg(not(windows))] - { - // Everything here is async-signal-safe: write(2) and poll(2), and - // `bun_sys::Error` carries no allocation. - let stderr = bun_sys::Fd::stderr(); - while !bytes.is_empty() { - match bun_sys::write(stderr, bytes) { - Ok(0) => break, - Ok(n) => bytes = &bytes[n..], - Err(err) => match err.get_errno() { - bun_sys::E::EINTR => {} - // fd 2 is O_NONBLOCK once `process.stderr` has touched a - // pipe (the flag is on the open file description, so it is - // inherited too). A full pipe only means the reader is - // behind: wait for it, as a blocking stderr would, instead - // of dropping the rest of the report. - bun_sys::E::EAGAIN => { - let mut pfd = [bun_sys::posix::PollFd { - fd: stderr.native(), - events: bun_sys::posix::POLL_OUT, - revents: 0, - }]; - if bun_sys::posix::poll(&mut pfd, -1).is_err() { - break; - } + let stderr = bun_sys::Fd::stderr(); + // Looked up live rather than through `Fd::stderr()`: the cache behind + // that is filled by `Output`'s stdio init, which runs after the crash + // handler is installed, and crashes in between still have to print. + #[cfg(windows)] + let Some(stderr) = bun_sys::windows::GetStdHandle(bun_sys::windows::STD_ERROR_HANDLE) + .map(bun_sys::Fd::from_system) + else { + return Ok(()); + }; + while !bytes.is_empty() { + match bun_sys::write(stderr, bytes) { + Ok(0) => break, + Ok(n) => bytes = &bytes[n..], + Err(err) => match err.get_errno() { + // `bun_sys::write` retries EINTR itself on Linux, but not on + // macOS (`write$NOCANCEL` is issued once). + #[cfg(unix)] + bun_sys::E::EINTR => {} + // fd 2 is O_NONBLOCK once `process.stderr` has touched a + // pipe (the flag is on the open file description, so it is + // inherited too). A full pipe only means the reader is + // behind: wait for it, as a blocking stderr would, instead + // of dropping the rest of the report. + #[cfg(unix)] + bun_sys::E::EAGAIN => { + let mut pfd = [bun_sys::posix::PollFd { + fd: stderr.native(), + events: bun_sys::posix::POLL_OUT, + revents: 0, + }]; + if bun_sys::posix::poll(&mut pfd, -1).is_err() { + break; } - _ => break, - }, - } + } + _ => break, + }, } } Ok(()) From 942f9eff7bace48d220f2995caeff91afa97d49b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 15:41:59 +0000 Subject: [PATCH 3/3] crash_handler: shorter StderrWriter comments; assert the handler is still waiting before draining in the test --- src/crash_handler/lib.rs | 33 ++++++++++---------------- test/cli/run/run-crash-handler.test.ts | 19 ++++++++++----- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/crash_handler/lib.rs b/src/crash_handler/lib.rs index b3b3ad8e6628..3b4a0f6e12b3 100644 --- a/src/crash_handler/lib.rs +++ b/src/crash_handler/lib.rs @@ -400,16 +400,11 @@ pub use bun_io::{FmtAdapter, Write}; /// Only impls `bun_io::Write` — `write!` resolves to `bun_io::Write::write_fmt` /// (alloc-free stack `Bridge`, async-signal-safe). /// -/// Never returns `Err`: callers `abort()` on error, which would also skip the -/// report upload and the re-raise of the original signal, so a closed stderr -/// or a vanished reader silently drops the bytes. A full stderr is waited for. -/// -/// Everything below is async-signal-safe: `bun_sys::write` is a bare -/// `write(2)` (a bare kernel32 `WriteFile` on Windows, which matters there -/// too: the CRT's `_write` takes a per-fd lock that the VEH handler would -/// self-deadlock on when the faulting thread was inside CRT stdio), -/// `bun_sys::posix::poll` is a bare `poll(2)`, and `bun_sys::Error` does not -/// allocate. +/// Waits for a full stderr but never returns `Err` for an unwritable one: +/// callers `abort()` on `Err`, skipping the report upload and signal re-raise. +/// Signal-safe: `bun_sys::write`/`posix::poll` are bare syscalls (on Windows a +/// bare `WriteFile`, not the CRT, whose per-fd lock a VEH handler can deadlock +/// on) and `bun_sys::Error` does not allocate. pub(crate) struct StderrWriter; pub(crate) fn stderr_writer() -> StderrWriter { StderrWriter @@ -418,9 +413,8 @@ impl Write for StderrWriter { fn write_all(&mut self, mut bytes: &[u8]) -> bun_io::Result<()> { #[cfg(not(windows))] let stderr = bun_sys::Fd::stderr(); - // Looked up live rather than through `Fd::stderr()`: the cache behind - // that is filled by `Output`'s stdio init, which runs after the crash - // handler is installed, and crashes in between still have to print. + // Not `Fd::stderr()`: its cache is filled by `Output`'s stdio init, + // which runs after the crash handler is installed. #[cfg(windows)] let Some(stderr) = bun_sys::windows::GetStdHandle(bun_sys::windows::STD_ERROR_HANDLE) .map(bun_sys::Fd::from_system) @@ -432,15 +426,14 @@ impl Write for StderrWriter { Ok(0) => break, Ok(n) => bytes = &bytes[n..], Err(err) => match err.get_errno() { - // `bun_sys::write` retries EINTR itself on Linux, but not on - // macOS (`write$NOCANCEL` is issued once). + // `bun_sys::write` only retries EINTR on Linux; macOS issues + // `write$NOCANCEL` once. #[cfg(unix)] bun_sys::E::EINTR => {} - // fd 2 is O_NONBLOCK once `process.stderr` has touched a - // pipe (the flag is on the open file description, so it is - // inherited too). A full pipe only means the reader is - // behind: wait for it, as a blocking stderr would, instead - // of dropping the rest of the report. + // A piped fd 2 is O_NONBLOCK once `process.stderr` has used + // it (the flag is on the shared open file description). + // Full only means the reader is behind: wait like a blocking + // stderr would instead of dropping the report. #[cfg(unix)] bun_sys::E::EAGAIN => { let mut pfd = [bun_sys::posix::PollFd { diff --git a/test/cli/run/run-crash-handler.test.ts b/test/cli/run/run-crash-handler.test.ts index ca596d54a447..c556b89bcd08 100644 --- a/test/cli/run/run-crash-handler.test.ts +++ b/test/cli/run/run-crash-handler.test.ts @@ -188,12 +188,19 @@ describe.if(isPosix)("crash report reaches a full non-blocking stderr", () => { const { value: marker } = await proc.stdout.getReader().read(); expect(new TextDecoder().decode(marker)).toBe("crashing\n"); - // The child crashes right after the marker and has its whole report - // written (or, before the fix, dropped) within a millisecond or so. - // Nothing observable separates "still formatting" from "waiting in - // poll(2) for us", so give it a moment before draining the fifo: without - // the fix the child exits on its own, with it the race times out. - await Promise.race([proc.exited, Bun.sleep(1_000)]); + // The child crashes right after the marker and every report write hits + // the full fifo within about a millisecond of it (measured on release and + // debug ASAN builds). A handler that waits is then blocked in poll(2), + // which nothing outside the process can observe, so the only evidence + // that it is waiting rather than dead is that it is still alive well + // after that point; a handler that dropped the report is gone within a + // few hundred milliseconds. The sleep is that margin, not a wait for the + // child to get going. + const exitedWithoutReader = await Promise.race([ + proc.exited.then(() => true), + Bun.sleep(1_000).then(() => false), + ]); + expect(exitedWithoutReader, "crash handler gave up on stderr instead of waiting for the reader").toBe(false); // Draining the fill unblocks the child; whatever follows it is the report. const report = (await Bun.file(readEnd).text()).slice(filled);