spawn(windows): release StaticPipeWriter start() ref when buffer-stdin write completes

Merged
RO
4mo ago
+40-6

On Windows, Bun.spawn/Bun.spawnSync with a non-empty ArrayBuffer/Blob stdin leaked one StaticPipeWriter allocation per spawn.

Cause

StaticPipeWriter::start() takes a +1 and sets started = true. #30875 added a release_start_ref block to on_write() that clears started and derefs once the buffer drains, but gated it behind #[cfg(not(windows))]:

https://github.com/oven-sh/bun/blob/892b1dabc6/src/spawn/static_pipe_writer.rs#L228-L241

On Windows, when the uv_write completes before the child exits (the common ordering for a small buffer):

on_write_complete(status=0)
  -> Parent::on_write(Pending)             (release_start_ref gated out)
     -> writer.close()
        -> BaseWindowsPipeWriter::close -> on_close_source
           -> StaticPipeWriter::on_close -> Subprocess::on_close_io(Stdin)
              stdin <- Writable::Ignore; buffer.deref()   (create()'s +1)
  scopeguard deref                                         (write()'s +1)

Net: started is still true but stdin is Writable::Ignore, so the other release site, take_pending_start_writer (called from on_process_exit / close_io), matches nothing and start()'s +1 is stranded. The error arm of on_write_complete (e.g. ERROR_BROKEN_PIPE when the child closes stdin without reading) reaches on_close via close() without ever calling Parent::on_write, so start()'s +1 is stranded there the same way.

ShellSubprocess has the same leak for cmd < ${buf} stdin since its on_process_exit/close_io never look at started at all and depend entirely on on_write's release.

Fix

  • StaticPipeWriter::on_write: drop the cfg(not(windows)) gates so release_start_ref runs on every platform. WindowsBufferedWriter never passes EndOfFile, so the existing status != EndOfFile guard is a no-op there and is left for the POSIX path (PosixBufferedWriter::_on_write still touches self after the callback on EndOfFile).
  • StaticPipeWriter::on_close (Windows only): claim started and deref after on_close_io for paths that reach close() without going through on_write (the on_write_complete error arm). write()'s +1, held by that callback's scopeguard, keeps self live past the release. This is gated to Windows because on POSIX drain_buffered_data may call on_error() -> close() -> on_close and then on_write() on the same object with no extra ref held; #34697 changes that ordering and the gate can go once it lands.
  • Subprocess::take_pending_start_writer: clear started when it claims the pointer. The three release sites are then mutually exclusive via started, covering the ordering where on_process_exit closes the pipe while a uv_write whose I/O already completed is still queued (libuv delivers that callback with its real status after uv_close, not ECANCELED).
  • WindowsBufferedWriter::on_write_complete: saturating_sub for has_pending_data so the late-success-after-close ordering does not underflow in debug builds.

SecurityScanSubprocess already derefs and writes started = false immediately after start(), so both new release sites see started = false; no change in its refcount balance.

Verification

Verified on Windows x64 by instrumenting Drop with a live counter and spawning 400 children per mode: unfixed leaves 400 live writers after each of the drain (64 B into sort) and reject (256 KB into cmd /c exit) workloads; fixed returns to 0 on both, in release and debug builds.

Also verified on Windows x64: spawn.test.ts -t 'stdin|Uint8Array|Blob' (45/45), spawnSync.test.ts (6/6), file-io.test.ts (26/26), child_process.test.ts (41/41), and on Linux: bunshell.test.ts (414/414), bun-security-scanner-workspaces.test.ts (3/3). rust:check-all is green across all 10 targets.

Found while working on #35286, which scopes its change to the empty-buffer path and does not overlap.