Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions src/shell/Builtin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/shell/IO.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/shell/IOReader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/shell/IOWriter.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/shell/interpreter.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/shell/subproc.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 15 additions & 0 deletions test/js/bun/shell/file-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});