Fix file descriptor leak in Bun.spawn()

src/bun.js/api/bun/spawn.zig

@@ -88,6 +88,9 @@ pub const PosixSpawn = struct {
     pub const Actions = struct {
         actions: system.posix_spawn_file_actions_t,
 
+        // Ensure we only close once.
+        close_list: std.ArrayListUnmanaged(fd_t) = .{},
+
         pub fn init() !Actions {
             var actions: system.posix_spawn_file_actions_t = undefined;
             switch (errno(system.posix_spawn_file_actions_init(&actions))) {
@@ -106,6 +109,8 @@ pub const PosixSpawn = struct {
                 _ = system.posix_spawn_file_actions_destroy(&self.actions);
             }
 
+            self.close_list.deinit(bun.default_allocator);
+
             self.* = undefined;
         }
 
@@ -126,6 +131,9 @@ pub const PosixSpawn = struct {
         }
 
         pub fn close(self: *Actions, fd: fd_t) !void {
+            if (std.mem.indexOfScalar(fd_t, self.close_list.items, fd) != null) return;
+            self.close_list.append(bun.default_allocator, fd) catch {};
+
             switch (errno(system.posix_spawn_file_actions_addclose(&self.actions, fd))) {
                 .SUCCESS => return,
                 .BADF => return error.InvalidFileDescriptor,

src/bun.js/api/bun/subprocess.zig

@@ -2436,6 +2436,10 @@ pub const Subprocess = struct {
         }
 
         pub fn useMemfd(this: *@This(), index: u32) void {
+            if (comptime !Environment.isLinux) {
+                return;
+            }
+
             const label = switch (index) {
                 0 => "spawn_stdio_stdin",
                 1 => "spawn_stdio_stdout",
@@ -2444,19 +2448,7 @@ pub const Subprocess = struct {
             };
 
             // We use the linux syscall api because the glibc requirement is 2.27, which is a little close for comfort.
-            const rc = std.os.linux.memfd_create(label, 0);
-
-            log("memfd_create({s}) = {d}", .{ label, rc });
-
-            switch (std.os.linux.getErrno(rc)) {
-                .SUCCESS => {},
-                else => |errno| {
-                    log("Failed to create memfd: {s}", .{@tagName(errno)});
-                    return;
-                },
-            }
-
-            const fd = bun.toFD(rc);
+            const fd = bun.sys.memfd_create(label, 0).unwrap() catch return;
 
             var remain = this.byteSlice();
 
@@ -2518,12 +2510,15 @@ pub const Subprocess = struct {
 
                     try actions.dup2(pipe_fd[idx], std_fileno);
                     try actions.close(pipe_fd[1 - idx]);
+                    try actions.close(pipe_fd[idx]);
                 },
                 .fd => |fd| {
                     try actions.dup2(fd, std_fileno);
+                    try actions.close(fd);
                 },
                 .memfd => |fd| {
                     try actions.dup2(fd, std_fileno);
+                    try actions.close(fd);
                 },
                 .path => |pathlike| {
                     const flag = if (std_fileno == bun.STDIN_FD) @as(u32, os.O.RDONLY) else @as(u32, std.os.O.WRONLY);

src/install/lifecycle_script_runner.zig

@@ -22,7 +22,7 @@ pub const LifecycleScriptSubprocess = struct {
     pid: std.os.pid_t = bun.invalid_fd,
 
     pid_poll: *Async.FilePoll,
-    waitpid_result: ?PosixSpawn.WaitPidResult,
+    waitpid_result: ?PosixSpawn.WaitPidResult = null,
     stdout: OutputReader = .{},
     stderr: OutputReader = .{},
     manager: *PackageManager,
@@ -195,6 +195,7 @@ pub const LifecycleScriptSubprocess = struct {
             combined_script,
             null,
         };
+
         // Have both stdout and stderr write to the same buffer
         const fdsOut, const fdsErr = if (!this.manager.options.log_level.isVerbose())
             .{ try std.os.pipe2(0), try std.os.pipe2(0) }
@@ -207,6 +208,27 @@ pub const LifecycleScriptSubprocess = struct {
         }
 
         const pid = brk: {
+            var close_all = false;
+
+            defer {
+                if (close_all) {
+                    if (fdsOut[0] != 0)
+                        _ = bun.sys.close(fdsOut[0]);
+                    if (fdsErr[0] != 0)
+                        _ = bun.sys.close(fdsErr[0]);
+                }
+
+                if (!this.manager.options.log_level.isVerbose() or close_all) {
+                    if (fdsOut[1] != 0)
+                        _ = bun.sys.close(fdsOut[1]);
+                    if (fdsErr[1] != 0)
+                        _ = bun.sys.close(fdsErr[1]);
+                }
+            }
+            errdefer {
+                close_all = true;
+            }
+
             var attr = try PosixSpawn.Attr.init();
             defer attr.deinit();
             try attr.set(@intCast(flags));
@@ -219,6 +241,11 @@ pub const LifecycleScriptSubprocess = struct {
             if (!this.manager.options.log_level.isVerbose()) {
                 try actions.dup2(fdsOut[1], bun.STDOUT_FD);
                 try actions.dup2(fdsErr[1], bun.STDERR_FD);
+
+                if (fdsOut[0] != 0) try actions.close(fdsOut[0]);
+                if (fdsErr[0] != 0) try actions.close(fdsErr[0]);
+                if (fdsOut[1] != 0) try actions.close(fdsOut[1]);
+                if (fdsErr[1] != 0) try actions.close(fdsErr[1]);
             } else {
                 if (comptime Environment.isMac) {
                     try actions.inherit(bun.STDOUT_FD);
@@ -231,13 +258,6 @@ pub const LifecycleScriptSubprocess = struct {
 
             try actions.chdir(cwd);
 
-            defer {
-                if (!this.manager.options.log_level.isVerbose()) {
-                    _ = bun.sys.close(fdsOut[1]);
-                    _ = bun.sys.close(fdsErr[1]);
-                }
-            }
-
             if (manager.options.log_level.isVerbose()) {
                 Output.prettyErrorln("<d>[LifecycleScriptSubprocess]<r> Spawning <b>\"{s}\"<r> script for package <b>\"{s}\"<r>", .{
                     this.scriptName(),
@@ -261,6 +281,7 @@ pub const LifecycleScriptSubprocess = struct {
                         @tagName(err.getErrno()),
                     });
                     Output.flush();
+                    close_all = true;
                     return;
                 },
                 .result => |pid| break :brk pid,
@@ -574,9 +595,14 @@ pub const LifecycleScriptSubprocess = struct {
         comptime log_level: PackageManager.Options.LogLevel,
     ) !void {
         var lifecycle_subprocess = try manager.allocator.create(LifecycleScriptSubprocess);
-        lifecycle_subprocess.scripts = list.items;
-        lifecycle_subprocess.manager = manager;
-        lifecycle_subprocess.envp = envp;
+        lifecycle_subprocess.* = .{
+            .manager = manager,
+            .envp = envp,
+            .scripts = list.items,
+            .package_name = "",
+            .pid_poll = undefined,
+            .waitpid_result = null,
+        };
 
         if (comptime log_level.isVerbose()) {
             Output.prettyErrorln("<d>[LifecycleScriptSubprocess]<r> Starting scripts for <b>\"{s}\"<r>", .{

src/linux_memfd_allocator.zig

@@ -131,27 +131,19 @@ pub const LinuxMemFdAllocator = struct {
             unreachable;
         }
 
-        const rc = brk: {
+        const fd = brk: {
             var label_buf: [128]u8 = undefined;
             const label = std.fmt.bufPrintZ(&label_buf, "memfd-num-{d}", .{memfd_counter.fetchAdd(1, .Monotonic)}) catch "";
 
             // Using huge pages was slower.
-            const code = std.os.linux.memfd_create(label.ptr, std.os.linux.MFD.CLOEXEC | 0);
+            const code = bun.sys.memfd_create(label.ptr, std.os.linux.MFD.CLOEXEC | 0);
+            if (code == .err) {
+                return .{ .err = code.err };
+            }
 
-            bun.sys.syslog("memfd_create({s}) = {d}", .{ label, code });
-            break :brk code;
+            break :brk code.result;
         };
 
-        switch (std.os.linux.getErrno(rc)) {
-            .SUCCESS => {},
-            else => |errno| {
-                bun.sys.syslog("Failed to create memfd: {s}", .{@tagName(errno)});
-                return .{ .err = bun.sys.Error.fromCode(errno, .open) };
-            },
-        }
-
-        const fd = bun.toFD(rc);
-
         if (bytes.len > 0)
             // Hint at the size of the file
             _ = bun.sys.ftruncate(fd, @intCast(bytes.len));

src/sys.zig

@@ -147,6 +147,20 @@ const open_sym = system.open;
 
 const mem = std.mem;
 
+pub fn memfd_create(label: ?[*:0]const u8, flags: u32) Maybe(bun.FileDescriptor) {
+    if (comptime !Environment.isLinux) {
+        @compileError("memfd_create is only supported on Linux");
+    }
+
+    const rc = linux.memfd_create(label orelse "", flags);
+
+    log("memfd_create({s}) = {d}", .{ label orelse "null", rc });
+
+    if (Maybe(bun.FileDescriptor).errnoSys(rc, .open)) |err| return err;
+
+    return Maybe(bun.FileDescriptor){ .result = bun.toFD(rc) };
+}
+
 pub fn getcwd(buf: *[bun.MAX_PATH_BYTES]u8) Maybe([]const u8) {
     const Result = Maybe([]const u8);
     buf[0] = 0;