io: leave the fd with the caller when a POSIX pipe writer fails to start - #38354
Open
robobun wants to merge 3 commits into
Open
io: leave the fd with the caller when a POSIX pipe writer fails to start#38354robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Aug 14, 2026 in 34m 31s
Code review found 1 important issue
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/spawn/static_pipe_writer.rs:183-189 |
Subprocess leaks after failed StaticPipeWriter::start() because on_close no longer fires |
| 🟡 Nit | src/io/pipes.rs:132-136 |
close_without_closing_fd() closes the fd on Windows despite its name |
Annotations
Check failure on line 189 in src/spawn/static_pipe_writer.rs
claude / Claude Code Review
Subprocess leaks after failed StaticPipeWriter::start() because on_close no longer fires
This trades the double-close for a permanent leak of the `Subprocess` (and its stdin source buffer) on the `Bun.spawn` Buffer/Blob-stdin path: after `start()` fails, `self.writer.handle` is now `PollOrFd::Closed`, so when the killed child exits and `on_process_exit` calls `buffer.close()`, `close_impl` sees `get_fd() == INVALID` and never invokes `on_close_fn` — `on_close_io(Stdin)` never swaps `stdin` from `Writable::Buffer` to `Ignore`, so `update_has_pending_activity()` keeps `this_value` Str
Check warning on line 136 in src/io/pipes.rs
claude / Claude Code Review
close_without_closing_fd() closes the fd on Windows despite its name
nit: `close_impl` explicitly discards `close_fd` on Windows (`#[cfg(windows)] let _ = close_fd;`, line 62) and unconditionally calls `Closer::close(fd, ...)` there, so this function would close the fd on Windows despite its name and doc comment. Unreachable today (only callers are the `Posix*` writers, and Windows writers use `Source` not `PollOrFd`), but gating with `#[cfg(not(windows))]` would match its callers and keep the name honest.
Loading