From 854c076ff7560de9b3f05ee15e1e30a90d738839 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 3 Jan 2026 20:10:13 -0800 Subject: [PATCH] std.Io.Threaded: improve posix spawning * avoid unreachable when the OS does something unexpected * make waiting for the fork/exec error report cancelable --- lib/std/Io/Threaded.zig | 188 +++++++++++++++++++++++----------- lib/std/posix.zig | 112 -------------------- lib/std/posix/test.zig | 31 ------ test/standalone/posix/cwd.zig | 6 +- 4 files changed, 129 insertions(+), 208 deletions(-) diff --git a/lib/std/Io/Threaded.zig b/lib/std/Io/Threaded.zig index e654afaacd59613542e94509dedccca9bd8888dc..6efd50aa0a1b8f0feaa042ae91a552b7ecc1f34f 100644 --- a/lib/std/Io/Threaded.zig +++ b/lib/std/Io/Threaded.zig @@ -1456,8 +1456,8 @@ pub fn io(t: *Threaded) Io { .processReplacePath = processReplacePath, .processSpawn = processSpawn, .processSpawnPath = processSpawnPath, - .childWait = childWait, // TODO audit for cancelation and unreachable - .childKill = childKill, // TODO audit for cancelation and unreachable + .childWait = childWait, + .childKill = childKill, .progressParentFile = progressParentFile, @@ -11853,38 +11853,7 @@ fn processSetCurrentDir(userdata: ?*anyopaque, dir: Dir) process.SetCurrentDirEr }; } - if (dir.handle == posix.AT.FDCWD) return; - - const syscall: Syscall = try .start(); - while (true) { - switch (posix.errno(posix.system.fchdir(dir.handle))) { - .SUCCESS => return syscall.finish(), - .INTR => { - try syscall.checkCancel(); - continue; - }, - .ACCES => { - syscall.finish(); - return error.AccessDenied; - }, - .BADF => |err| { - syscall.finish(); - return errnoBug(err); - }, - .NOTDIR => { - syscall.finish(); - return error.NotDir; - }, - .IO => { - syscall.finish(); - return error.FileSystem; - }, - else => |err| { - syscall.finish(); - return posix.unexpectedErrno(err); - }, - } - } + return fchdir(dir.handle); } pub const PosixAddress = extern union { @@ -12960,57 +12929,64 @@ fn spawnPosix(t: *Threaded, options: process.SpawnOptions) process.SpawnError!Sp }; if (pid_result == 0) { - // We are the child. + defer comptime unreachable; // We are the child. if (Thread.current) |current_thread| current_thread.cancel_protection = .blocked; + const ep1 = err_pipe[1]; - setUpChildIo(options.stdin, stdin_pipe[0], posix.STDIN_FILENO, dev_null_fd) catch |err| forkBail(err_pipe[1], err); - setUpChildIo(options.stdout, stdout_pipe[1], posix.STDOUT_FILENO, dev_null_fd) catch |err| forkBail(err_pipe[1], err); - setUpChildIo(options.stderr, stderr_pipe[1], posix.STDERR_FILENO, dev_null_fd) catch |err| forkBail(err_pipe[1], err); + setUpChildIo(options.stdin, stdin_pipe[0], posix.STDIN_FILENO, dev_null_fd) catch |err| forkBail(ep1, err); + setUpChildIo(options.stdout, stdout_pipe[1], posix.STDOUT_FILENO, dev_null_fd) catch |err| forkBail(ep1, err); + setUpChildIo(options.stderr, stderr_pipe[1], posix.STDERR_FILENO, dev_null_fd) catch |err| forkBail(ep1, err); if (options.cwd_dir) |cwd| { - posix.fchdir(cwd.handle) catch |err| forkBail(err_pipe[1], err); + fchdir(cwd.handle) catch |err| forkBail(ep1, err); } else if (options.cwd) |cwd| { - posix.chdir(cwd) catch |err| forkBail(err_pipe[1], err); + chdir(cwd) catch |err| forkBail(ep1, err); } // Must happen after fchdir above, the cwd file descriptor might be // equal to prog_fileno and be clobbered by this dup2 call. - if (prog_pipe[1] != -1) posix.dup2(prog_pipe[1], prog_fileno) catch |err| forkBail(err_pipe[1], err); + if (prog_pipe[1] != -1) dup2(prog_pipe[1], prog_fileno) catch |err| forkBail(ep1, err); if (options.gid) |gid| { - posix.setregid(gid, gid) catch |err| forkBail(err_pipe[1], err); + switch (posix.errno(posix.system.setregid(gid, gid))) { + .SUCCESS => {}, + .AGAIN => forkBail(ep1, error.ResourceLimitReached), + .INVAL => forkBail(ep1, error.InvalidUserId), + .PERM => forkBail(ep1, error.PermissionDenied), + else => forkBail(ep1, error.Unexpected), + } } if (options.uid) |uid| { switch (posix.errno(posix.system.setreuid(uid, uid))) { .SUCCESS => {}, - .AGAIN => forkBail(err_pipe[1], error.ResourceLimitReached), - .INVAL => forkBail(err_pipe[1], error.InvalidUserId), - .PERM => forkBail(err_pipe[1], error.PermissionDenied), - else => forkBail(err_pipe[1], error.Unexpected), + .AGAIN => forkBail(ep1, error.ResourceLimitReached), + .INVAL => forkBail(ep1, error.InvalidUserId), + .PERM => forkBail(ep1, error.PermissionDenied), + else => forkBail(ep1, error.Unexpected), } } if (options.pgid) |pid| { switch (posix.errno(posix.system.setpgid(0, pid))) { .SUCCESS => {}, - .ACCES => forkBail(err_pipe[1], error.ProcessAlreadyExec), - .INVAL => forkBail(err_pipe[1], error.InvalidProcessGroupId), - .PERM => forkBail(err_pipe[1], error.PermissionDenied), - else => forkBail(err_pipe[1], error.Unexpected), + .ACCES => forkBail(ep1, error.ProcessAlreadyExec), + .INVAL => forkBail(ep1, error.InvalidProcessGroupId), + .PERM => forkBail(ep1, error.PermissionDenied), + else => forkBail(ep1, error.Unexpected), } } if (options.start_suspended) { switch (posix.errno(posix.system.kill(posix.system.getpid(), .STOP))) { .SUCCESS => {}, - .PERM => forkBail(err_pipe[1], error.PermissionDenied), - else => forkBail(err_pipe[1], error.Unexpected), + .PERM => forkBail(ep1, error.PermissionDenied), + else => forkBail(ep1, error.Unexpected), } } const err = posixExecv(options.expand_arg0, argv_buf.ptr[0].?, argv_buf.ptr, envp, PATH); - forkBail(err_pipe[1], err); + forkBail(ep1, err); } const pid: posix.pid_t = @intCast(pid_result); // We are the parent. @@ -13050,9 +13026,10 @@ fn getDevNullFd(t: *Threaded) !posix.fd_t { defer t.mutex.unlock(); if (t.null_file.fd != -1) return t.null_file.fd; } + const mode: u32 = 0; const syscall: Syscall = try .start(); while (true) { - const rc = open_sym("/dev/null", .{ .ACCMODE = .RDWR }, 0); + const rc = open_sym("/dev/null", .{ .ACCMODE = .RDWR }, mode); switch (posix.errno(rc)) { .SUCCESS => { syscall.finish(); @@ -13089,10 +13066,15 @@ fn processSpawnPosix(userdata: ?*anyopaque, options: process.SpawnOptions) proce defer posix.close(spawned.err_fd); // Wait for the child to report any errors in or before `execvpe`. - if (readIntFd(t, spawned.err_fd)) |child_err_int| { + if (readIntFd(spawned.err_fd)) |child_err_int| { const child_err: process.SpawnError = @errorCast(@errorFromInt(child_err_int)); return child_err; } else |read_err| switch (read_err) { + error.Canceled => { + // We don't want to wait for the error to be reported, but we do + // need to return the child so that it can be cleaned up. + recancelInner(); + }, error.EndOfStream => { // Write end closed by CLOEXEC at the time of the `execvpe` call, // indicating success. @@ -13159,7 +13141,7 @@ fn childKillWindows(t: *Threaded, child: *process.Child, exit_code: windows.UINT fn childWaitWindows(child: *process.Child) process.Child.WaitError!process.Child.Term { const handle = child.id.?; - var syscall: Syscall = try .start(); + const syscall: Syscall = try .start(); while (true) switch (windows.kernel32.WaitForSingleObjectEx(handle, windows.INFINITE, windows.FALSE)) { windows.WAIT_OBJECT_0 => break syscall.finish(), windows.WAIT_ABANDONED, windows.WAIT_TIMEOUT => { @@ -13393,21 +13375,25 @@ fn writeIntFd(fd: posix.fd_t, value: ErrInt) !void { } } -fn readIntFd(t: *Threaded, fd: posix.fd_t) !ErrInt { - _ = t; // TODO cancelation +fn readIntFd(fd: posix.fd_t) !ErrInt { var buffer: [8]u8 = undefined; var i: usize = 0; + const syscall: Syscall = try .start(); while (true) { const rc = posix.system.read(fd, buffer[i..].ptr, buffer.len - i); switch (posix.errno(rc)) { .SUCCESS => { + syscall.finish(); const n: usize = @intCast(rc); if (n == 0) break; i += n; continue; }, - .INTR => continue, - else => |err| return posix.unexpectedErrno(err), + .INTR => { + try syscall.checkCancel(); + continue; + }, + else => |err| return syscall.unexpectedErrno(err), } } if (buffer.len - i != 0) return error.EndOfStream; @@ -13423,10 +13409,10 @@ fn destroyPipe(pipe: [2]posix.fd_t) void { fn setUpChildIo(stdio: process.SpawnOptions.StdIo, pipe_fd: i32, std_fileno: i32, dev_null_fd: i32) !void { switch (stdio) { - .pipe => try posix.dup2(pipe_fd, std_fileno), + .pipe => try dup2(pipe_fd, std_fileno), .close => posix.close(std_fileno), .inherit => {}, - .ignore => try posix.dup2(dev_null_fd, std_fileno), + .ignore => try dup2(dev_null_fd, std_fileno), .file => @panic("TODO implement setUpChildIo when file is used"), } } @@ -15289,3 +15275,81 @@ pub fn pipe2(flags: posix.O) PipeError![2]posix.fd_t { return fds; } + +pub const DupError = error{ + ProcessFdQuotaExceeded, + SystemResources, +} || Io.UnexpectedError || Io.Cancelable; + +pub fn dup2(old_fd: posix.fd_t, new_fd: posix.fd_t) DupError!void { + const syscall: Syscall = try .start(); + while (true) switch (posix.errno(posix.system.dup2(old_fd, new_fd))) { + .SUCCESS => return syscall.finish(), + .BUSY, .INTR => { + try syscall.checkCancel(); + continue; + }, + .INVAL => |err| return syscall.errnoBug(err), // invalid parameters + .BADF => |err| return syscall.errnoBug(err), // use after free + .MFILE => return syscall.fail(error.ProcessFdQuotaExceeded), + .NOMEM => return syscall.fail(error.SystemResources), + else => |err| return syscall.unexpectedErrno(err), + }; +} + +pub const FchdirError = error{ + AccessDenied, + NotDir, + FileSystem, +} || Io.Cancelable || Io.UnexpectedError; + +pub fn fchdir(fd: posix.fd_t) FchdirError!void { + if (fd == posix.AT.FDCWD) return; + const syscall: Syscall = try .start(); + while (true) switch (posix.errno(posix.system.fchdir(fd))) { + .SUCCESS => return syscall.finish(), + .INTR => { + try syscall.checkCancel(); + continue; + }, + .ACCES => return syscall.fail(error.AccessDenied), + .NOTDIR => return syscall.fail(error.NotDir), + .IO => return syscall.fail(error.FileSystem), + .BADF => |err| return syscall.errnoBug(err), + else => |err| return syscall.unexpectedErrno(err), + }; +} + +pub const ChdirError = error{ + AccessDenied, + FileSystem, + SymLinkLoop, + NameTooLong, + FileNotFound, + SystemResources, + NotDir, + BadPathName, +} || Io.Cancelable || Io.UnexpectedError; + +pub fn chdir(dir_path: []const u8) ChdirError!void { + var path_buffer: [posix.PATH_MAX]u8 = undefined; + const dir_path_posix = try pathToPosix(dir_path, &path_buffer); + const syscall: Syscall = try .start(); + while (true) switch (posix.errno(posix.system.chdir(dir_path_posix))) { + .SUCCESS => return syscall.finish(), + .INTR => { + try syscall.checkCancel(); + continue; + }, + .ACCES => return syscall.fail(error.AccessDenied), + .IO => return syscall.fail(error.FileSystem), + .LOOP => return syscall.fail(error.SymLinkLoop), + .NAMETOOLONG => return syscall.fail(error.NameTooLong), + .NOENT => return syscall.fail(error.FileNotFound), + .NOMEM => return syscall.fail(error.SystemResources), + .NOTDIR => return syscall.fail(error.NotDir), + .ILSEQ => return syscall.fail(error.BadPathName), + .FAULT => |err| return syscall.errnoBug(err), + else => |err| return syscall.unexpectedErrno(err), + }; +} diff --git a/lib/std/posix.zig b/lib/std/posix.zig index fed55f34cfb748744853c300380223b125286332..736dcf824f50a4b89ab63d15079702725828f828 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -772,29 +772,6 @@ pub fn openatZ(dir_fd: fd_t, file_path: [*:0]const u8, flags: O, mode: mode_t) O } } -pub fn dup(old_fd: fd_t) !fd_t { - const rc = system.dup(old_fd); - return switch (errno(rc)) { - .SUCCESS => return @intCast(rc), - .MFILE => error.ProcessFdQuotaExceeded, - .BADF => unreachable, // invalid file descriptor - else => |err| return unexpectedErrno(err), - }; -} - -pub fn dup2(old_fd: fd_t, new_fd: fd_t) !void { - while (true) { - switch (errno(system.dup2(old_fd, new_fd))) { - .SUCCESS => return, - .BUSY, .INTR => continue, - .MFILE => return error.ProcessFdQuotaExceeded, - .INVAL => unreachable, // invalid parameters passed to dup2 - .BADF => unreachable, // invalid file descriptor - else => |err| return unexpectedErrno(err), - } - } -} - pub fn getppid() pid_t { return system.getppid(); } @@ -832,85 +809,6 @@ pub fn getcwd(out_buffer: []u8) GetCwdError![]u8 { } } -/// Same as `mkdir` but the parameter is null-terminated. -/// On Windows, `dir_path` should be encoded as [WTF-8](https://wtf-8.codeberg.page/). -/// On WASI, `dir_path` should be encoded as valid UTF-8. -/// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding. -pub const ChangeCurDirError = error{ - AccessDenied, - FileSystem, - SymLinkLoop, - NameTooLong, - FileNotFound, - SystemResources, - NotDir, - /// WASI: file paths must be valid UTF-8. - /// Windows: file paths provided by the user must be valid WTF-8. - /// https://wtf-8.codeberg.page/ - BadPathName, -} || UnexpectedError; - -/// Changes the current working directory of the calling process. -/// On Windows, `dir_path` should be encoded as [WTF-8](https://wtf-8.codeberg.page/). -/// On WASI, `dir_path` should be encoded as valid UTF-8. -/// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding. -pub fn chdir(dir_path: []const u8) ChangeCurDirError!void { - if (native_os == .wasi and !builtin.link_libc) { - @compileError("unsupported OS"); - } else if (native_os == .windows) { - @compileError("unsupported OS"); - } else { - const dir_path_c = try toPosixPath(dir_path); - return chdirZ(&dir_path_c); - } -} - -/// Same as `chdir` except the parameter is null-terminated. -/// On Windows, `dir_path` should be encoded as [WTF-8](https://wtf-8.codeberg.page/). -/// On WASI, `dir_path` should be encoded as valid UTF-8. -/// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding. -pub fn chdirZ(dir_path: [*:0]const u8) ChangeCurDirError!void { - if (native_os == .windows) { - @compileError("unsupported OS"); - } else if (native_os == .wasi and !builtin.link_libc) { - @compileError("unsupported OS"); - } - switch (errno(system.chdir(dir_path))) { - .SUCCESS => return, - .ACCES => return error.AccessDenied, - .FAULT => unreachable, - .IO => return error.FileSystem, - .LOOP => return error.SymLinkLoop, - .NAMETOOLONG => return error.NameTooLong, - .NOENT => return error.FileNotFound, - .NOMEM => return error.SystemResources, - .NOTDIR => return error.NotDir, - .ILSEQ => return error.BadPathName, - else => |err| return unexpectedErrno(err), - } -} - -pub const FchdirError = error{ - AccessDenied, - NotDir, - FileSystem, -} || UnexpectedError; - -pub fn fchdir(dirfd: fd_t) FchdirError!void { - if (dirfd == AT.FDCWD) return; - while (true) { - switch (errno(system.fchdir(dirfd))) { - .SUCCESS => return, - .ACCES => return error.AccessDenied, - .BADF => unreachable, - .NOTDIR => return error.NotDir, - .INTR => continue, - .IO => return error.FileSystem, - else => |err| return unexpectedErrno(err), - } - } -} - pub const SetEidError = error{ InvalidUserId, PermissionDenied, @@ -956,16 +854,6 @@ pub fn setegid(uid: uid_t) SetEidError!void { } } -pub fn setregid(rgid: gid_t, egid: gid_t) SetIdError!void { - switch (errno(system.setregid(rgid, egid))) { - .SUCCESS => return, - .AGAIN => return error.ResourceLimitReached, - .INVAL => return error.InvalidUserId, - .PERM => return error.PermissionDenied, - else => |err| return unexpectedErrno(err), - } -} - pub fn getuid() uid_t { return system.getuid(); } diff --git a/lib/std/posix/test.zig b/lib/std/posix/test.zig index f385a82663ff95be31d1e361eb7580fb857abb46..905b3438e4636898fb9b6e4db795346d27901d04 100644 --- a/lib/std/posix/test.zig +++ b/lib/std/posix/test.zig @@ -433,37 +433,6 @@ test "sigset add/del" { } } -test "dup & dup2" { - switch (native_os) { - .linux, .illumos => {}, - else => return error.SkipZigTest, - } - - const io = testing.io; - - var tmp = tmpDir(.{}); - defer tmp.cleanup(); - - { - var file = try tmp.dir.createFile(io, "os_dup_test", .{}); - defer file.close(io); - - var duped = Io.File{ .handle = try posix.dup(file.handle) }; - defer duped.close(io); - try duped.writeStreamingAll(io, "dup"); - - // Tests aren't run in parallel so using the next fd shouldn't be an issue. - const new_fd = duped.handle + 1; - try posix.dup2(file.handle, new_fd); - var dup2ed = Io.File{ .handle = new_fd }; - defer dup2ed.close(io); - try dup2ed.writeStreamingAll(io, "dup2"); - } - - var buffer: [8]u8 = undefined; - try expectEqualStrings("dupdup2", try tmp.dir.readFile(io, "os_dup_test", &buffer)); -} - test "getpid" { if (native_os == .wasi) return error.SkipZigTest; if (native_os == .windows) return error.SkipZigTest; diff --git a/test/standalone/posix/cwd.zig b/test/standalone/posix/cwd.zig index fd5ceae1ec6472fe8aaa41a67c5fb22fae297025..d43713247595bf1a2206a6a84e5f37b52bfc6af3 100644 --- a/test/standalone/posix/cwd.zig +++ b/test/standalone/posix/cwd.zig @@ -31,7 +31,7 @@ fn test_chdir_self() !void { const old_cwd = try std.posix.getcwd(old_cwd_buf[0..]); // Try changing to the current directory - try std.posix.chdir(old_cwd); + try std.Io.Threaded.chdir(old_cwd); try expect_cwd(old_cwd); } @@ -42,7 +42,7 @@ fn test_chdir_absolute() !void { const parent = std.fs.path.dirname(old_cwd) orelse unreachable; // old_cwd should be absolute // Try changing to the parent via a full path - try std.posix.chdir(parent); + try std.Io.Threaded.chdir(parent); try expect_cwd(parent); } @@ -63,7 +63,7 @@ fn test_chdir_relative(gpa: Allocator, io: Io) !void { defer gpa.free(expected_path); // change current working directory to new test directory - try std.posix.chdir(relative_dir_name); + try std.Io.Threaded.chdir(relative_dir_name); var new_cwd_buf: [path_max]u8 = undefined; const new_cwd = try std.posix.getcwd(new_cwd_buf[0..]); -- 2.54.0