Skip to content

Commit

Permalink
Fix edgecase with socketpair() impacting shell and spawn
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner committed Dec 12, 2024
1 parent fddc28d commit 28fd698
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 65 deletions.
5 changes: 5 additions & 0 deletions src/StandaloneModuleGraph.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,11 @@ pub const StandaloneModuleGraph = struct {

const cleanup = struct {
pub fn toClean(name: [:0]const u8, fd: bun.FileDescriptor) void {
// Ensure we own the file
if (Environment.isPosix) {
// Make the file writable so we can delete it
_ = Syscall.fchmod(fd, 0o777);
}
_ = Syscall.close(fd);
_ = Syscall.unlink(name);
}
Expand Down
53 changes: 15 additions & 38 deletions src/bun.js/api/bun/process.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ pub const PosixSpawnOptions = struct {
windows: void = {},
argv0: ?[*:0]const u8 = null,
stream: bool = true,
sync: bool = false,

/// Apple Extension: If this bit is set, rather
/// than returning to the caller, posix_spawn(2)
Expand Down Expand Up @@ -998,6 +999,7 @@ pub const WindowsSpawnResult = struct {
stderr: StdioResult = .unavailable,
extra_pipes: std.ArrayList(StdioResult) = std.ArrayList(StdioResult).init(bun.default_allocator),
stream: bool = true,
sync: bool = false,

pub const StdioResult = union(enum) {
/// inherit, ignore, path, pipe
Expand Down Expand Up @@ -1240,8 +1242,7 @@ pub fn spawnProcessPosix(
var to_set_cloexec = std.ArrayList(bun.FileDescriptor).init(allocator);
defer {
for (to_set_cloexec.items) |fd| {
const fcntl_flags = bun.sys.fcntl(fd, std.posix.F.GETFD, 0).unwrap() catch continue;
_ = bun.sys.fcntl(fd, std.posix.F.SETFD, bun.C.FD_CLOEXEC | fcntl_flags);
_ = bun.sys.setCloseOnExec(fd);
}
to_set_cloexec.clearAndFree();

Expand Down Expand Up @@ -1323,24 +1324,9 @@ pub fn spawnProcessPosix(
}

const fds: [2]bun.FileDescriptor = brk: {
var fds_: [2]std.c.fd_t = undefined;
const rc = std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &fds_);
if (rc != 0) {
return error.SystemResources;
}

{
const before = std.c.fcntl(fds_[if (i == 0) 1 else 0], std.posix.F.GETFD);
_ = std.c.fcntl(fds_[if (i == 0) 1 else 0], std.posix.F.SETFD, before | std.posix.FD_CLOEXEC);
}
const pair = try bun.sys.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, .blocking).unwrap();

if (comptime Environment.isMac) {
// SO_NOSIGPIPE
const before: i32 = 1;
_ = std.c.setsockopt(fds_[if (i == 0) 1 else 0], std.posix.SOL.SOCKET, std.posix.SO.NOSIGPIPE, &before, @sizeOf(c_int));
}

break :brk .{ bun.toFD(fds_[if (i == 0) 1 else 0]), bun.toFD(fds_[if (i == 0) 0 else 1]) };
break :brk .{ bun.toFD(pair[if (i == 0) 1 else 0]), bun.toFD(pair[if (i == 0) 0 else 1]) };
};

if (i == 0) {
Expand Down Expand Up @@ -1381,6 +1367,10 @@ pub fn spawnProcessPosix(
try to_close_at_end.append(fds[1]);
try to_close_on_error.append(fds[0]);

if (!options.sync) {
try bun.sys.setNonblocking(fds[0]).unwrap();
}

try actions.dup2(fds[1], fileno);
if (fds[1] != fileno)
try actions.close(fds[1]);
Expand Down Expand Up @@ -1414,25 +1404,12 @@ pub fn spawnProcessPosix(
try actions.open(fileno, path, bun.O.RDWR | bun.O.CREAT, 0o664);
},
.ipc, .buffer => {
const fds: [2]bun.FileDescriptor = brk: {
var fds_: [2]std.c.fd_t = undefined;
const rc = std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &fds_);
if (rc != 0) {
return error.SystemResources;
}

// enable non-block
var before = std.c.fcntl(fds_[0], std.posix.F.GETFD);

_ = std.c.fcntl(fds_[0], std.posix.F.SETFD, before | bun.C.FD_CLOEXEC);

if (comptime Environment.isMac) {
// SO_NOSIGPIPE
_ = std.c.setsockopt(fds_[if (i == 0) 1 else 0], std.posix.SOL.SOCKET, std.posix.SO.NOSIGPIPE, &before, @sizeOf(c_int));
}

break :brk .{ bun.toFD(fds_[0]), bun.toFD(fds_[1]) };
};
const fds: [2]bun.FileDescriptor = try bun.sys.socketpair(
std.posix.AF.UNIX,
std.posix.SOCK.STREAM,
0,
.nonblocking,
).unwrap();

try to_close_at_end.append(fds[1]);
try to_close_on_error.append(fds[0]);
Expand Down
33 changes: 9 additions & 24 deletions src/shell/interpreter.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3574,31 +3574,16 @@ pub const Interpreter = struct {
pipe[0] = bun.FDImpl.fromUV(fds[0]).encode();
pipe[1] = bun.FDImpl.fromUV(fds[1]).encode();
} else {
const fds: [2]bun.FileDescriptor = brk: {
var fds_: [2]std.c.fd_t = undefined;
const rc = std.c.socketpair(std.posix.AF.UNIX, std.posix.SOCK.STREAM, 0, &fds_);
if (rc != 0) {
return bun.sys.Maybe(void).errno(bun.sys.getErrno(rc), .socketpair);
}

var before = std.c.fcntl(fds_[0], std.posix.F.GETFL);

const result = std.c.fcntl(fds_[0], std.posix.F.SETFL, before | bun.O.CLOEXEC);
if (result == -1) {
_ = bun.sys.close(bun.toFD(fds_[0]));
_ = bun.sys.close(bun.toFD(fds_[1]));
return Maybe(void).errno(bun.sys.getErrno(result), .fcntl);
}

if (comptime bun.Environment.isMac) {
// SO_NOSIGPIPE
before = 1;
_ = std.c.setsockopt(fds_[0], std.posix.SOL.SOCKET, std.posix.SO.NOSIGPIPE, &before, @sizeOf(c_int));
}

break :brk .{ bun.toFD(fds_[0]), bun.toFD(fds_[1]) };
const fds: [2]bun.FileDescriptor = switch (bun.sys.socketpair(
std.posix.AF.UNIX,
std.posix.SOCK.STREAM,
0,
.blocking,
)) {
.result => |fds| .{ bun.toFD(fds[0]), bun.toFD(fds[1]) },
.err => |err| return .{ .err = err },
};
log("socketpair() = {{{}, {}}}", .{ fds[0], fds[1] });

pipe.* = fds;
}
set_count.* += 1;
Expand Down
140 changes: 137 additions & 3 deletions src/sys.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ pub const Tag = enum(u8) {
pipe,
try_write,
socketpair,
setsockopt,

uv_spawn,
uv_pipe,
Expand Down Expand Up @@ -751,9 +752,16 @@ pub fn mkdirOSPath(file_path: bun.OSPathSliceZ, flags: bun.Mode) Maybe(void) {

const fnctl_int = if (Environment.isLinux) usize else c_int;
pub fn fcntl(fd: bun.FileDescriptor, cmd: i32, arg: fnctl_int) Maybe(fnctl_int) {
const result = fcntl_symbol(fd.cast(), cmd, arg);
if (Maybe(fnctl_int).errnoSys(result, .fcntl)) |err| return err;
return .{ .result = @intCast(result) };
while (true) {
const result = fcntl_symbol(fd.cast(), cmd, arg);
if (Maybe(fnctl_int).errnoSys(result, .fcntl)) |err| {
if (err.getErrno() == .INTR) continue;
return err;
}
return .{ .result = @intCast(result) };
}

unreachable;
}

pub fn getErrno(rc: anytype) bun.C.E {
Expand Down Expand Up @@ -2308,6 +2316,132 @@ pub fn mmapFile(path: [:0]const u8, flags: std.c.MAP, wanted_size: ?usize, offse
return .{ .result = map };
}

pub fn setCloseOnExec(fd: bun.FileDescriptor) Maybe(void) {
switch (fcntl(fd, std.posix.F.GETFD, 0)) {
.result => |fl| {
switch (fcntl(fd, std.posix.F.SETFD, fl | std.posix.FD_CLOEXEC)) {
.result => {},
.err => |err| return .{ .err = err },
}
},
.err => |err| return .{ .err = err },
}

return .{ .result = {} };
}

pub fn setsockopt(fd: bun.FileDescriptor, level: c_int, optname: u32, value: i32) Maybe(i32) {
while (true) {
const rc = syscall.setsockopt(fd.cast(), level, optname, &value, @sizeOf(i32));
if (Maybe(i32).errnoSys(rc, .setsockopt)) |err| {
if (err.getErrno() == .INTR) continue;
log("setsockopt() = {d} {s}", .{ err.err.errno, err.err.name() });
return err;
}
log("setsockopt({d}, {d}, {d}) = {d}", .{ fd.cast(), level, optname, rc });
return .{ .result = @intCast(rc) };
}

unreachable;
}

pub fn setNoSigpipe(fd: bun.FileDescriptor) Maybe(void) {
if (comptime Environment.isMac) {
return switch (setsockopt(fd, std.posix.SOL.SOCKET, std.posix.SO.NOSIGPIPE, 1)) {
.result => .{ .result = {} },
.err => |err| .{ .err = err },
};
}

return .{ .result = {} };
}

const socketpair_t = if (Environment.isLinux) i32 else c_uint;

/// libc socketpair() except it defaults to:
/// - SOCK_CLOEXEC on Linux
/// - SO_NOSIGPIPE on macOS
///
/// On POSIX it otherwise makes it do O_CLOEXEC.
pub fn socketpair(domain: socketpair_t, socktype: socketpair_t, protocol: socketpair_t, nonblocking_status: enum { blocking, nonblocking }) Maybe([2]bun.FileDescriptor) {
if (comptime !Environment.isPosix) @compileError("linux only!");

var fds_i: [2]syscall.fd_t = .{ 0, 0 };

if (comptime Environment.isLinux) {
while (true) {
const nonblock_flag: i32 = if (nonblocking_status == .nonblocking) linux.SOCK.NONBLOCK else 0;
const rc = std.os.linux.socketpair(domain, socktype | linux.SOCK.CLOEXEC | nonblock_flag, protocol, &fds_i);
if (Maybe(void).errnoSys(rc, .socketpair)) |err| {
if (err.getErrno() == .INTR) continue;

log("socketpair() = {d} {s}", .{ err.err.errno, err.err.name() });
return .{ .err = err };
}

break;
}
} else {
while (true) {
const err = libc.socketpair(domain, socktype, protocol, &fds_i);

if (Maybe([2]bun.FileDescriptor).errnoSys(err, .socketpair)) |err2| {
if (err2.getErrno() == .INTR) continue;
log("socketpair() = {d} {s}", .{ err2.err.errno, err2.err.name() });
return err2;
}

break;
}

const err: ?Syscall.Error = err: {

// Set O_CLOEXEC first.
inline for (0..2) |i| {
switch (setCloseOnExec(bun.toFD(fds_i[i]))) {
.err => |err| break :err err,
.result => {},
}
}

if (comptime Environment.isMac) {
inline for (0..2) |i| {
switch (setNoSigpipe(bun.toFD(fds_i[i]))) {
.err => |err| break :err err,
else => {},
}
}
}

if (nonblocking_status == .nonblocking) {
inline for (0..2) |i| {
switch (setNonblocking(bun.toFD(fds_i[i]))) {
.err => |err| break :err err,
.result => {},
}
}
}

break :err null;
};

// On any error after socketpair(), we need to close it.
if (err) |errr| {
inline for (0..2) |i| {
_ = close(bun.toFD(fds_i[i]));
}

log("socketpair() = {d} {s}", .{ errr.errno, errr.name() });

return .{ .err = errr };
}
}

log("socketpair() = [{d} {d}]", .{ fds_i[0], fds_i[1] });

return Maybe([2]bun.FileDescriptor){ .result = .{ bun.toFD(fds_i[0]), bun.toFD(fds_i[1]) } };
}

pub fn munmap(memory: []align(mem.page_size) const u8) Maybe(void) {
if (Maybe(void).errnoSys(syscall.munmap(memory.ptr, memory.len), .munmap)) |err| {
return err;
Expand Down

0 comments on commit 28fd698

Please sign in to comment.