From 8178600aad72567191b593e67d97de52dc347059 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 17 Dec 2025 15:53:33 -0800 Subject: [PATCH 1/2] fix(shell): prevent double-close of fd when using &> redirect with builtins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using `&>` to redirect both stdout and stderr to the same file with a shell builtin command (e.g., `pwd &> file.txt`), the code was creating two separate IOWriter instances that shared the same file descriptor. When both IOWriters were destroyed, they both tried to close the same fd, causing an EBADF (bad file descriptor) error. The fix: 1. Share a single IOWriter between stdout and stderr when both are redirected to the same file, with proper reference counting 2. Rename `refSelf` to `dupeRef` for clarity across IOReader, IOWriter, CowFd, and add it to Blob for consistency 3. Fix the Body.Value blob case to also properly reference count when the same blob is assigned to multiple outputs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Latest model --- src/shell/Builtin.zig | 39 +++++++++++++++++++++++++++++++-------- src/shell/IO.zig | 2 +- src/shell/IOReader.zig | 2 +- src/shell/IOWriter.zig | 2 +- src/shell/interpreter.zig | 2 +- src/shell/subproc.zig | 2 +- 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/shell/Builtin.zig b/src/shell/Builtin.zig index a5713ca804d2..b29ff2400947 100644 --- a/src/shell/Builtin.zig +++ b/src/shell/Builtin.zig @@ -264,6 +264,11 @@ pub const BuiltinIO = struct { ref_count: RefCount, blob: bun.webcore.Blob, + fn dupeRef(this: *Blob) *Blob { + this.ref(); + return this; + } + fn deinit(this: *Blob) void { this.blob.deinit(); bun.destroy(this); @@ -340,16 +345,16 @@ pub fn init( io: *IO, ) ?Yield { const stdin: BuiltinIO.Input = switch (io.stdin) { - .fd => |fd| .{ .fd = fd.refSelf() }, + .fd => |fd| .{ .fd = fd.dupeRef() }, .ignore => .ignore, }; const stdout: BuiltinIO.Output = switch (io.stdout) { - .fd => |val| .{ .fd = .{ .writer = val.writer.refSelf(), .captured = val.captured } }, + .fd => |val| .{ .fd = .{ .writer = val.writer.dupeRef(), .captured = val.captured } }, .pipe => .{ .buf = std.array_list.Managed(u8).init(cmd.base.allocator()) }, .ignore => .ignore, }; const stderr: BuiltinIO.Output = switch (io.stderr) { - .fd => |val| .{ .fd = .{ .writer = val.writer.refSelf(), .captured = val.captured } }, + .fd => |val| .{ .fd = .{ .writer = val.writer.dupeRef(), .captured = val.captured } }, .pipe => .{ .buf = std.array_list.Managed(u8).init(cmd.base.allocator()) }, .ignore => .ignore, }; @@ -481,13 +486,26 @@ fn initRedirections( cmd.exec.bltn.stdin.deref(); cmd.exec.bltn.stdin = .{ .fd = IOReader.init(redirfd, cmd.base.eventLoop()) }; } + + if (!node.redirect.stdout and !node.redirect.stderr) { + return null; + } + + const redirect_writer: *IOWriter = .init( + redirfd, + .{ .pollable = is_pollable, .nonblocking = is_nonblocking, .is_socket = is_socket }, + cmd.base.eventLoop(), + ); + defer redirect_writer.deref(); + if (node.redirect.stdout) { cmd.exec.bltn.stdout.deref(); - cmd.exec.bltn.stdout = .{ .fd = .{ .writer = IOWriter.init(redirfd, .{ .pollable = is_pollable, .nonblocking = is_nonblocking, .is_socket = is_socket }, cmd.base.eventLoop()) } }; + cmd.exec.bltn.stdout = .{ .fd = .{ .writer = redirect_writer.dupeRef() } }; } + if (node.redirect.stderr) { cmd.exec.bltn.stderr.deref(); - cmd.exec.bltn.stderr = .{ .fd = .{ .writer = IOWriter.init(redirfd, .{ .pollable = is_pollable, .nonblocking = is_nonblocking, .is_socket = is_socket }, cmd.base.eventLoop()) } }; + cmd.exec.bltn.stderr = .{ .fd = .{ .writer = redirect_writer.dupeRef() } }; } }, .jsbuf => |val| { @@ -522,24 +540,29 @@ fn initRedirections( var original_blob = body.use(); defer original_blob.deinit(); + if (!node.redirect.stdin and !node.redirect.stdout and !node.redirect.stderr) { + return null; + } + const blob: *BuiltinIO.Blob = bun.new(BuiltinIO.Blob, .{ .ref_count = .init(), .blob = original_blob.dupe(), }); + defer blob.deref(); if (node.redirect.stdin) { cmd.exec.bltn.stdin.deref(); - cmd.exec.bltn.stdin = .{ .blob = blob }; + cmd.exec.bltn.stdin = .{ .blob = blob.dupeRef() }; } if (node.redirect.stdout) { cmd.exec.bltn.stdout.deref(); - cmd.exec.bltn.stdout = .{ .blob = blob }; + cmd.exec.bltn.stdout = .{ .blob = blob.dupeRef() }; } if (node.redirect.stderr) { cmd.exec.bltn.stderr.deref(); - cmd.exec.bltn.stderr = .{ .blob = blob }; + cmd.exec.bltn.stderr = .{ .blob = blob.dupeRef() }; } } else if (interpreter.jsobjs[file.jsbuf.idx].as(jsc.WebCore.Blob)) |blob| { if ((node.redirect.stdout or node.redirect.stderr) and !blob.needsToReadFile()) { diff --git a/src/shell/IO.zig b/src/shell/IO.zig index dc5730026f9f..b95520424adf 100644 --- a/src/shell/IO.zig +++ b/src/shell/IO.zig @@ -170,7 +170,7 @@ pub const OutKind = union(enum) { fn to_subproc_stdio(this: OutKind, shellio: *?*shell.IOWriter) bun.shell.subproc.Stdio { return switch (this) { .fd => |val| brk: { - shellio.* = val.writer.refSelf(); + shellio.* = val.writer.dupeRef(); break :brk if (val.captured) |cap| .{ .capture = .{ .buf = cap, diff --git a/src/shell/IOReader.zig b/src/shell/IOReader.zig index 15ffb786e175..540b81cbc772 100644 --- a/src/shell/IOReader.zig +++ b/src/shell/IOReader.zig @@ -30,7 +30,7 @@ const InitFlags = packed struct(u8) { __unused: u5 = 0, }; -pub fn refSelf(this: *IOReader) *IOReader { +pub fn dupeRef(this: *IOReader) *IOReader { this.ref(); return this; } diff --git a/src/shell/IOWriter.zig b/src/shell/IOWriter.zig index 51aa360d0081..868570ee9a79 100644 --- a/src/shell/IOWriter.zig +++ b/src/shell/IOWriter.zig @@ -70,7 +70,7 @@ pub const Poll = WriterImpl; // pub fn __onClose(_: *IOWriter) void {} // pub fn __flush(_: *IOWriter) void {} -pub fn refSelf(this: *IOWriter) *IOWriter { +pub fn dupeRef(this: *IOWriter) *IOWriter { this.ref(); return this; } diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 723b2c05cbeb..d4a1d6aa502d 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -177,7 +177,7 @@ pub const CowFd = struct { this.refcount += 1; } - pub fn refSelf(this: *CowFd) *CowFd { + pub fn dupeRef(this: *CowFd) *CowFd { this.ref(); return this; } diff --git a/src/shell/subproc.zig b/src/shell/subproc.zig index b4e6bb08e468..0e28c43ec63b 100644 --- a/src/shell/subproc.zig +++ b/src/shell/subproc.zig @@ -1121,7 +1121,7 @@ pub const PipeReader = struct { log("PipeReader(0x{x}, {s}) create()", .{ @intFromPtr(this), @tagName(this.out_type) }); if (capture) |cap| { - this.captured_writer.writer = cap.refSelf(); + this.captured_writer.writer = cap.dupeRef(); this.captured_writer.dead = false; } From 2eebcbf06d61ee1e0562cf103c1b070674a2be06 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 17 Dec 2025 15:57:58 -0800 Subject: [PATCH 2/2] test(shell): add tests for &> redirect with builtins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests that verify the fix for the double-close fd bug when using &> redirect with builtin commands like pwd and echo. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Latest model --- test/js/bun/shell/file-io.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/js/bun/shell/file-io.test.ts b/test/js/bun/shell/file-io.test.ts index 6c8f8e87e3a9..09d8e8c55faa 100644 --- a/test/js/bun/shell/file-io.test.ts +++ b/test/js/bun/shell/file-io.test.ts @@ -140,4 +140,19 @@ describe("IOWriter file output redirection", () => { .fileEquals("pipe_output.txt", "pipe test\n") .runAsTest("pipe with file redirection"); }); + + describe("&> redirect (stdout and stderr to same file)", () => { + // This test verifies the fix for the bug where using &> with a builtin + // command caused the same file descriptor to be closed twice, resulting + // in an EBADF error. The issue was that two separate IOWriter instances + // were created for the same fd when both stdout and stderr were redirected. + TestBuilder.command`pwd &> pwd_output.txt`.exitCode(0).runAsTest("builtin pwd with &> redirect"); + + TestBuilder.command`echo "hello" &> echo_output.txt` + .exitCode(0) + .fileEquals("echo_output.txt", "hello\n") + .runAsTest("builtin echo with &> redirect"); + + TestBuilder.command`pwd &>> append_output.txt`.exitCode(0).runAsTest("builtin pwd with &>> append redirect"); + }); });