From f4e01b0e489e6d1439117b07199af2881bc5bd7f Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Mon, 29 Jul 2024 00:38:37 +0200 Subject: [PATCH 01/12] std.process.Child: detached process spawning --- lib/std/c.zig | 2 ++ lib/std/os/linux.zig | 4 +++ lib/std/os/windows.zig | 13 ++++++++ lib/std/os/windows/kernel32.zig | 7 ++++ lib/std/posix.zig | 23 +++++++++++++ lib/std/process/Child.zig | 59 ++++++++++++++++++++++++++++++--- 6 files changed, 103 insertions(+), 5 deletions(-) diff --git a/lib/std/c.zig b/lib/std/c.zig index ff2c7dbd85bc..a8597d3127bc 100644 --- a/lib/std/c.zig +++ b/lib/std/c.zig @@ -9168,6 +9168,7 @@ pub extern "c" fn setregid(rgid: gid_t, egid: gid_t) c_int; pub extern "c" fn setresuid(ruid: uid_t, euid: uid_t, suid: uid_t) c_int; pub extern "c" fn setresgid(rgid: gid_t, egid: gid_t, sgid: gid_t) c_int; pub extern "c" fn setpgid(pid: pid_t, pgid: pid_t) c_int; +pub extern "c" fn setsid() pid_t; pub extern "c" fn malloc(usize) ?*anyopaque; pub extern "c" fn realloc(?*anyopaque, usize) ?*anyopaque; @@ -9351,6 +9352,7 @@ pub extern "c" fn setlogmask(maskpri: c_int) c_int; pub extern "c" fn if_nametoindex([*:0]const u8) c_int; pub extern "c" fn getpid() pid_t; +pub extern "c" fn getsid(pid: pid_t) pid_t; /// These are implementation defined but share identical values in at least musl and glibc: /// - https://git.musl-libc.org/cgit/musl/tree/include/locale.h?id=ab31e9d6a0fa7c5c408856c89df2dfb12c344039#n18 diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 4f3db110b261..56caf40dd31e 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -1581,6 +1581,10 @@ pub fn setsid() pid_t { return @bitCast(@as(u32, @truncate(syscall0(.setsid)))); } +pub fn getsid(pid: pid_t) pid_t { + return @bitCast(@as(u32, @truncate(syscall1(.getsid, @intCast(pid))))); +} + pub fn getpid() pid_t { return @bitCast(@as(u32, @truncate(syscall0(.getpid)))); } diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index e06a3ff02cd9..fd7fc0685804 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -154,6 +154,12 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN } } +pub fn GetProcessId(Process: HANDLE) UnexpectedError!DWORD { + const ret_code = kernel32.GetProcessId(Process); + if (ret_code == 0) return unexpectedError(GetLastError()); + return ret_code; +} + pub fn GetCurrentProcess() HANDLE { const process_pseudo_handle: usize = @bitCast(@as(isize, -1)); return @ptrFromInt(process_pseudo_handle); @@ -1813,6 +1819,12 @@ pub fn VirtualQuery(lpAddress: ?LPVOID, lpBuffer: PMEMORY_BASIC_INFORMATION, dwL return rc; } +pub fn GetConsoleProcessList(lpdwProcessList: []DWORD) UnexpectedError!DWORD { + const ret_code = kernel32.GetConsoleProcessList(lpdwProcessList.ptr, @min(lpdwProcessList.len, math.maxInt(DWORD))); + if (ret_code == 0) return unexpectedError(GetLastError()); + return ret_code; +} + pub const SetConsoleTextAttributeError = error{Unexpected}; pub fn SetConsoleTextAttribute(hConsoleOutput: HANDLE, wAttributes: WORD) SetConsoleTextAttributeError!void { @@ -3727,6 +3739,7 @@ pub const COORD = extern struct { Y: SHORT, }; +pub const DETACHED_PROCESS = 8; pub const CREATE_UNICODE_ENVIRONMENT = 1024; pub const TLS_OUT_OF_INDEXES = 4294967295; diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index b8ae72af5571..769a7e07f5e3 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -340,6 +340,8 @@ pub extern "kernel32" fn GetExitCodeProcess( // TODO: Already a wrapper for this, see `windows.GetCurrentProcess`. pub extern "kernel32" fn GetCurrentProcess() callconv(WINAPI) HANDLE; +pub extern "kernel32" fn GetProcessId(Process: HANDLE) callconv(WINAPI) DWORD; + // TODO: memcpy peb().ProcessParameters.Environment, mem.span(0). Requires locking the PEB. pub extern "kernel32" fn GetEnvironmentStringsW() callconv(WINAPI) ?LPWSTR; @@ -519,6 +521,11 @@ pub extern "kernel32" fn ReadConsoleOutputCharacterW( lpNumberOfCharsRead: *DWORD, ) callconv(windows.WINAPI) BOOL; +pub extern "kernel32" fn GetConsoleProcessList( + lpdwProcessList: [*]DWORD, + dwProcessCount: DWORD, +) callconv(WINAPI) DWORD; + // Memory Mapping/Allocation // TODO: Wrapper around RtlCreateHeap. diff --git a/lib/std/posix.zig b/lib/std/posix.zig index e04efbbcc061..9448e7ec797d 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -3438,6 +3438,29 @@ pub fn setpgid(pid: pid_t, pgid: pid_t) SetPgidError!void { } } +pub const SetSidError = error{PermissionDenied} || UnexpectedError; + +pub fn setsid() SetSidError!pid_t { + const res = system.setsid(); + switch (errno(@as(isize, res))) { + .SUCCESS => return res, + .PERM => return error.PermissionDenied, + else => |err| return unexpectedErrno(err), + } +} + +pub const GetSidError = error{ProcessNotFound} || SetSidError; + +pub fn getsid(pid: pid_t) GetSidError!pid_t { + const res = system.getsid(pid); + switch (errno(@as(isize, res))) { + .SUCCESS => return res, + .PERM => return error.PermissionDenied, + .SRCH => return error.ProcessNotFound, + else => |err| return unexpectedErrno(err), + } +} + /// Test whether a file descriptor refers to a terminal. pub fn isatty(handle: fd_t) bool { if (native_os == .windows) { diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index dfe0c6578eb5..a0293277f8a0 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -57,6 +57,9 @@ stdin_behavior: StdIo, stdout_behavior: StdIo, stderr_behavior: StdIo, +/// Set to spawn a detached process. +detached: bool, + /// Set to change the user id when spawning the child process. uid: if (native_os == .windows or native_os == .wasi) void else ?posix.uid_t, @@ -172,6 +175,7 @@ pub const SpawnError = error{ posix.ExecveError || posix.SetIdError || posix.SetPgidError || + posix.SetSidError || posix.ChangeCurDirError || windows.CreateProcessError || windows.GetProcessMemoryInfoError || @@ -215,6 +219,7 @@ pub fn init(argv: []const []const u8, allocator: mem.Allocator) ChildProcess { .term = null, .env_map = null, .cwd = null, + .detached = false, .uid = if (native_os == .windows or native_os == .wasi) {} else null, .gid = if (native_os == .windows or native_os == .wasi) {} else null, .pgid = if (native_os == .windows or native_os == .wasi) {} else null, @@ -658,6 +663,10 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void { const pid_result = try posix.fork(); if (pid_result == 0) { // we are the child + if (self.detached) { + _ = posix.setsid() catch |err| forkChildErrReport(err_pipe[1], err); + } + setUpChildIo(self.stdin_behavior, stdin_pipe[0], posix.STDIN_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err); setUpChildIo(self.stdout_behavior, stdout_pipe[1], posix.STDOUT_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err); setUpChildIo(self.stderr_behavior, stderr_pipe[1], posix.STDERR_FILENO, dev_null_fd) catch |err| forkChildErrReport(err_pipe[1], err); @@ -844,6 +853,10 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { .lpReserved2 = null, }; var piProcInfo: windows.PROCESS_INFORMATION = undefined; + var dwCreationFlags: windows.DWORD = windows.CREATE_UNICODE_ENVIRONMENT; + if (self.detached) { + dwCreationFlags |= windows.DETACHED_PROCESS; + } const cwd_w = if (self.cwd) |cwd| try unicode.wtf8ToWtf16LeAllocZ(self.allocator, cwd) else null; defer if (cwd_w) |cwd| self.allocator.free(cwd); @@ -928,7 +941,7 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { dir_buf.shrinkRetainingCapacity(normalized_len); } - windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| { + windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo, dwCreationFlags) catch |no_path_err| { const original_err = switch (no_path_err) { // argv[0] contains unsupported characters that will never resolve to a valid exe. error.InvalidArg0 => return error.FileNotFound, @@ -956,7 +969,7 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { const normalized_len = windows.normalizePath(u16, dir_buf.items) catch continue; dir_buf.shrinkRetainingCapacity(normalized_len); - if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) { + if (windowsCreateProcessPathExt(self.allocator, &dir_buf, &app_buf, PATHEXT, &cmd_line_cache, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo, dwCreationFlags)) { break :run; } else |err| switch (err) { // argv[0] contains unsupported characters that will never resolve to a valid exe. @@ -1057,6 +1070,7 @@ fn windowsCreateProcessPathExt( cwd_ptr: ?[*:0]u16, lpStartupInfo: *windows.STARTUPINFOW, lpProcessInformation: *windows.PROCESS_INFORMATION, + dwCreationFlags: windows.DWORD, ) !void { const app_name_len = app_buf.items.len; const dir_path_len = dir_buf.items.len; @@ -1205,7 +1219,7 @@ fn windowsCreateProcessPathExt( else full_app_name; - if (windowsCreateProcess(app_name_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_ptr, lpStartupInfo, lpProcessInformation)) |_| { + if (windowsCreateProcess(app_name_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_ptr, lpStartupInfo, lpProcessInformation, dwCreationFlags)) |_| { return; } else |err| switch (err) { error.FileNotFound, @@ -1260,7 +1274,7 @@ fn windowsCreateProcessPathExt( else full_app_name; - if (windowsCreateProcess(app_name_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_ptr, lpStartupInfo, lpProcessInformation)) |_| { + if (windowsCreateProcess(app_name_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_ptr, lpStartupInfo, lpProcessInformation, dwCreationFlags)) |_| { return; } else |err| switch (err) { error.FileNotFound => continue, @@ -1288,6 +1302,7 @@ fn windowsCreateProcess( cwd_ptr: ?[*:0]u16, lpStartupInfo: *windows.STARTUPINFOW, lpProcessInformation: *windows.PROCESS_INFORMATION, + dwCreationFlags: windows.DWORD, ) !void { // TODO the docs for environment pointer say: // > A pointer to the environment block for the new process. If this parameter @@ -1312,7 +1327,7 @@ fn windowsCreateProcess( null, null, windows.TRUE, - windows.CREATE_UNICODE_ENVIRONMENT, + dwCreationFlags, @as(?*anyopaque, @ptrCast(envp_ptr)), cwd_ptr, lpStartupInfo, @@ -1857,3 +1872,37 @@ fn argvToScriptCommandLineWindows( return try unicode.wtf8ToWtf16LeAllocZ(allocator, buf.items); } + +test "detached child" { + const cmd = if (native_os == .windows) + &.{ "ping", "-n", "1", "-w", "10000", "127.255.255.255" } + else &.{ "sleep", "10" }; + var child = ChildProcess.init(cmd, std.testing.allocator); + child.stdin_behavior = .Ignore; + child.stderr_behavior = .Ignore; + child.stdout_behavior = .Pipe; + child.detached = true; + try child.spawn(); + + if (native_os == .windows) { + const child_pid = try windows.GetProcessId(child.id); + + // Give the process some time to actually start doing something. + // If we check the process list immediately, we might be done before + // the new process attaches to the console. + var read_buffer: [1]u8 = undefined; + try std.testing.expectEqual(1, try child.stdout.?.read(&read_buffer)); + + var proc_buffer: [16]windows.DWORD = undefined; + const proc_count = try windows.GetConsoleProcessList(&proc_buffer); + if (proc_count > 16) return error.ProcessBufferTooSmall; + + for (proc_buffer[0..proc_count]) |proc| { + if (proc == child_pid) return error.ProcessAttachedToConsole; + } + } else { + const current_sid = try posix.getsid(0); + const child_sid = try posix.getsid(child.id); + try std.testing.expect(current_sid != child_sid); + } +} From bd91dc5883b1e66db3e614869da4c449662a585b Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Wed, 31 Jul 2024 00:15:11 +0200 Subject: [PATCH 02/12] std.process.Child: kill child after test --- lib/std/process/Child.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index a0293277f8a0..c45e98e1da61 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -1875,8 +1875,8 @@ fn argvToScriptCommandLineWindows( test "detached child" { const cmd = if (native_os == .windows) - &.{ "ping", "-n", "1", "-w", "10000", "127.255.255.255" } - else &.{ "sleep", "10" }; + &.{ "ping", "-n", "1", "-w", "30000", "127.255.255.255" } + else &.{ "sleep", "30" }; var child = ChildProcess.init(cmd, std.testing.allocator); child.stdin_behavior = .Ignore; child.stderr_behavior = .Ignore; @@ -1905,4 +1905,6 @@ test "detached child" { const child_sid = try posix.getsid(child.id); try std.testing.expect(current_sid != child_sid); } + + _ = try child.kill(); } From 7f9bb47430bde39143b3d40d5907e89702f84046 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Wed, 31 Jul 2024 16:47:42 +0200 Subject: [PATCH 03/12] std.process.Child: move detached child test to standalone --- lib/std/process/Child.zig | 36 ---------------- test/standalone/build.zig.zon | 3 ++ test/standalone/detached_child/build.zig | 32 ++++++++++++++ test/standalone/detached_child/child.zig | 19 ++++++++ test/standalone/detached_child/main.zig | 55 ++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 36 deletions(-) create mode 100644 test/standalone/detached_child/build.zig create mode 100644 test/standalone/detached_child/child.zig create mode 100644 test/standalone/detached_child/main.zig diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index c45e98e1da61..a156b2be70c1 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -1872,39 +1872,3 @@ fn argvToScriptCommandLineWindows( return try unicode.wtf8ToWtf16LeAllocZ(allocator, buf.items); } - -test "detached child" { - const cmd = if (native_os == .windows) - &.{ "ping", "-n", "1", "-w", "30000", "127.255.255.255" } - else &.{ "sleep", "30" }; - var child = ChildProcess.init(cmd, std.testing.allocator); - child.stdin_behavior = .Ignore; - child.stderr_behavior = .Ignore; - child.stdout_behavior = .Pipe; - child.detached = true; - try child.spawn(); - - if (native_os == .windows) { - const child_pid = try windows.GetProcessId(child.id); - - // Give the process some time to actually start doing something. - // If we check the process list immediately, we might be done before - // the new process attaches to the console. - var read_buffer: [1]u8 = undefined; - try std.testing.expectEqual(1, try child.stdout.?.read(&read_buffer)); - - var proc_buffer: [16]windows.DWORD = undefined; - const proc_count = try windows.GetConsoleProcessList(&proc_buffer); - if (proc_count > 16) return error.ProcessBufferTooSmall; - - for (proc_buffer[0..proc_count]) |proc| { - if (proc == child_pid) return error.ProcessAttachedToConsole; - } - } else { - const current_sid = try posix.getsid(0); - const child_sid = try posix.getsid(child.id); - try std.testing.expect(current_sid != child_sid); - } - - _ = try child.kill(); -} diff --git a/test/standalone/build.zig.zon b/test/standalone/build.zig.zon index 8e4d727642ec..48c646c6bb04 100644 --- a/test/standalone/build.zig.zon +++ b/test/standalone/build.zig.zon @@ -62,6 +62,9 @@ .child_process = .{ .path = "child_process", }, + .detached_child = .{ + .path = "detached_child", + }, .embed_generated_file = .{ .path = "embed_generated_file", }, diff --git a/test/standalone/detached_child/build.zig b/test/standalone/detached_child/build.zig new file mode 100644 index 000000000000..4299baf11624 --- /dev/null +++ b/test/standalone/detached_child/build.zig @@ -0,0 +1,32 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +pub fn build(b: *std.Build) void { + const test_step = b.step("test", "Test it"); + b.default_step = test_step; + + const optimize: std.builtin.OptimizeMode = .Debug; + const target = b.graph.host; + + if (builtin.os.tag == .wasi) return; + + const child = b.addExecutable(.{ + .name = "child", + .root_source_file = b.path("child.zig"), + .optimize = optimize, + .target = target, + }); + + const main = b.addExecutable(.{ + .name = "main", + .root_source_file = b.path("main.zig"), + .optimize = optimize, + .target = target, + }); + + const run = b.addRunArtifact(main); + run.addArtifactArg(child); + run.expectExitCode(0); + + test_step.dependOn(&run.step); +} diff --git a/test/standalone/detached_child/child.zig b/test/standalone/detached_child/child.zig new file mode 100644 index 000000000000..027f3ff5132f --- /dev/null +++ b/test/standalone/detached_child/child.zig @@ -0,0 +1,19 @@ +const std = @import("std"); + +pub fn main() !void { + var gpa_state = std.heap.GeneralPurposeAllocator(.{ .safety = true }){}; + defer if (gpa_state.deinit() == .leak) @panic("leaks were detected"); + const gpa = gpa_state.allocator(); + var args = try std.process.argsWithAllocator(gpa); + defer args.deinit(); + _ = args.next() orelse unreachable; // skip executable name + const sleep_seconds = try std.fmt.parseInt(u32, args.next() orelse unreachable, 0); + + const stdout = std.io.getStdOut(); + _ = try stdout.write("started"); + + const end_time = std.time.timestamp() + sleep_seconds; + while (std.time.timestamp() < end_time) { + std.time.sleep(@max(end_time - std.time.timestamp(), 0) * 1_000_000_000); + } +} diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig new file mode 100644 index 000000000000..6f430bddf30a --- /dev/null +++ b/test/standalone/detached_child/main.zig @@ -0,0 +1,55 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +pub fn main() !void { + var gpa_state = std.heap.GeneralPurposeAllocator(.{ .safety = true }){}; + defer if (gpa_state.deinit() == .leak) @panic("memory leak detected"); + const gpa = gpa_state.allocator(); + + var args = try std.process.argsWithAllocator(gpa); + defer args.deinit(); + _ = args.next() orelse unreachable; // skip executable name + const child_path = args.next() orelse unreachable; + + var child = std.process.Child.init(&.{ child_path, "30" }, gpa); + child.stdin_behavior = .Ignore; + child.stderr_behavior = .Inherit; + child.stdout_behavior = .Pipe; + child.detached = true; + try child.spawn(); + defer { + _ = child.kill() catch {}; + } + + switch (builtin.os.tag) { + .windows => { + const windows = std.os.windows; + const child_pid = try windows.GetProcessId(child.id); + + // Give the process some time to actually start doing something. + // If we check the process list immediately, we might be done before + // the new process attaches to the console. + var read_buffer: [1]u8 = undefined; + try std.testing.expectEqual(1, try child.stdout.?.read(&read_buffer)); + + var proc_buffer: [16]windows.DWORD = undefined; + const proc_count = try windows.GetConsoleProcessList(&proc_buffer); + if (proc_count > 16) @panic("process buffer is too small"); + + for (proc_buffer[0..proc_count]) |proc| { + if (proc == child_pid) { + return error.ProcessAttachedToConsole; + } + } + }, + else => { + const posix = std.posix; + const current_sid = try posix.getsid(0); + const child_sid = try posix.getsid(child.id); + + if (current_sid == child_sid) { + return error.SameChildSession; + } + }, + } +} From bb8776dd697de85b63f755835697ec92662f976b Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Thu, 1 Aug 2024 03:36:53 +0200 Subject: [PATCH 04/12] std.process.Child: fix failing test --- test/standalone/detached_child/main.zig | 30 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig index 6f430bddf30a..a1ce626e9250 100644 --- a/test/standalone/detached_child/main.zig +++ b/test/standalone/detached_child/main.zig @@ -21,24 +21,32 @@ pub fn main() !void { _ = child.kill() catch {}; } + // Give the process some time to actually start doing something before + // checking if it properly detached. + var read_buffer: [1]u8 = undefined; + if (try child.stdout.?.read(&read_buffer) != 1) { + return error.OutputReadFailed; + } + switch (builtin.os.tag) { .windows => { const windows = std.os.windows; const child_pid = try windows.GetProcessId(child.id); - // Give the process some time to actually start doing something. - // If we check the process list immediately, we might be done before - // the new process attaches to the console. - var read_buffer: [1]u8 = undefined; - try std.testing.expectEqual(1, try child.stdout.?.read(&read_buffer)); + var proc_buffer: []windows.DWORD = undefined; + var proc_count: windows.DWORD = 16; + while (true) { + proc_buffer = try gpa.alloc(windows.DWORD, proc_count); + defer gpa.free(proc_buffer); - var proc_buffer: [16]windows.DWORD = undefined; - const proc_count = try windows.GetConsoleProcessList(&proc_buffer); - if (proc_count > 16) @panic("process buffer is too small"); + proc_count = try windows.GetConsoleProcessList(proc_buffer); + if (proc_count == 0) return error.ConsoleProcessListFailed; - for (proc_buffer[0..proc_count]) |proc| { - if (proc == child_pid) { - return error.ProcessAttachedToConsole; + if (proc_count <= proc_buffer.len) { + for (proc_buffer[0..proc_count]) |proc| { + if (proc == child_pid) return error.ProcessAttachedToConsole; + } + break; } } }, From 647f76ccd627b857d50dc90d836b80e2ee45de97 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Sat, 3 Aug 2024 00:31:27 +0200 Subject: [PATCH 05/12] std.process.Child: return any pre-exec errors from spawn --- lib/std/process/Child.zig | 24 ++++++++++++- test/standalone/build.zig.zon | 3 ++ test/standalone/child_spawn_fail/build.zig | 32 +++++++++++++++++ test/standalone/child_spawn_fail/child.zig | 19 ++++++++++ test/standalone/child_spawn_fail/main.zig | 40 ++++++++++++++++++++++ 5 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 test/standalone/child_spawn_fail/build.zig create mode 100644 test/standalone/child_spawn_fail/child.zig create mode 100644 test/standalone/child_spawn_fail/main.zig diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index a156b2be70c1..26c14b67760b 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -233,13 +233,25 @@ pub fn init(argv: []const []const u8, allocator: mem.Allocator) ChildProcess { }; } +/// Call this if you have no intention of calling `kill` or `wait` to properly +/// dispose of any resources related to the child process. +pub fn deinit(self: *ChildProcess) void { + if (native_os == .windows) { + posix.close(self.thread_handle); + posix.close(self.id); + } + self.cleanupStreams(); +} + pub fn setUserName(self: *ChildProcess, name: []const u8) !void { const user_info = try process.getUserInfo(name); self.uid = user_info.uid; self.gid = user_info.gid; } -/// On success must call `kill` or `wait`. +/// On success must call `kill` or `wait`. In the case of a detached process, +/// consider using `deinit` instead if you have no intention of synchronizing +/// with the child. /// After spawning the `id` is available. pub fn spawn(self: *ChildProcess) SpawnError!void { if (!process.can_spawn) { @@ -693,6 +705,8 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void { posix.setpgid(0, pid) catch |err| forkChildErrReport(err_pipe[1], err); } + try writeIntFd(err_pipe[1], maxInt(ErrInt)); + const err = switch (self.expand_arg0) { .expand => posix.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_buf.ptr, envp), .no_expand => posix.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_buf.ptr, envp), @@ -701,6 +715,14 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void { } // we are the parent + + // we perform a blocking read on the err_pipe that gets us either an error + // that occured between fork and exec or a maxInt(ErrInt) if there wasn't any + const err_int = try readIntFd(err_pipe[0]); + if (err_int != maxInt(ErrInt)) { + return @as(SpawnError, @errorCast(@errorFromInt(err_int))); + } + const pid: i32 = @intCast(pid_result); if (self.stdin_behavior == .Pipe) { self.stdin = .{ .handle = stdin_pipe[1] }; diff --git a/test/standalone/build.zig.zon b/test/standalone/build.zig.zon index 48c646c6bb04..aa529cb8b554 100644 --- a/test/standalone/build.zig.zon +++ b/test/standalone/build.zig.zon @@ -65,6 +65,9 @@ .detached_child = .{ .path = "detached_child", }, + .child_spawn_fail = .{ + .path = "child_spawn_fail", + }, .embed_generated_file = .{ .path = "embed_generated_file", }, diff --git a/test/standalone/child_spawn_fail/build.zig b/test/standalone/child_spawn_fail/build.zig new file mode 100644 index 000000000000..4299baf11624 --- /dev/null +++ b/test/standalone/child_spawn_fail/build.zig @@ -0,0 +1,32 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +pub fn build(b: *std.Build) void { + const test_step = b.step("test", "Test it"); + b.default_step = test_step; + + const optimize: std.builtin.OptimizeMode = .Debug; + const target = b.graph.host; + + if (builtin.os.tag == .wasi) return; + + const child = b.addExecutable(.{ + .name = "child", + .root_source_file = b.path("child.zig"), + .optimize = optimize, + .target = target, + }); + + const main = b.addExecutable(.{ + .name = "main", + .root_source_file = b.path("main.zig"), + .optimize = optimize, + .target = target, + }); + + const run = b.addRunArtifact(main); + run.addArtifactArg(child); + run.expectExitCode(0); + + test_step.dependOn(&run.step); +} diff --git a/test/standalone/child_spawn_fail/child.zig b/test/standalone/child_spawn_fail/child.zig new file mode 100644 index 000000000000..027f3ff5132f --- /dev/null +++ b/test/standalone/child_spawn_fail/child.zig @@ -0,0 +1,19 @@ +const std = @import("std"); + +pub fn main() !void { + var gpa_state = std.heap.GeneralPurposeAllocator(.{ .safety = true }){}; + defer if (gpa_state.deinit() == .leak) @panic("leaks were detected"); + const gpa = gpa_state.allocator(); + var args = try std.process.argsWithAllocator(gpa); + defer args.deinit(); + _ = args.next() orelse unreachable; // skip executable name + const sleep_seconds = try std.fmt.parseInt(u32, args.next() orelse unreachable, 0); + + const stdout = std.io.getStdOut(); + _ = try stdout.write("started"); + + const end_time = std.time.timestamp() + sleep_seconds; + while (std.time.timestamp() < end_time) { + std.time.sleep(@max(end_time - std.time.timestamp(), 0) * 1_000_000_000); + } +} diff --git a/test/standalone/child_spawn_fail/main.zig b/test/standalone/child_spawn_fail/main.zig new file mode 100644 index 000000000000..9acb3d0d6888 --- /dev/null +++ b/test/standalone/child_spawn_fail/main.zig @@ -0,0 +1,40 @@ +const std = @import("std"); +const builtin = @import("builtin"); + +pub fn main() !void { + var gpa_state = std.heap.GeneralPurposeAllocator(.{ .safety = true }){}; + defer if (gpa_state.deinit() == .leak) @panic("memory leak detected"); + const gpa = gpa_state.allocator(); + + var args = try std.process.argsWithAllocator(gpa); + defer args.deinit(); + _ = args.next() orelse unreachable; // skip executable name + const child_path = args.next() orelse unreachable; + + var child = std.process.Child.init(&.{ child_path, "30" }, gpa); + child.stdin_behavior = .Ignore; + child.stderr_behavior = .Ignore; + child.stdout_behavior = .Pipe; + child.detached = true; + + // try to put the child in the same process group as the current session + // leader, which should fail since the child will be in a different session + child.pgid = try std.posix.getsid(0); + defer { + _ = child.kill() catch {}; + child.deinit(); + } + + if (child.spawn()) { + return error.SpawnSilencedError; + } else |_| {} + + child.deinit(); + child = std.process.Child.init(&.{ child_path, "30" }, gpa); + child.stdin_behavior = .Ignore; + child.stdout_behavior = .Ignore; + child.stderr_behavior = .Inherit; + + // this spawn should succeed and return without an error + try child.spawn(); +} From 61385ec3d6199dd4df3045e1d4a48c4a9fc823d2 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Mon, 5 Aug 2024 12:29:12 +0200 Subject: [PATCH 06/12] std.process.Child: spawn fail test on windows --- test/standalone/child_spawn_fail/main.zig | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/standalone/child_spawn_fail/main.zig b/test/standalone/child_spawn_fail/main.zig index 9acb3d0d6888..7c3f1baaafec 100644 --- a/test/standalone/child_spawn_fail/main.zig +++ b/test/standalone/child_spawn_fail/main.zig @@ -11,25 +11,21 @@ pub fn main() !void { _ = args.next() orelse unreachable; // skip executable name const child_path = args.next() orelse unreachable; - var child = std.process.Child.init(&.{ child_path, "30" }, gpa); + const argv = if (builtin.os.tag == .windows) &.{""} else &.{ child_path, "30" }; + var child = std.process.Child.init(argv, gpa); child.stdin_behavior = .Ignore; child.stderr_behavior = .Ignore; child.stdout_behavior = .Pipe; child.detached = true; - - // try to put the child in the same process group as the current session - // leader, which should fail since the child will be in a different session - child.pgid = try std.posix.getsid(0); + child.pgid = if (builtin.os.tag == .windows) void{} else try std.posix.getsid(0); defer { _ = child.kill() catch {}; - child.deinit(); } if (child.spawn()) { return error.SpawnSilencedError; } else |_| {} - child.deinit(); child = std.process.Child.init(&.{ child_path, "30" }, gpa); child.stdin_behavior = .Ignore; child.stdout_behavior = .Ignore; From b9ab14f3b48fa78a15d2558351d6d7b018758121 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Mon, 5 Aug 2024 17:15:06 +0200 Subject: [PATCH 07/12] std.process.Child: properly handle early error detection when using eventfd --- lib/std/process/Child.zig | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index 26c14b67760b..502b35a7541b 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -705,7 +705,7 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void { posix.setpgid(0, pid) catch |err| forkChildErrReport(err_pipe[1], err); } - try writeIntFd(err_pipe[1], maxInt(ErrInt)); + writeIntFd(err_pipe[1], maxInt(ErrInt)) catch {}; const err = switch (self.expand_arg0) { .expand => posix.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_buf.ptr, envp), @@ -716,11 +716,23 @@ fn spawnPosix(self: *ChildProcess) SpawnError!void { // we are the parent - // we perform a blocking read on the err_pipe that gets us either an error - // that occured between fork and exec or a maxInt(ErrInt) if there wasn't any - const err_int = try readIntFd(err_pipe[0]); + // We perform a blocking read on the err_pipe that gets us either an error + // that occured between fork and exec or a maxInt(ErrInt) if there wasn't any. + // Since we use eventfd on linux, it can happen that we get both + // a maxInt(ErrInt) and an error code in the first read if the exec in child failed + // and its error was written before this read (eventfd just sums the values up). + const err_int = blk: { + if (native_os == .linux) { + const file = File{ .handle = err_pipe[0] }; + const err_int = file.reader().readInt(u64, .little) catch return error.SystemResources; + break :blk err_int; + } else { + break :blk try readIntFd(err_pipe[0]); + } + }; if (err_int != maxInt(ErrInt)) { - return @as(SpawnError, @errorCast(@errorFromInt(err_int))); + const err = @errorFromInt(@as(ErrInt, @intCast(err_int % maxInt(ErrInt)))); + return @as(SpawnError, @errorCast(err)); } const pid: i32 = @intCast(pid_result); From b0f2258844e7fc223d72a5ae7b203f4bea8ef2ee Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Tue, 20 Aug 2024 21:57:01 +0200 Subject: [PATCH 08/12] std.process.Child: move test-specific kernel32 functions to test file --- lib/std/os/windows.zig | 12 ------------ lib/std/os/windows/kernel32.zig | 7 ------- test/standalone/detached_child/main.zig | 13 ++++++++++--- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index fd7fc0685804..55ba0504bbce 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -154,12 +154,6 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN } } -pub fn GetProcessId(Process: HANDLE) UnexpectedError!DWORD { - const ret_code = kernel32.GetProcessId(Process); - if (ret_code == 0) return unexpectedError(GetLastError()); - return ret_code; -} - pub fn GetCurrentProcess() HANDLE { const process_pseudo_handle: usize = @bitCast(@as(isize, -1)); return @ptrFromInt(process_pseudo_handle); @@ -1819,12 +1813,6 @@ pub fn VirtualQuery(lpAddress: ?LPVOID, lpBuffer: PMEMORY_BASIC_INFORMATION, dwL return rc; } -pub fn GetConsoleProcessList(lpdwProcessList: []DWORD) UnexpectedError!DWORD { - const ret_code = kernel32.GetConsoleProcessList(lpdwProcessList.ptr, @min(lpdwProcessList.len, math.maxInt(DWORD))); - if (ret_code == 0) return unexpectedError(GetLastError()); - return ret_code; -} - pub const SetConsoleTextAttributeError = error{Unexpected}; pub fn SetConsoleTextAttribute(hConsoleOutput: HANDLE, wAttributes: WORD) SetConsoleTextAttributeError!void { diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index 769a7e07f5e3..b8ae72af5571 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -340,8 +340,6 @@ pub extern "kernel32" fn GetExitCodeProcess( // TODO: Already a wrapper for this, see `windows.GetCurrentProcess`. pub extern "kernel32" fn GetCurrentProcess() callconv(WINAPI) HANDLE; -pub extern "kernel32" fn GetProcessId(Process: HANDLE) callconv(WINAPI) DWORD; - // TODO: memcpy peb().ProcessParameters.Environment, mem.span(0). Requires locking the PEB. pub extern "kernel32" fn GetEnvironmentStringsW() callconv(WINAPI) ?LPWSTR; @@ -521,11 +519,6 @@ pub extern "kernel32" fn ReadConsoleOutputCharacterW( lpNumberOfCharsRead: *DWORD, ) callconv(windows.WINAPI) BOOL; -pub extern "kernel32" fn GetConsoleProcessList( - lpdwProcessList: [*]DWORD, - dwProcessCount: DWORD, -) callconv(WINAPI) DWORD; - // Memory Mapping/Allocation // TODO: Wrapper around RtlCreateHeap. diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig index a1ce626e9250..c306ad1397d6 100644 --- a/test/standalone/detached_child/main.zig +++ b/test/standalone/detached_child/main.zig @@ -1,5 +1,12 @@ const std = @import("std"); const builtin = @import("builtin"); +const windows = std.os.windows; + +extern "kernel32" fn GetProcessId(Process: windows.HANDLE) callconv(windows.WINAPI) windows.HANDLE; +extern "kernel32" fn GetConsoleProcessList( + lpdwProcessList: [*]windows.DWORD, + dwProcessCount: windows.DWORD, +) callconv(windows.WINAPI) windows.DWORD; pub fn main() !void { var gpa_state = std.heap.GeneralPurposeAllocator(.{ .safety = true }){}; @@ -30,8 +37,8 @@ pub fn main() !void { switch (builtin.os.tag) { .windows => { - const windows = std.os.windows; - const child_pid = try windows.GetProcessId(child.id); + const child_pid = GetProcessId(child.id); + if (child_pid == 0) return error.GetProcessIdFailed; var proc_buffer: []windows.DWORD = undefined; var proc_count: windows.DWORD = 16; @@ -39,7 +46,7 @@ pub fn main() !void { proc_buffer = try gpa.alloc(windows.DWORD, proc_count); defer gpa.free(proc_buffer); - proc_count = try windows.GetConsoleProcessList(proc_buffer); + proc_count = GetConsoleProcessList(proc_buffer); if (proc_count == 0) return error.ConsoleProcessListFailed; if (proc_count <= proc_buffer.len) { From c9b7ad7efa95a1ba8610c68b2461dd659b4b0a05 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Wed, 21 Aug 2024 21:49:04 +0200 Subject: [PATCH 09/12] std.process.Child: fix incorrect return type --- test/standalone/detached_child/main.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig index c306ad1397d6..c2c83564e51e 100644 --- a/test/standalone/detached_child/main.zig +++ b/test/standalone/detached_child/main.zig @@ -2,7 +2,7 @@ const std = @import("std"); const builtin = @import("builtin"); const windows = std.os.windows; -extern "kernel32" fn GetProcessId(Process: windows.HANDLE) callconv(windows.WINAPI) windows.HANDLE; +extern "kernel32" fn GetProcessId(Process: windows.HANDLE) callconv(windows.WINAPI) windows.DWORD; extern "kernel32" fn GetConsoleProcessList( lpdwProcessList: [*]windows.DWORD, dwProcessCount: windows.DWORD, From 58cc61dbf2252b2348de60862157dffeaaf3e8de Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Fri, 23 Aug 2024 21:20:25 +0200 Subject: [PATCH 10/12] std.process.Child: fix function signature --- test/standalone/detached_child/main.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig index c2c83564e51e..16f3fb2b4a4d 100644 --- a/test/standalone/detached_child/main.zig +++ b/test/standalone/detached_child/main.zig @@ -46,7 +46,7 @@ pub fn main() !void { proc_buffer = try gpa.alloc(windows.DWORD, proc_count); defer gpa.free(proc_buffer); - proc_count = GetConsoleProcessList(proc_buffer); + proc_count = GetConsoleProcessList(proc_buffer.ptr, @min(proc_buffer.len, std.math.maxInt(windows.DWORD))); if (proc_count == 0) return error.ConsoleProcessListFailed; if (proc_count <= proc_buffer.len) { From 23b376fe413c91de5de01c8df27001b68cdb0c0e Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Sat, 29 Mar 2025 16:09:26 +0100 Subject: [PATCH 11/12] detached flag handling on windows --- lib/std/process/Child.zig | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/std/process/Child.zig b/lib/std/process/Child.zig index fe9285550533..b868823a7c68 100644 --- a/lib/std/process/Child.zig +++ b/lib/std/process/Child.zig @@ -830,10 +830,6 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { .lpReserved2 = null, }; var piProcInfo: windows.PROCESS_INFORMATION = undefined; - var dwCreationFlags: windows.DWORD = windows.CREATE_UNICODE_ENVIRONMENT; - if (self.detached) { - dwCreationFlags |= windows.DETACHED_PROCESS; - } const cwd_w = if (self.cwd) |cwd| try unicode.wtf8ToWtf16LeAllocZ(self.allocator, cwd) else null; defer if (cwd_w) |cwd| self.allocator.free(cwd); @@ -887,6 +883,7 @@ fn spawnWindows(self: *ChildProcess) SpawnError!void { .create_suspended = self.start_suspended, .create_unicode_environment = true, .create_no_window = self.create_no_window, + .detached_process = self.detached, }; run: { From 44f7c0717ff09cc02017f9ea2a8969c0742327c1 Mon Sep 17 00:00:00 2001 From: Vojtech Antosik Date: Sun, 30 Mar 2025 15:02:17 +0200 Subject: [PATCH 12/12] fix standalone test --- test/standalone/child_spawn_fail/main.zig | 6 ++++-- test/standalone/detached_child/main.zig | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/standalone/child_spawn_fail/main.zig b/test/standalone/child_spawn_fail/main.zig index 7c3f1baaafec..1f4add59e4ce 100644 --- a/test/standalone/child_spawn_fail/main.zig +++ b/test/standalone/child_spawn_fail/main.zig @@ -11,7 +11,7 @@ pub fn main() !void { _ = args.next() orelse unreachable; // skip executable name const child_path = args.next() orelse unreachable; - const argv = if (builtin.os.tag == .windows) &.{""} else &.{ child_path, "30" }; + const argv = &.{""}; var child = std.process.Child.init(argv, gpa); child.stdin_behavior = .Ignore; child.stderr_behavior = .Ignore; @@ -23,7 +23,9 @@ pub fn main() !void { } if (child.spawn()) { - return error.SpawnSilencedError; + if (child.waitForSpawn()) { + return error.SpawnSilencedError; + } else |_| {} } else |_| {} child = std.process.Child.init(&.{ child_path, "30" }, gpa); diff --git a/test/standalone/detached_child/main.zig b/test/standalone/detached_child/main.zig index 16f3fb2b4a4d..57e1735218d6 100644 --- a/test/standalone/detached_child/main.zig +++ b/test/standalone/detached_child/main.zig @@ -24,6 +24,7 @@ pub fn main() !void { child.stdout_behavior = .Pipe; child.detached = true; try child.spawn(); + try child.waitForSpawn(); defer { _ = child.kill() catch {}; }