From 6ebbe5a23bce3c4b59831bdfa2b2139e7a50a3a2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:55:26 +0000 Subject: [PATCH 1/3] spawn(windows): free the buffer-stdin writer when its uv_write fails inside start() StaticPipeWriter::start() takes a ref on the writer and records it in `started` after the buffered writer has started. On Windows starting the writer issues the uv_write, and when that fails synchronously the writer closes itself inside the call: on_close runs before `started` is set, so it releases nothing, and the owner's on_close_io drops create()'s ref and empties its slot. start() then set `started` on a writer nothing could reach any more, stranding its ref, so the writer was never freed. start() now releases its own ref when it finds the writer closed underneath it and reports Ok, the same outcome as a write that fails asynchronously. That release frees the writer for Subprocess and ShellSubprocess, so start() takes a raw pointer instead of `&mut self`. SecurityScanSubprocess, which releases start()'s ref itself right after start() returns, now claims it through the `started` token so it does not release a ref start() already released; this also covers start() returning Err, where it was releasing one ref too many. The failing uv_write cannot be arranged from JS, so debug builds get a fault-injection flag for it, plus create()/deinit() lines in the StaticPipeWriter debug scope that the test counts for all three owners. --- src/bun_core/env_var.rs | 3 + .../PackageManager/security_scanner.rs | 26 +++-- src/io/PipeWriter.rs | 29 +++-- src/runtime/api/bun/js_bun_spawn_bindings.rs | 6 +- src/runtime/shell/subproc.rs | 14 ++- src/spawn/static_pipe_writer.rs | 102 ++++++++++++------ .../bun/spawn/buffer-stdin-owners-fixture.ts | 36 +++++++ .../bun/spawn/spawn-pipe-start-error.test.ts | 98 ++++++++++++++++- 8 files changed, 259 insertions(+), 55 deletions(-) create mode 100644 test/js/bun/spawn/buffer-stdin-owners-fixture.ts diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index eb14e27b6a99..42fc883ec370 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -273,6 +273,9 @@ pub mod feature_flag { new_feature_flag!(pub BUN_INTERNAL_BUNX_INSTALL, "BUN_INTERNAL_BUNX_INSTALL", {}); // Debug-only fault injection for test/js/bun/spawn/spawn-pipe-start-error.test.ts. new_feature_flag!(pub BUN_INTERNAL_FAIL_PIPE_READER_START, "BUN_INTERNAL_FAIL_PIPE_READER_START", {}); + // Debug-only fault injection for test/js/bun/spawn/spawn-pipe-start-error.test.ts: + // every stream write the Windows buffered pipe writer issues fails synchronously. + new_feature_flag!(pub BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE, "BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE", {}); // Test-only: bypass the stdin isatty gate in `bun update --interactive` so // tests can drive the multi-select by writing keystrokes to a pipe. new_feature_flag!(pub BUN_INTERNAL_INTERACTIVE_ASSUME_TTY, "BUN_INTERNAL_INTERACTIVE_ASSUME_TTY", {}); diff --git a/src/install/PackageManager/security_scanner.rs b/src/install/PackageManager/security_scanner.rs index a8bad2738929..885d0973f81b 100644 --- a/src/install/PackageManager/security_scanner.rs +++ b/src/install/PackageManager/security_scanner.rs @@ -944,7 +944,7 @@ pub(crate) type StaticPipeWriter = subprocess::StaticPipeWriter subprocess::StaticPipeWriterProcess for SecurityScanSubprocess<'a> { const POLL_OWNER_TAG: bun_io::PollTag = bun_io::PollTag::SecurityScanStaticPipeWriter; unsafe fn on_close_io(this: *mut Self, kind: subprocess::StdioKind) { @@ -1288,8 +1288,8 @@ impl<'a> SecurityScanSubprocess<'a> { (*parent).process = Some(process); } - // Assign the field BEFORE `start()`. `start()` may complete the write synchronously - // (small JSON fits the 64KB pipe buffer on POSIX) and re-enter + // Assign the field BEFORE `start()`. On Windows a write that fails + // synchronously closes the writer inside `start()`, re-entering // `on_close_io` via the `parent` backref; that callback must observe // `json_writer.is_some()` to decrement `remaining_fds`, otherwise // `is_done()` never returns true and `sleep_until` hangs. @@ -1316,13 +1316,21 @@ impl<'a> SecurityScanSubprocess<'a> { }); let writer_ptr = writer_local.as_ptr(); - // SAFETY: `writer_local` holds a live ref; `start()` mutates the writer - // in place (raw intrusive object — no Rust aliasing across the RefPtr). - let start_result = unsafe { (*writer_ptr).start() }; - // SAFETY: `writer_local` keeps `*writer_ptr` live; we own the `start()` ref. - unsafe { RefCount::::deref(writer_ptr) }; + // SAFETY: `writer_local` holds a ref, so the writer stays live across + // and after `start()` whatever it does (it may re-enter `on_close_io` + // and release its own ref; see `StaticPipeWriter::start`). + let start_result = unsafe { StaticPipeWriter::start(writer_ptr) }; + // Claim start()'s ref through the `started` token, the same token the + // writer's own release sites check: `writer_local` covers this frame and + // `json_writer` covers the write, so this owner has no use for it. + // `start()` leaves the token unset when it already released the ref + // itself (it failed, or the write failed synchronously). // SAFETY: `writer_local` keeps `*writer_ptr` live. - unsafe { (*writer_ptr).started = false }; + if unsafe { core::mem::replace(&mut (*writer_ptr).started, false) } { + // SAFETY: `started` was the token for the outstanding start() ref; + // cleared above so no other site releases it. Not the last ref. + unsafe { RefCount::::deref(writer_ptr) }; + } match start_result { Err(e) => { writer_local.deref(); diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 69ec758b42d8..e238324d63e9 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -1716,15 +1716,26 @@ impl WindowsBufferedWriter { self.pending_payload_size = buffer_len; self.write_buffer = write_buf; let self_ptr = self as *mut Self; - if let Some(write_err) = self - .write_req - // SAFETY: `p` is `self_ptr`; libuv invokes on the loop thread with no - // other Rust borrow of `*p` live, so `&mut *p` is the sole alias. - .write(stream_raw, &self.write_buffer, self_ptr, |p, s| unsafe { - (*p).on_write_complete(s) - }) - .to_error(sys::Tag::write) - { + let write_err = 'issue: { + // Debug-only fault injection for + // test/js/bun/spawn/spawn-pipe-start-error.test.ts: a uv_write + // that fails synchronously on a freshly spawned stdio pipe (its + // other end already closed) cannot be arranged from JS. + #[cfg(debug_assertions)] + if bun_core::env_var::feature_flag::BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE.get() + == Some(true) + { + break 'issue Some(sys::Error::from_code(sys::E::PIPE, sys::Tag::write)); + } + self.write_req + // SAFETY: `p` is `self_ptr`; libuv invokes on the loop thread with no + // other Rust borrow of `*p` live, so `&mut *p` is the sole alias. + .write(stream_raw, &self.write_buffer, self_ptr, |p, s| unsafe { + (*p).on_write_complete(s) + }) + .to_error(sys::Tag::write) + }; + if let Some(write_err) = write_err { self.close(); self.parent_on_error(write_err); } else { diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 5397137e80f4..c5291afbd17c 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -1766,7 +1766,11 @@ fn spawn_maybe_sync( } if let Writable::Buffer(buffer) = subprocess.stdin.get() { - if let Err(err) = Writable::buffer_writer_mut(buffer).start() { + // SAFETY: `buffer` holds `create()`'s ref on a live writer. `start()` + // may free the writer (a write that fails synchronously on Windows), + // in which case `on_close_io` has already replaced this stdin slot; + // neither `buffer` nor the writer is used after the call. + if let Err(err) = unsafe { Subprocess::StaticPipeWriter::start(buffer.as_ptr()) } { let _ = subprocess.try_kill(subprocess.kill_signal); let _ = global_this.throw_value(err.to_js(global_this)); return Err(JsError::Thrown); diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index a6ea81124a14..23d36c5b97c0 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -905,12 +905,18 @@ impl ShellSubprocess { } } - // SAFETY: borrow of the stdin slot scoped to this match; single-threaded. - let stdin_start_err = match unsafe { &(*subprocess).stdin } { - // SAFETY: single-threaded; the writer is uniquely reachable here. - Writable::Buffer(buffer) => unsafe { buffer_mut(buffer) }.start().err(), + // SAFETY: borrow of the stdin slot scoped to this match; it ends before + // `start()` below, which may overwrite the slot through `on_close_io`. + let stdin_writer = match unsafe { &(*subprocess).stdin } { + Writable::Buffer(buffer) => Some(buffer.as_ptr()), _ => None, }; + let stdin_start_err = stdin_writer.and_then(|writer| { + // SAFETY: the slot holds `create()`'s ref on a live writer. `start()` + // may free it (a write that fails synchronously on Windows, reported + // through `on_close_io`); nothing touches it afterwards. + unsafe { StaticPipeWriter::start(writer) }.err() + }); if let Some(err) = stdin_start_err { let sys_err = err.to_shell_system_error(); // SAFETY: scoped `&mut` for the kill; `abort_after_failed_start` diff --git a/src/spawn/static_pipe_writer.rs b/src/spawn/static_pipe_writer.rs index 2d6d006f1054..0c7497bead29 100644 --- a/src/spawn/static_pipe_writer.rs +++ b/src/spawn/static_pipe_writer.rs @@ -10,6 +10,8 @@ use bun_sys; use crate::process::StdioKind; use crate::subprocess::{Source, StdioResult}; +// `BUN_DEBUG_StaticPipeWriter=1`; test/js/bun/spawn/spawn-pipe-start-error.test.ts +// counts the `create()` / `deinit()` lines to check writers are freed. bun_output::declare_scope!(StaticPipeWriter, hidden); /// Trait bound for the owning process type `P` of [`StaticPipeWriter`]. @@ -138,6 +140,11 @@ impl StaticPipeWriter

{ } } let this = bun_core::heap::into_raw(boxed); + bun_output::scoped_log!( + StaticPipeWriter, + "StaticPipeWriter(0x{:x}) create()", + this as usize + ); // SAFETY: `this` was just leaked above; borrow scoped to registering // the parent backref. unsafe { (*this).writer.set_parent(this) }; @@ -145,51 +152,79 @@ impl StaticPipeWriter

{ unsafe { IntrusiveRc::from_raw(this) } } - pub fn start(&mut self) -> bun_sys::Result<()> { + /// Takes start()'s `+1` (tracked by `started`) and begins writing. + /// + /// Raw `this` rather than `&mut self`: on Windows a `uv_write` that fails + /// synchronously closes the writer inside this call. `on_close` then runs + /// while `started` is still unset, so it releases nothing, and the owner's + /// `on_close_io` empties its slot and drops `create()`'s ref. Nothing can + /// claim start()'s `+1` after that, so this function releases it itself, + /// which frees `*this`. + /// + /// # Safety + /// `this` must point to a live writer whose owner holds `create()`'s ref. + /// `*this` may be freed when this returns (see above), so the caller must + /// not touch it afterwards; the owner is notified through `on_close_io`. + pub unsafe fn start(this: *mut Self) -> bun_sys::Result<()> { bun_output::scoped_log!( StaticPipeWriter, "StaticPipeWriter(0x{:x}) start()", - std::ptr::from_ref(self) as usize + this as usize ); - // Intrusive-refcount increment. - // SAFETY: `self` is a live `Self` (created via `create()`/`heap::alloc`). - unsafe { RefCount::::ref_(std::ptr::from_mut::(self)) }; - // Self-borrow into `self.source` — see `buffer` field invariant. - self.buffer = RawSlice::new(self.source.slice()); + // SAFETY: caller contract: `this` is live. + unsafe { RefCount::::ref_(this) }; + // Self-borrow into `source`; see the `buffer` field invariant. + // SAFETY: live; the borrow is confined to this statement. + unsafe { (*this).buffer = RawSlice::new((*this).source.slice()) }; #[cfg(windows)] { - let r = self.writer.start_with_current_pipe(); - self.started = r.is_ok(); - if r.is_err() { - // start() failed: `started` stays false so no release site - // fires — release start()'s `+1` here. - // SAFETY: `self` is the live `Self` we ref'd at the top of - // `start()`; the caller's `IntrusiveRc` keeps it alive and - // `started` is false so no other site re-derefs. - unsafe { RefCount::::deref(std::ptr::from_mut::(self)) }; + // SAFETY: the ref taken above keeps `*this` live across the call + // even when it closes the writer (see the doc comment); the borrow + // of the field ends when the call returns. + let result = unsafe { (*this).writer.start_with_current_pipe() }; + // SAFETY: still live, see above. + if result.is_err() || unsafe { (*this).writer.is_done() } { + // `Err`: nothing was closed; the owner still holds `create()`'s + // ref and tears the writer down when the caller reports the + // error. Closed: the write failed synchronously and has been + // reported through `on_error`/`on_close` just like a write + // that fails asynchronously, so the caller sees `Ok`; the + // owner's ref is already gone and this release frees the + // writer. Either way `started` stays false, so no other site + // releases start()'s `+1`. + // SAFETY: releases the ref taken above; last use of `this`. + unsafe { RefCount::::deref(this) }; + return result; } - return r; + // SAFETY: live: the writer is open, so the owner still holds its ref. + unsafe { (*this).started = true }; + bun_sys::Result::Ok(()) } #[cfg(not(windows))] { - // On POSIX `StdioResult` is an `Option`. - match self.writer.start(self.stdio_result.unwrap(), true) { + // On POSIX `StdioResult` is an `Option`. The buffered writer's + // `start()` only registers the poll and never reports to the + // parent, so nothing in this arm can close or free the writer. + // SAFETY: live; each borrow is confined to its own statement. + let fd = unsafe { (*this).stdio_result.unwrap() }; + match unsafe { (*this).writer.start(fd, true) } { bun_sys::Result::Err(err) => { - // start() failed: `started` stays false so no release - // site fires — release start()'s `+1` here. - // SAFETY: `self` is the live `Self` we ref'd at the top - // of `start()`; the caller's `IntrusiveRc` keeps it alive - // and `started` is false so no other site re-derefs. - unsafe { RefCount::::deref(std::ptr::from_mut::(self)) }; + // `started` stays false so no release site fires; release + // start()'s `+1` here. Not the last ref: the owner still + // holds `create()`'s. + // SAFETY: releases the ref taken above; last use of `this`. + unsafe { RefCount::::deref(this) }; bun_sys::Result::Err(err) } bun_sys::Result::Ok(()) => { - self.started = true; + // SAFETY: live, see above. + unsafe { (*this).started = true }; #[cfg(unix)] { // `handle` is `PollOrFd` (enum); flag mutation goes // through the FilePoll vtable shim. - if let Some(poll) = self.writer.handle.get_poll() { + // SAFETY: live, see above. + if let Some(poll) = unsafe { (*this).writer.handle.get_poll() } { poll.set_flag(bun_io::FilePollFlag::Socket); } } @@ -261,9 +296,11 @@ impl StaticPipeWriter

{ // reaches here via `close()` without ever calling `Parent::on_write`, so // this is the last point `started` can be claimed for that path. // `write()`'s +1 (held by that callback's scopeguard) keeps `self` live - // past the deref. POSIX must not release here: `drain_buffered_data` - // may call `on_error()` -> `close()` -> here and then `on_write()` on - // the same object, with no extra ref held. + // past the deref. A write that fails synchronously inside `start()` + // also passes through here, before `started` is set; `start()` releases + // its own ref for that case. POSIX must not release here: + // `drain_buffered_data` may call `on_error()` -> `close()` -> here and + // then `on_write()` on the same object, with no extra ref held. #[cfg(windows)] let release_start_ref = core::mem::replace(&mut self.started, false); // `buffer` aliases `self.source`'s storage; clear it before detach() @@ -296,6 +333,11 @@ impl StaticPipeWriter

{ /// The heap free is handled by `IntrusiveRc` after `drop` returns. impl Drop for StaticPipeWriter

{ fn drop(&mut self) { + bun_output::scoped_log!( + StaticPipeWriter, + "StaticPipeWriter(0x{:x}) deinit()", + std::ptr::from_ref(self) as usize + ); self.writer.end(); // `buffer` aliases `self.source`'s storage; clear it before detach() // frees that storage (upholds the field's documented invariant). diff --git a/test/js/bun/spawn/buffer-stdin-owners-fixture.ts b/test/js/bun/spawn/buffer-stdin-owners-fixture.ts new file mode 100644 index 000000000000..0f1e47425380 --- /dev/null +++ b/test/js/bun/spawn/buffer-stdin-owners-fixture.ts @@ -0,0 +1,36 @@ +// Runs one child per owner of a buffer-stdin writer in this process (Bun.spawn, +// Bun.spawnSync, the shell's `< ${buffer}` redirect). Each child prints how many +// stdin bytes it received; the results go out as one "RESULT " line. +// spawn-pipe-start-error.test.ts runs this with BUN_DEBUG_StaticPipeWriter set and +// counts the writers this process creates and frees. +import { $ } from "bun"; + +const input = Buffer.alloc(4096, "x"); +const cmd = [process.execPath, "-e", "console.log((await Bun.stdin.bytes()).length)"]; +// The children must neither inherit the fault injection (their own stdio is not +// under test) nor log writer events into the stdout this process is measured by. +const env = Object.fromEntries( + Object.entries(process.env).filter( + ([key]) => key !== "BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE" && key !== "BUN_DEBUG_StaticPipeWriter", + ), +); + +const results: Record = {}; + +{ + await using proc = Bun.spawn({ cmd, env, stdin: input, stdout: "pipe", stderr: "inherit" }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + results.spawn = { stdout: stdout.trim(), exitCode }; +} + +{ + const { stdout, exitCode } = Bun.spawnSync({ cmd, env, stdin: input, stdout: "pipe", stderr: "inherit" }); + results.spawnSync = { stdout: stdout.toString().trim(), exitCode }; +} + +{ + const { stdout, exitCode } = await $`${cmd} < ${input}`.env(env).quiet(); + results.shell = { stdout: stdout.toString().trim(), exitCode }; +} + +console.log("RESULT " + JSON.stringify(results)); diff --git a/test/js/bun/spawn/spawn-pipe-start-error.test.ts b/test/js/bun/spawn/spawn-pipe-start-error.test.ts index d239f9ca925d..b28cbcc8c10f 100644 --- a/test/js/bun/spawn/spawn-pipe-start-error.test.ts +++ b/test/js/bun/spawn/spawn-pipe-start-error.test.ts @@ -1,5 +1,6 @@ -import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isDebug, isWindows } from "harness"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isDebug, isWindows, tempDir } from "harness"; +import path from "node:path"; // On Windows, when the initial uv_read_start on a subprocess stdout/stderr // pipe fails (observed from libuv as UV_EINVAL after a bad FileAccessInformation @@ -62,3 +63,96 @@ try { expect(exitCode).toBe(0); }, ); + +// The writer behind a Buffer/Blob stdin (StaticPipeWriter) issues its single +// uv_write from start(). On Windows that uv_write fails synchronously when the +// child's end of the pipe is already gone, and the writer closes itself inside +// start(): on_close runs before start() has recorded its own ref, so it releases +// nothing, and the owner (Subprocess, ShellSubprocess) drops its ref and forgets +// the writer. start() has to release its own ref when it finds the writer closed +// underneath it; without that the writer stays allocated for the rest of the +// process (created 3, freed 0 below). +// +// The security scanner's JSON writer is the third owner. It releases start()'s +// ref itself right after start() returns, and must not when start() already +// has; otherwise the release in start() is one release too many for it. +// +// The failing uv_write cannot be arranged from JS either, so the same kind of +// debug-only fault injection is used; the writers are counted through the +// create()/deinit() lines of the StaticPipeWriter debug scope. The non-injected +// variants check the counting itself and the ordinary path of the same code. +describe.skipIf(!isWindows || !isDebug)("buffer stdin writer whose uv_write fails synchronously (windows)", () => { + function writerCounts(output: string) { + return { + created: output.match(/StaticPipeWriter\(0x[0-9a-f]+\) create\(\)/g)?.length ?? 0, + freed: output.match(/StaticPipeWriter\(0x[0-9a-f]+\) deinit\(\)/g)?.length ?? 0, + }; + } + + function env(failWrite: boolean) { + return failWrite + ? { ...bunEnv, BUN_DEBUG_StaticPipeWriter: "1", BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE: "1" } + : { ...bunEnv, BUN_DEBUG_StaticPipeWriter: "1" }; + } + + test.concurrent.each([true, false])( + "Bun.spawn, Bun.spawnSync and the shell free the writer (write fails: %p)", + async failWrite => { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "buffer-stdin-owners-fixture.ts")], + env: env(failWrite), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // With the write failing the child only ever sees EOF; this also proves the + // injection fired. The fixture's Buffer is 4096 bytes. + const child = { stdout: failWrite ? "0" : "4096", exitCode: 0 }; + const result = stdout.split(/\r?\n/).find(line => line.startsWith("RESULT ")); + expect(result, stderr).toBeDefined(); + expect(JSON.parse(result!.slice("RESULT ".length))).toEqual({ spawn: child, spawnSync: child, shell: child }); + // Scoped debug logging goes to stdout; search both streams anyway. + expect(writerCounts(stdout + stderr)).toEqual({ created: 3, freed: 3 }); + expect(exitCode).toBe(0); + }, + ); + + test.concurrent.each([true, false])( + "bun install frees the security scanner's JSON writer (write fails: %p)", + async failWrite => { + using dir = tempDir("scanner-json-writer", { + "package.json": JSON.stringify({ name: "scanner-json-writer", version: "1.0.0" }), + "bunfig.toml": `[install.security]\nscanner = "./scanner.js"\n`, + // bun install runs the scanner even with nothing to scan, so no registry is needed. + "scanner.js": `module.exports = { + scanner: { + version: "1", + scan: async ({ packages }) => { + console.log("scanner received " + packages.length + " packages"); + return []; + }, + }, + };`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: env(failWrite), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + if (failWrite) { + // The scanner reads its package list from the pipe the failed write closed, + // so it gets an empty document; this also proves the injection fired. + expect(stderr).toContain("Failed to parse packages JSON"); + } else { + expect(stdout).toContain("scanner received 0 packages"); + } + expect(writerCounts(stdout + stderr)).toEqual({ created: 1, freed: 1 }); + expect(exitCode).toBe(failWrite ? 1 : 0); + }, + ); +}); From 44eb98e720610a33c78b8c3b80e2a466b4e0f552 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:47:45 +0000 Subject: [PATCH 2/3] Drop the cfg-stripped label in the buffered write path and document the POSIX start unsafe block --- src/io/PipeWriter.rs | 26 +++++++++++++++----------- src/spawn/static_pipe_writer.rs | 3 ++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index e238324d63e9..5feb0ce65a2d 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -1646,6 +1646,18 @@ impl WindowsBufferedWriter { Self::r(this).on_write_complete(uv::ReturnCode::zero()); } + /// Debug-only fault injection (`BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE`) for + /// test/js/bun/spawn/spawn-pipe-start-error.test.ts: stands in for a + /// `uv_write` that fails synchronously, which JS cannot arrange. + fn injected_write_error() -> Option { + #[cfg(debug_assertions)] + if bun_core::env_var::feature_flag::BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE.get() == Some(true) + { + return Some(sys::Error::from_code(sys::E::PIPE, sys::Tag::write)); + } + None + } + pub fn write(&mut self) { let buffer = self.get_buffer_internal(); // if we are already done or if we have some pending payload we just wait until next write @@ -1716,17 +1728,9 @@ impl WindowsBufferedWriter { self.pending_payload_size = buffer_len; self.write_buffer = write_buf; let self_ptr = self as *mut Self; - let write_err = 'issue: { - // Debug-only fault injection for - // test/js/bun/spawn/spawn-pipe-start-error.test.ts: a uv_write - // that fails synchronously on a freshly spawned stdio pipe (its - // other end already closed) cannot be arranged from JS. - #[cfg(debug_assertions)] - if bun_core::env_var::feature_flag::BUN_INTERNAL_FAIL_PIPE_WRITER_WRITE.get() - == Some(true) - { - break 'issue Some(sys::Error::from_code(sys::E::PIPE, sys::Tag::write)); - } + let write_err = if let Some(err) = Self::injected_write_error() { + Some(err) + } else { self.write_req // SAFETY: `p` is `self_ptr`; libuv invokes on the loop thread with no // other Rust borrow of `*p` live, so `&mut *p` is the sole alias. diff --git a/src/spawn/static_pipe_writer.rs b/src/spawn/static_pipe_writer.rs index 0c7497bead29..19c7a80e6c85 100644 --- a/src/spawn/static_pipe_writer.rs +++ b/src/spawn/static_pipe_writer.rs @@ -205,8 +205,9 @@ impl StaticPipeWriter

{ // On POSIX `StdioResult` is an `Option`. The buffered writer's // `start()` only registers the poll and never reports to the // parent, so nothing in this arm can close or free the writer. - // SAFETY: live; each borrow is confined to its own statement. + // SAFETY: live; the borrow ends before the match body runs. let fd = unsafe { (*this).stdio_result.unwrap() }; + // SAFETY: live; the borrow of the field ends when the call returns. match unsafe { (*this).writer.start(fd, true) } { bun_sys::Result::Err(err) => { // `started` stays false so no release site fires; release From 2ffd7c526be8dd67c3c557131a6dd684ef0d732a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:11:44 +0000 Subject: [PATCH 3/3] ci: retrigger