From 0d2b44392e6a4db96776ef7dfd9dc299b35fa2f6 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Fri, 27 May 2022 10:29:13 +0200 Subject: [PATCH 01/16] std.child_process: enable non-standard pipes The user provides an array of struct with info for direction for the pipe. Each struct also contains the space for the input and output end of pipe, which simplifies handling. Typically a low number of non-standard pipes are used, so no premature optimization and complicating the usage. Also, it is expected that the upper bound of used pipes per process is comptime-known, so slicing a continuous chunk would be most efficient. * provide user with function pointer to enable parsing of necessary file descriptor on Linux and file handles on Windows in the child process - They can not be comptime-set, because the Kernel or user expects the program to work in all cases. * keep logic the same as for posix_spawn * use fcntl and SetHandleInformation to prevent file leakage into subsequently created children * test if both fcntl and HandleInformation were correctly set * test non-standard stream with 1 pipe type Note: Standard streams are expected to be inheritable by child processes, so those streams are "intentionally leaked to the child". Benchmarking with realistic workloads is needed to decide the final design. Especially, since storing space for one additionally unused pipe end can be wasteful. This is #11701 with conflicts fixed and will be used for the panic test runner. --- lib/std/build.zig | 6 +- lib/std/build/RunStep.zig | 2 +- lib/std/child_process.zig | 396 +++++++++++++++--- src/Compilation.zig | 4 +- src/link/Elf.zig | 4 +- src/link/Wasm.zig | 4 +- src/main.zig | 4 +- src/mingw.zig | 2 +- test/standalone.zig | 1 + .../childprocess_extrapipe/build.zig | 12 + .../childprocess_extrapipe/child.zig | 34 ++ .../childprocess_extrapipe/parent.zig | 77 ++++ test/tests.zig | 2 +- 13 files changed, 469 insertions(+), 79 deletions(-) create mode 100644 test/standalone/childprocess_extrapipe/build.zig create mode 100644 test/standalone/childprocess_extrapipe/child.zig create mode 100644 test/standalone/childprocess_extrapipe/parent.zig diff --git a/lib/std/build.zig b/lib/std/build.zig index 4d2b5de1f14c..39123e19631c 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -978,8 +978,8 @@ pub const Builder = struct { child.cwd = cwd; child.env_map = env_map; - const term = child.spawnAndWait() catch |err| { - log.err("Unable to spawn {s}: {s}", .{ argv[0], @errorName(err) }); + const term = child.spawnAndWait(null) catch |err| { + log.err("Unable to spawn {s}: {s}\n", .{ argv[0], @errorName(err) }); return err; }; @@ -1194,7 +1194,7 @@ pub const Builder = struct { child.stderr_behavior = stderr_behavior; child.env_map = self.env_map; - try child.spawn(); + try child.spawn(null); const stdout = child.stdout.?.reader().readAllAlloc(self.allocator, max_output_size) catch { return error.ReadFailure; diff --git a/lib/std/build/RunStep.zig b/lib/std/build/RunStep.zig index 3422b83a6d83..d867e7c0cc62 100644 --- a/lib/std/build/RunStep.zig +++ b/lib/std/build/RunStep.zig @@ -224,7 +224,7 @@ pub fn runCommand( if (print) printCmd(cwd, argv); - child.spawn() catch |err| { + child.spawn(null) catch |err| { std.debug.print("Unable to spawn {s}: {s}\n", .{ argv[0], @errorName(err) }); return err; }; diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 7bff9fe0892b..aa44d27bc681 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -1,4 +1,4 @@ -const std = @import("std.zig"); +const std = @import("std"); const builtin = @import("builtin"); const cstr = std.cstr; const unicode = std.unicode; @@ -28,6 +28,21 @@ pub const ChildProcess = struct { stdin: ?File, stdout: ?File, stderr: ?File, + /// Additional streams other than stdin, stdout, stderr + /// * unidirectional input xor output pipe for portability. + /// If non-null (= used) + /// * user must provide function to tell child process file handle + /// (stdin of child process [must be in pipe mode] or environment variables) + /// * user must process pipe handle in the child process + /// - attach the handle ie via `var file = File{.handle=handle};` + /// after parsing the integer. + /// - close it, if not needed anymore + /// - methods like readAll() read until the pipe write end is closed + /// - Non-blocking methods require message separators like ETB, EOT + /// and some more thought about message content/sanitization + /// - It is recommended to use fcntl and SetHandleInformation in the child + /// process to prevent inheritance by grandchildren + extra_streams: ?[]ExtraStream, term: ?(SpawnError!Term), @@ -79,6 +94,10 @@ pub const ChildProcess = struct { /// Windows-only. `cwd` was provided, but the path did not exist when spawning the child process. CurrentWorkingDirectoryUnlinked, } || + // POSIX-only. Non-standard streams are used and fcntl failed. + os.FcntlError || + // Non-standard streams are used and writing to stdin of child process failed. + os.WriteError || os.ExecveError || os.SetIdError || os.ChangeCurDirError || @@ -100,6 +119,22 @@ pub const ChildProcess = struct { Close, }; + pub const PipeDirection = enum { + parent_to_child, + child_to_parent, + }; + + /// ExtraStream (= pipe) is unidirected (in->out) for portability. + pub const ExtraStream = struct { + /// parent_to_child => parent writes into pipe.output + /// child_to_parent => parent reads from pipe.input + direction: PipeDirection, + /// pipe input end for reading + input: ?File, + /// pipe output end for writing + output: ?File, + }; + /// First argument in argv is the executable. pub fn init(argv: []const []const u8, allocator: mem.Allocator) ChildProcess { return .{ @@ -117,6 +152,7 @@ pub const ChildProcess = struct { .stdin = null, .stdout = null, .stderr = null, + .extra_streams = null, .stdin_behavior = StdIo.Inherit, .stdout_behavior = StdIo.Inherit, .stderr_behavior = StdIo.Inherit, @@ -130,25 +166,33 @@ pub const ChildProcess = struct { self.gid = user_info.gid; } + const ExPipeInfoProto = switch (builtin.zig_backend) { + // workaround until we replace stage1 with stage2 + .stage1 => ?fn (self: *ChildProcess) ChildProcess.SpawnError!void, + else => ?*const fn (self: *ChildProcess) ChildProcess.SpawnError!void, + }; + /// On success must call `kill` or `wait`. - pub fn spawn(self: *ChildProcess) SpawnError!void { + /// Use null xor function pointer to tell child process about extra pipes + /// See field extra_streams for more information + pub fn spawn(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { if (!std.process.can_spawn) { @compileError("the target operating system cannot spawn processes"); } if (comptime builtin.target.isDarwin()) { - return self.spawnMacos(); + return self.spawnMacos(pipe_info_fn); } if (builtin.os.tag == .windows) { - return self.spawnWindows(); + return self.spawnWindows(pipe_info_fn); } else { - return self.spawnPosix(); + return self.spawnPosix(pipe_info_fn); } } - pub fn spawnAndWait(self: *ChildProcess) SpawnError!Term { - try self.spawn(); + pub fn spawnAndWait(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!Term { + try self.spawn(pipe_info_fn); return self.wait(); } @@ -373,6 +417,7 @@ pub const ChildProcess = struct { /// Spawns a child process, waits for it, collecting stdout and stderr, and then returns. /// If it succeeds, the caller owns result.stdout and result.stderr memory. + /// Using extra pipes is not supported. pub fn exec(args: struct { allocator: mem.Allocator, argv: []const []const u8, @@ -391,7 +436,7 @@ pub const ChildProcess = struct { child.env_map = args.env_map; child.expand_arg0 = args.expand_arg0; - try child.spawn(); + try child.spawn(null); if (builtin.os.tag == .haiku) { const stdout_in = child.stdout.?.reader(); @@ -481,6 +526,8 @@ pub const ChildProcess = struct { self.term = self.cleanupAfterWait(status); } + /// Close file handles of parent process to child process, + /// if they were not closed in the meantime fn cleanupStreams(self: *ChildProcess) void { if (self.stdin) |*stdin| { stdin.close(); @@ -494,6 +541,21 @@ pub const ChildProcess = struct { stderr.close(); self.stderr = null; } + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + if (extra.*.output) |*outpipe| { + outpipe.close(); + extra.*.output = null; + } + } else { + if (extra.*.input) |*inpipe| { + inpipe.close(); + extra.*.input = null; + } + } + } + } } fn cleanupAfterWait(self: *ChildProcess, status: u32) !Term { @@ -547,7 +609,7 @@ pub const ChildProcess = struct { Term{ .Unknown = status }; } - fn spawnMacos(self: *ChildProcess) SpawnError!void { + fn spawnMacos(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0; const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined; errdefer if (self.stdin_behavior == StdIo.Pipe) destroyPipe(stdin_pipe); @@ -558,6 +620,22 @@ pub const ChildProcess = struct { const stderr_pipe = if (self.stderr_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined; errdefer if (self.stderr_behavior == StdIo.Pipe) destroyPipe(stderr_pipe); + if (self.stdin_behavior == StdIo.Pipe) { + self.stdin = File{ .handle = stdin_pipe[1] }; + } else { + self.stdin = null; + } + if (self.stdout_behavior == StdIo.Pipe) { + self.stdout = File{ .handle = stdout_pipe[0] }; + } else { + self.stdout = null; + } + if (self.stderr_behavior == StdIo.Pipe) { + self.stderr = File{ .handle = stderr_pipe[0] }; + } else { + self.stderr = null; + } + const any_ignore = (self.stdin_behavior == StdIo.Ignore or self.stdout_behavior == StdIo.Ignore or self.stderr_behavior == StdIo.Ignore); const dev_null_fd = if (any_ignore) os.openZ("/dev/null", os.O.RDWR, 0) catch |err| switch (err) { @@ -575,6 +653,26 @@ pub const ChildProcess = struct { undefined; defer if (any_ignore) os.close(dev_null_fd); + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra, i| { + std.debug.assert(extra.*.input == null); + std.debug.assert(extra.*.output == null); + + const tmp_pipe = os.pipe() catch |err| { + for (extra_streams[0..i]) |*extra_fail| { + extra_fail.*.input.?.close(); + extra_fail.*.output.?.close(); + extra_fail.*.input = null; + extra_fail.*.output = null; + } + return err; + }; + + extra.*.input = File{ .handle = tmp_pipe[0] }; + extra.*.output = File{ .handle = tmp_pipe[1] }; + } + } + var attr = try os.posix_spawn.Attr.init(); defer attr.deinit(); var flags: u16 = os.darwin.POSIX_SPAWN_SETSIGDEF | os.darwin.POSIX_SPAWN_SETSIGMASK; @@ -593,6 +691,16 @@ pub const ChildProcess = struct { try setUpChildIoPosixSpawn(self.stdout_behavior, &actions, stdout_pipe, os.STDOUT_FILENO, dev_null_fd); try setUpChildIoPosixSpawn(self.stderr_behavior, &actions, stderr_pipe, os.STDERR_FILENO, dev_null_fd); + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + try actions.close(extra.*.output.?.handle); + } else { + try actions.close(extra.*.input.?.handle); + } + } + } + if (self.cwd_dir) |cwd| { try actions.fchdir(cwd.fd); } else if (self.cwd) |cwd| { @@ -611,24 +719,14 @@ pub const ChildProcess = struct { break :m envp_buf.ptr; } else std.c.environ; - const pid = try os.posix_spawn.spawnp(self.argv[0], actions, attr, argv_buf, envp); - - if (self.stdin_behavior == StdIo.Pipe) { - self.stdin = File{ .handle = stdin_pipe[1] }; - } else { - self.stdin = null; - } - if (self.stdout_behavior == StdIo.Pipe) { - self.stdout = File{ .handle = stdout_pipe[0] }; - } else { - self.stdout = null; - } - if (self.stderr_behavior == StdIo.Pipe) { - self.stderr = File{ .handle = stderr_pipe[0] }; - } else { - self.stderr = null; + // user must communicate extra pipes to child process either via + // environment variables or stdin + if (self.extra_streams != null) { + std.debug.assert(pipe_info_fn != null); + try pipe_info_fn.?(self); } + const pid = try os.posix_spawn.spawnp(self.argv[0], actions, attr, argv_buf, envp); self.pid = pid; self.term = null; @@ -641,6 +739,26 @@ pub const ChildProcess = struct { if (self.stderr_behavior == StdIo.Pipe) { os.close(stderr_pipe[1]); } + + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + extra.*.input.?.close(); // parent does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + } else { + extra.*.output.?.close(); // parent does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + } + } + } } fn setUpChildIoPosixSpawn( @@ -662,7 +780,7 @@ pub const ChildProcess = struct { } } - fn spawnPosix(self: *ChildProcess) SpawnError!void { + fn spawnPosix(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0; const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined; errdefer if (self.stdin_behavior == StdIo.Pipe) { @@ -679,6 +797,22 @@ pub const ChildProcess = struct { destroyPipe(stderr_pipe); }; + if (self.stdin_behavior == StdIo.Pipe) { + self.stdin = File{ .handle = stdin_pipe[1] }; + } else { + self.stdin = null; + } + if (self.stdout_behavior == StdIo.Pipe) { + self.stdout = File{ .handle = stdout_pipe[0] }; + } else { + self.stdout = null; + } + if (self.stderr_behavior == StdIo.Pipe) { + self.stderr = File{ .handle = stderr_pipe[0] }; + } else { + self.stderr = null; + } + const any_ignore = (self.stdin_behavior == StdIo.Ignore or self.stdout_behavior == StdIo.Ignore or self.stderr_behavior == StdIo.Ignore); const dev_null_fd = if (any_ignore) os.openZ("/dev/null", os.O.RDWR, 0) catch |err| switch (err) { @@ -698,6 +832,26 @@ pub const ChildProcess = struct { if (any_ignore) os.close(dev_null_fd); } + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra, i| { + std.debug.assert(extra.*.input == null); + std.debug.assert(extra.*.output == null); + + const tmp_pipe = os.pipe() catch |err| { + for (extra_streams[0..i]) |*extra_fail| { + extra_fail.*.input.?.close(); + extra_fail.*.output.?.close(); + extra_fail.*.input = null; + extra_fail.*.output = null; + } + return err; + }; + + extra.*.input = File{ .handle = tmp_pipe[0] }; + extra.*.output = File{ .handle = tmp_pipe[1] }; + } + } + var arena_allocator = std.heap.ArenaAllocator.init(self.allocator); defer arena_allocator.deinit(); const arena = arena_allocator.allocator(); @@ -730,6 +884,13 @@ pub const ChildProcess = struct { } }; + // user must communicate extra pipes to child process either via + // environment variables or stdin + if (self.extra_streams != null) { + std.debug.assert(pipe_info_fn != null); + try pipe_info_fn.?(self); + } + // This pipe is used to communicate errors between the time of fork // and execve from the child process to the parent process. const err_pipe = blk: { @@ -764,6 +925,20 @@ pub const ChildProcess = struct { os.close(stderr_pipe[1]); } + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + extra.*.output.?.close(); // child does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + } else { + extra.*.input.?.close(); // child does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + } + } + } + if (self.cwd_dir) |cwd| { os.fchdir(cwd.fd) catch |err| forkChildErrReport(err_pipe[1], err); } else if (self.cwd) |cwd| { @@ -787,22 +962,6 @@ pub const ChildProcess = struct { // we are the parent const pid = @intCast(i32, pid_result); - if (self.stdin_behavior == StdIo.Pipe) { - self.stdin = File{ .handle = stdin_pipe[1] }; - } else { - self.stdin = null; - } - if (self.stdout_behavior == StdIo.Pipe) { - self.stdout = File{ .handle = stdout_pipe[0] }; - } else { - self.stdout = null; - } - if (self.stderr_behavior == StdIo.Pipe) { - self.stderr = File{ .handle = stderr_pipe[0] }; - } else { - self.stderr = null; - } - self.pid = pid; self.err_pipe = err_pipe; self.term = null; @@ -816,9 +975,31 @@ pub const ChildProcess = struct { if (self.stderr_behavior == StdIo.Pipe) { os.close(stderr_pipe[1]); } + + // close unused pipe end in parent and set CLOEXEC + // to prevent non-standard streams to leak into children + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + extra.*.input.?.close(); // parent does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + } else { + extra.*.output.?.close(); // parent does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + } + } + } } - fn spawnWindows(self: *ChildProcess) SpawnError!void { + fn spawnWindows(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { const saAttr = windows.SECURITY_ATTRIBUTES{ .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES), .bInheritHandle = windows.TRUE, @@ -876,7 +1057,12 @@ pub const ChildProcess = struct { var g_hChildStd_OUT_Wr: ?windows.HANDLE = null; switch (self.stdout_behavior) { StdIo.Pipe => { - try windowsMakeAsyncPipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr); + try windowsMakeAsyncPipe( + &g_hChildStd_OUT_Rd, + &g_hChildStd_OUT_Wr, + &saAttr, + PipeDirection.child_to_parent, + ); }, StdIo.Ignore => { g_hChildStd_OUT_Wr = nul_handle; @@ -896,7 +1082,12 @@ pub const ChildProcess = struct { var g_hChildStd_ERR_Wr: ?windows.HANDLE = null; switch (self.stderr_behavior) { StdIo.Pipe => { - try windowsMakeAsyncPipe(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr); + try windowsMakeAsyncPipe( + &g_hChildStd_ERR_Rd, + &g_hChildStd_ERR_Wr, + &saAttr, + ChildProcess.PipeDirection.child_to_parent, + ); }, StdIo.Ignore => { g_hChildStd_ERR_Wr = nul_handle; @@ -912,6 +1103,54 @@ pub const ChildProcess = struct { windowsDestroyPipe(g_hChildStd_ERR_Rd, g_hChildStd_ERR_Wr); }; + if (g_hChildStd_IN_Wr) |h| { + self.stdin = File{ .handle = h }; + } else { + self.stdin = null; + } + if (g_hChildStd_OUT_Rd) |h| { + self.stdout = File{ .handle = h }; + } else { + self.stdout = null; + } + if (g_hChildStd_ERR_Rd) |h| { + self.stderr = File{ .handle = h }; + } else { + self.stderr = null; + } + // TODO simplify to operate on file handles + errdefer { + self.stdin = null; + self.stdout = null; + self.stderr = null; + } + + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra, i| { + std.debug.assert(extra.*.input == null); + std.debug.assert(extra.*.output == null); + + var g_hChildExtra_Stream_Rd: ?windows.HANDLE = null; + var g_hChildExtra_Stream_Wr: ?windows.HANDLE = null; + windowsMakeAsyncPipe( + &g_hChildExtra_Stream_Rd, + &g_hChildExtra_Stream_Wr, + &saAttr, + extra.*.direction, + ) catch |err| { + for (extra_streams[0..i]) |*extra_fail| { + extra_fail.*.input.?.close(); + extra_fail.*.output.?.close(); + extra_fail.*.input = null; + extra_fail.*.output = null; + } + return err; + }; + extra.*.input = File{ .handle = g_hChildExtra_Stream_Rd.? }; + extra.*.output = File{ .handle = g_hChildExtra_Stream_Wr.? }; + } + } + const cmd_line = try windowsCreateCommandLine(self.allocator, self.argv); defer self.allocator.free(cmd_line); @@ -989,6 +1228,13 @@ pub const ChildProcess = struct { const cmd_line_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, cmd_line); defer self.allocator.free(cmd_line_w); + // user must communicate extra pipes to child process either via + // environment variables or stdin + if (self.extra_streams != null) { + std.debug.assert(pipe_info_fn != null); + try pipe_info_fn.?(self); + } + exec: { const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{}; const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{}; @@ -1082,6 +1328,20 @@ pub const ChildProcess = struct { if (self.stdout_behavior == StdIo.Pipe) { os.close(g_hChildStd_OUT_Wr.?); } + + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + if (extra.*.direction == .parent_to_child) { + extra.*.input.?.close(); // reading end + extra.*.input = null; + try windows.SetHandleInformation(extra.*.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + } else { + extra.*.output.?.close(); // writing end + extra.*.output = null; + try windows.SetHandleInformation(extra.*.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + } + } + } } fn setUpChildIo(stdio: StdIo, pipe_fd: i32, std_fileno: i32, dev_null_fd: i32) !void { @@ -1324,23 +1584,20 @@ fn windowsCreateProcessPathExt( } fn windowsCreateProcess(app_name: [*:0]u16, cmd_line: [*:0]u16, envp_ptr: ?[*]u16, cwd_ptr: ?[*:0]u16, lpStartupInfo: *windows.STARTUPINFOW, lpProcessInformation: *windows.PROCESS_INFORMATION) !void { - // TODO the docs for environment pointer say: - // > A pointer to the environment block for the new process. If this parameter - // > is NULL, the new process uses the environment of the calling process. - // > ... - // > An environment block can contain either Unicode or ANSI characters. If - // > the environment block pointed to by lpEnvironment contains Unicode - // > characters, be sure that dwCreationFlags includes CREATE_UNICODE_ENVIRONMENT. - // > If this parameter is NULL and the environment block of the parent process - // > contains Unicode characters, you must also ensure that dwCreationFlags - // > includes CREATE_UNICODE_ENVIRONMENT. - // This seems to imply that we have to somehow know whether our process parent passed - // CREATE_UNICODE_ENVIRONMENT if we want to pass NULL for the environment parameter. - // Since we do not know this information that would imply that we must not pass NULL - // for the parameter. - // However this would imply that programs compiled with -DUNICODE could not pass - // environment variables to programs that were not, which seems unlikely. - // More investigation is needed. + // See https://stackoverflow.com/a/4207169/9306292 + // One can manually write in unicode even if one doesn't compile in unicode + // (-DUNICODE). + // Thus CREATE_UNICODE_ENVIRONMENT, according to how one constructed the + // environment block, is necessary, since CreateProcessA and CreateProcessW may + // work with either Ansi or Unicode. + // * The environment variables can still be inherited from parent process, + // if set to NULL + // * The OS can for an unspecified environment block not figure out, + // if it is Unicode or ANSI. + // * Applications may break without specification of the environment variable + // due to inability of Windows to check (+translate) the character encodings. + // TODO This breaks applications only compatible with Ansi. + // - Clarify, if we want to add complexity to support those. return windows.CreateProcessW( app_name, cmd_line, @@ -1465,7 +1722,13 @@ fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const w var pipe_name_counter = std.atomic.Atomic(u32).init(1); -fn windowsMakeAsyncPipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { +// direction defines which side of the handle is inherited by ChildProcess +fn windowsMakeAsyncPipe( + rd: *?windows.HANDLE, + wr: *?windows.HANDLE, + sattr: *const windows.SECURITY_ATTRIBUTES, + direction: ChildProcess.PipeDirection, +) !void { var tmp_bufw: [128]u16 = undefined; // Anonymous pipes are built upon Named pipes. @@ -1520,8 +1783,11 @@ fn windowsMakeAsyncPipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *cons } errdefer os.close(write_handle); - try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); - + if (direction == .child_to_parent) { + try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); + } else { + try windows.SetHandleInformation(write_handle, windows.HANDLE_FLAG_INHERIT, 0); + } rd.* = read_handle; wr.* = write_handle; } diff --git a/src/Compilation.zig b/src/Compilation.zig index b385fa5f72ba..00c097e73368 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -4052,7 +4052,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait() catch |err| { + const term = child.spawnAndWait(null) catch |err| { return comp.failCObj(c_object, "unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); }; switch (term) { @@ -4070,7 +4070,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(); + try child.spawn(null); const stderr_reader = child.stderr.?.reader(); diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 30c88fe9b200..67a4437837bd 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1870,7 +1870,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait() catch |err| { + const term = child.spawnAndWait(null) catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); return error.UnableToSpawnSelf; }; @@ -1887,7 +1887,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(); + try child.spawn(null); const stderr = try child.stderr.?.reader().readAllAlloc(arena, 10 * 1024 * 1024); diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 346c92ebbef1..a1b1fe8058f4 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -3548,7 +3548,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait() catch |err| { + const term = child.spawnAndWait(null) catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); return error.UnableToSpawnWasm; }; @@ -3565,7 +3565,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(); + try child.spawn(null); const stderr = try child.stderr.?.reader().readAllAlloc(arena, 10 * 1024 * 1024); diff --git a/src/main.zig b/src/main.zig index ec0eb74e9338..40e8fea311f4 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3376,7 +3376,7 @@ fn runOrTest( comp_destroyed.* = true; } - const term = child.spawnAndWait() catch |err| { + const term = child.spawnAndWait(null) catch |err| { try warnAboutForeignBinaries(arena, arg_mode, target_info, link_libc); const cmd = try std.mem.join(arena, " ", argv.items); fatal("the following command failed with '{s}':\n{s}", .{ @errorName(err), cmd }); @@ -4059,7 +4059,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = try child.spawnAndWait(); + const term = try child.spawnAndWait(null); switch (term) { .Exited => |code| { if (code == 0) return cleanExit(); diff --git a/src/mingw.zig b/src/mingw.zig index a4f2f0cf9165..a6813aa0c6c3 100644 --- a/src/mingw.zig +++ b/src/mingw.zig @@ -376,7 +376,7 @@ pub fn buildImportLib(comp: *Compilation, lib_name: []const u8) !void { child.stdout_behavior = .Pipe; child.stderr_behavior = .Pipe; - try child.spawn(); + try child.spawn(null); const stderr_reader = child.stderr.?.reader(); diff --git a/test/standalone.zig b/test/standalone.zig index f2d497e6eaef..7b4e15ecf6cf 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -61,6 +61,7 @@ pub fn addCases(cases: *tests.StandaloneContext) void { (builtin.os.tag != .windows or builtin.cpu.arch != .aarch64)) { cases.addBuildFile("test/standalone/load_dynamic_library/build.zig", .{}); + cases.addBuildFile("test/standalone/childprocess_extrapipe/build.zig", .{}); } if (builtin.os.tag == .windows) { diff --git a/test/standalone/childprocess_extrapipe/build.zig b/test/standalone/childprocess_extrapipe/build.zig new file mode 100644 index 000000000000..9acbd9e04ce4 --- /dev/null +++ b/test/standalone/childprocess_extrapipe/build.zig @@ -0,0 +1,12 @@ +const Builder = @import("std").build.Builder; + +pub fn build(b: *Builder) void { + const child = b.addExecutable("child", "child.zig"); + child.install(); + const parent = b.addExecutable("parent", "parent.zig"); + parent.install(); + const run_cmd = parent.run(); + run_cmd.step.dependOn(b.getInstallStep()); + const test_step = b.step("test", "Test it"); + test_step.dependOn(&run_cmd.step); +} diff --git a/test/standalone/childprocess_extrapipe/child.zig b/test/standalone/childprocess_extrapipe/child.zig new file mode 100644 index 000000000000..fd740c186a28 --- /dev/null +++ b/test/standalone/childprocess_extrapipe/child.zig @@ -0,0 +1,34 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const windows = std.os.windows; +pub fn main() !void { + const alloc = std.testing.allocator; + const stdin = std.io.getStdIn(); + const stdin_reader = stdin.reader(); + const stdin_cont = try stdin_reader.readUntilDelimiterAlloc(alloc, '\n', 2_000); + defer alloc.free(stdin_cont); + var file_handle = file_handle: { + if (builtin.target.os.tag == .windows) { + var handle_int = try std.fmt.parseInt(usize, stdin_cont, 10); + break :file_handle @intToPtr(windows.HANDLE, handle_int); + } else { + break :file_handle try std.fmt.parseInt(std.os.fd_t, stdin_cont, 10); + } + }; + if (builtin.target.os.tag == .windows) { + var handle_flags = windows.DWORD; + try windows.GetHandleInformation(file_handle, &handle_flags); + std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); + try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0); + } else { + var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0); + try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0); + _ = try std.os.fcntl(file_handle, std.os.F.SETFD, std.os.FD_CLOEXEC); + } + var extra_stream_in = std.fs.File{ .handle = file_handle }; + defer extra_stream_in.close(); + const extra_str_in_rd = extra_stream_in.reader(); + const all_extra_str_in = try extra_str_in_rd.readUntilDelimiterAlloc(alloc, '\x17', 20_000); + defer alloc.free(all_extra_str_in); + try std.testing.expectEqualSlices(u8, all_extra_str_in, "test123"); +} diff --git a/test/standalone/childprocess_extrapipe/parent.zig b/test/standalone/childprocess_extrapipe/parent.zig new file mode 100644 index 000000000000..5e3c085fbafe --- /dev/null +++ b/test/standalone/childprocess_extrapipe/parent.zig @@ -0,0 +1,77 @@ +const builtin = @import("builtin"); +const std = @import("std"); +const ChildProcess = std.ChildProcess; +const math = std.math; +const windows = std.windows; +const os = std.os; +const testing = std.testing; + +fn testPipeInfo(self: *ChildProcess) ChildProcess.SpawnError!void { + const windowsPtrDigits: usize = std.math.log10(math.maxInt(usize)); + const otherPtrDigits: usize = std.math.log10(math.maxInt(u32)) + 1; // +1 for sign + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + const size = comptime size: { + if (builtin.target.os.tag == .windows) { + break :size windowsPtrDigits; + } else { + break :size otherPtrDigits; + } + }; + var buf = comptime [_]u8{0} ** size; + var s_chpipe_h: []u8 = undefined; + std.debug.assert(extra.direction == .parent_to_child); + const handle = handle: { + if (builtin.target.os.tag == .windows) { + // handle is *anyopaque and there is no other way to cast + break :handle @ptrToInt(extra.*.input.?.handle); + } else { + break :handle extra.*.input.?.handle; + } + }; + s_chpipe_h = std.fmt.bufPrint( + buf[0..], + "{d}", + .{handle}, + ) catch unreachable; + try self.stdin.?.writer().writeAll(s_chpipe_h); + try self.stdin.?.writer().writeAll("\n"); + } + } +} + +pub fn main() !void { + var general_purpose_allocator = std.heap.GeneralPurposeAllocator(.{}){}; + defer std.debug.assert(!general_purpose_allocator.deinit()); + const gpa = general_purpose_allocator.allocator(); + + var child_process = ChildProcess.init( + &[_][]const u8{"zig-out/bin/child"}, + gpa, + ); + child_process.stdin_behavior = .Pipe; + var extra_streams = [_]ChildProcess.ExtraStream{ + .{ + .direction = .parent_to_child, + .input = null, + .output = null, + }, + }; + child_process.extra_streams = &extra_streams; + + try child_process.spawn(testPipeInfo); + try std.testing.expect(child_process.extra_streams.?[0].input == null); + if (builtin.target.os.tag == .windows) { + var handle_flags = windows.DWORD; + try windows.GetHandleInformation(child_process.extra_streams.?[0].output.?.handle, &handle_flags); + std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); + } else { + const fcntl_flags = try os.fcntl(child_process.extra_streams.?[0].output.?.handle, os.F.GETFD, 0); + try std.testing.expect((fcntl_flags & os.FD_CLOEXEC) != 0); + } + + const extra_str_wr = child_process.extra_streams.?[0].output.?.writer(); + try extra_str_wr.writeAll("test123\x17"); // ETB = \x17 + const ret_val = try child_process.wait(); + try testing.expectEqual(ret_val, .{ .Exited = 0 }); +} diff --git a/test/tests.zig b/test/tests.zig index d884a41599f5..ab5c8252e178 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -887,7 +887,7 @@ pub const StackTracesContext = struct { if (b.verbose) { printInvocation(args.items); } - child.spawn() catch |err| debug.panic("Unable to spawn {s}: {s}\n", .{ full_exe_path, @errorName(err) }); + child.spawn(null) catch |err| debug.panic("Unable to spawn {s}: {s}\n", .{ full_exe_path, @errorName(err) }); const stdout = child.stdout.?.reader().readAllAlloc(b.allocator, max_stdout_size) catch unreachable; defer b.allocator.free(stdout); From 3b8192671a2abfb0334d87882bc9c469c001736c Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Fri, 27 May 2022 15:20:51 +0200 Subject: [PATCH 02/16] make method to make disabling file inheritance less error prone reason: works differently on windows and Linux --- lib/std/os.zig | 10 ++++++++++ test/standalone/childprocess_extrapipe/child.zig | 7 ++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index b0884cef05d9..b14a74e0a025 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -4840,6 +4840,16 @@ pub fn lseek_CUR_get(fd: fd_t) SeekError!u64 { } } +const UnsetFileInheritanceError = FcntlError || windows.SetHandleInformationError; + +pub inline fn disableFileInheritance(file_handle: fd_t) UnsetFileInheritanceError!void { + if (builtin.os.tag == .windows) { + try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0); + } else { + _ = try fcntl(file_handle, F.SETFD, FD_CLOEXEC); + } +} + pub const FcntlError = error{ PermissionDenied, FileBusy, diff --git a/test/standalone/childprocess_extrapipe/child.zig b/test/standalone/childprocess_extrapipe/child.zig index fd740c186a28..9e9e76a3d0d5 100644 --- a/test/standalone/childprocess_extrapipe/child.zig +++ b/test/standalone/childprocess_extrapipe/child.zig @@ -16,15 +16,16 @@ pub fn main() !void { } }; if (builtin.target.os.tag == .windows) { + // windows.HANDLE_FLAG_INHERIT is enabled var handle_flags = windows.DWORD; try windows.GetHandleInformation(file_handle, &handle_flags); - std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); - try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0); + try std.testing.expect(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); } else { + // FD_CLOEXEC is not set var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0); try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0); - _ = try std.os.fcntl(file_handle, std.os.F.SETFD, std.os.FD_CLOEXEC); } + try std.os.disableFileInheritance(file_handle); var extra_stream_in = std.fs.File{ .handle = file_handle }; defer extra_stream_in.close(); const extra_str_in_rd = extra_stream_in.reader(); From 5d6221b7608ff999cba5f7460707d2ba413dc32e Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Fri, 27 May 2022 16:42:27 +0200 Subject: [PATCH 03/16] point user to function without footgun --- lib/std/child_process.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index aa44d27bc681..9270891069de 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -40,8 +40,8 @@ pub const ChildProcess = struct { /// - methods like readAll() read until the pipe write end is closed /// - Non-blocking methods require message separators like ETB, EOT /// and some more thought about message content/sanitization - /// - It is recommended to use fcntl and SetHandleInformation in the child - /// process to prevent inheritance by grandchildren + /// - It is recommended to use `os.disableFileInheritance()` + /// in the child process to prevent inheritance by grandchildren extra_streams: ?[]ExtraStream, term: ?(SpawnError!Term), From 9a93760036821fa6eb54038aa2f9d9f7141d6e5b Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Fri, 27 May 2022 16:50:10 +0200 Subject: [PATCH 04/16] also check for leaks in child --- .../childprocess_extrapipe/child.zig | 12 +-- .../childprocess_pipetest/build.zig | 12 +++ .../childprocess_pipetest/child.zig | 34 ++++++++ .../childprocess_pipetest/parent.zig | 77 +++++++++++++++++++ 4 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 test/standalone/childprocess_pipetest/build.zig create mode 100644 test/standalone/childprocess_pipetest/child.zig create mode 100644 test/standalone/childprocess_pipetest/parent.zig diff --git a/test/standalone/childprocess_extrapipe/child.zig b/test/standalone/childprocess_extrapipe/child.zig index 9e9e76a3d0d5..742e9d8bf060 100644 --- a/test/standalone/childprocess_extrapipe/child.zig +++ b/test/standalone/childprocess_extrapipe/child.zig @@ -2,11 +2,13 @@ const std = @import("std"); const builtin = @import("builtin"); const windows = std.os.windows; pub fn main() !void { - const alloc = std.testing.allocator; + var general_purpose_allocator = std.heap.GeneralPurposeAllocator(.{}){}; + defer std.debug.assert(!general_purpose_allocator.deinit()); + const gpa = general_purpose_allocator.allocator(); const stdin = std.io.getStdIn(); const stdin_reader = stdin.reader(); - const stdin_cont = try stdin_reader.readUntilDelimiterAlloc(alloc, '\n', 2_000); - defer alloc.free(stdin_cont); + const stdin_cont = try stdin_reader.readUntilDelimiterAlloc(gpa, '\n', 2_000); + defer gpa.free(stdin_cont); var file_handle = file_handle: { if (builtin.target.os.tag == .windows) { var handle_int = try std.fmt.parseInt(usize, stdin_cont, 10); @@ -29,7 +31,7 @@ pub fn main() !void { var extra_stream_in = std.fs.File{ .handle = file_handle }; defer extra_stream_in.close(); const extra_str_in_rd = extra_stream_in.reader(); - const all_extra_str_in = try extra_str_in_rd.readUntilDelimiterAlloc(alloc, '\x17', 20_000); - defer alloc.free(all_extra_str_in); + const all_extra_str_in = try extra_str_in_rd.readUntilDelimiterAlloc(gpa, '\x17', 20_000); + defer gpa.free(all_extra_str_in); try std.testing.expectEqualSlices(u8, all_extra_str_in, "test123"); } diff --git a/test/standalone/childprocess_pipetest/build.zig b/test/standalone/childprocess_pipetest/build.zig new file mode 100644 index 000000000000..9acbd9e04ce4 --- /dev/null +++ b/test/standalone/childprocess_pipetest/build.zig @@ -0,0 +1,12 @@ +const Builder = @import("std").build.Builder; + +pub fn build(b: *Builder) void { + const child = b.addExecutable("child", "child.zig"); + child.install(); + const parent = b.addExecutable("parent", "parent.zig"); + parent.install(); + const run_cmd = parent.run(); + run_cmd.step.dependOn(b.getInstallStep()); + const test_step = b.step("test", "Test it"); + test_step.dependOn(&run_cmd.step); +} diff --git a/test/standalone/childprocess_pipetest/child.zig b/test/standalone/childprocess_pipetest/child.zig new file mode 100644 index 000000000000..fd740c186a28 --- /dev/null +++ b/test/standalone/childprocess_pipetest/child.zig @@ -0,0 +1,34 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const windows = std.os.windows; +pub fn main() !void { + const alloc = std.testing.allocator; + const stdin = std.io.getStdIn(); + const stdin_reader = stdin.reader(); + const stdin_cont = try stdin_reader.readUntilDelimiterAlloc(alloc, '\n', 2_000); + defer alloc.free(stdin_cont); + var file_handle = file_handle: { + if (builtin.target.os.tag == .windows) { + var handle_int = try std.fmt.parseInt(usize, stdin_cont, 10); + break :file_handle @intToPtr(windows.HANDLE, handle_int); + } else { + break :file_handle try std.fmt.parseInt(std.os.fd_t, stdin_cont, 10); + } + }; + if (builtin.target.os.tag == .windows) { + var handle_flags = windows.DWORD; + try windows.GetHandleInformation(file_handle, &handle_flags); + std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); + try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0); + } else { + var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0); + try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0); + _ = try std.os.fcntl(file_handle, std.os.F.SETFD, std.os.FD_CLOEXEC); + } + var extra_stream_in = std.fs.File{ .handle = file_handle }; + defer extra_stream_in.close(); + const extra_str_in_rd = extra_stream_in.reader(); + const all_extra_str_in = try extra_str_in_rd.readUntilDelimiterAlloc(alloc, '\x17', 20_000); + defer alloc.free(all_extra_str_in); + try std.testing.expectEqualSlices(u8, all_extra_str_in, "test123"); +} diff --git a/test/standalone/childprocess_pipetest/parent.zig b/test/standalone/childprocess_pipetest/parent.zig new file mode 100644 index 000000000000..ca18835ca3e6 --- /dev/null +++ b/test/standalone/childprocess_pipetest/parent.zig @@ -0,0 +1,77 @@ +const builtin = @import("builtin"); +const std = @import("std"); +const ChildProcess = std.ChildProcess; +const math = std.math; +const os = std.os; +const windows = os.windows; + +fn testPipeInfo(self: *ChildProcess) ChildProcess.SpawnError!void { + const windowsPtrDigits: usize = std.math.log10(math.maxInt(usize)); + const otherPtrDigits: usize = std.math.log10(math.maxInt(u32)) + 1; // +1 for sign + if (self.extra_streams) |extra_streams| { + for (extra_streams) |*extra| { + const size = comptime size: { + if (builtin.target.os.tag == .windows) { + break :size windowsPtrDigits; + } else { + break :size otherPtrDigits; + } + }; + var buf = comptime [_]u8{0} ** size; + var s_chpipe_h: []u8 = undefined; + std.debug.assert(extra.direction == .parent_to_child); + const handle = handle: { + if (builtin.target.os.tag == .windows) { + // handle is *anyopaque and there is no other way to cast + break :handle @ptrToInt(extra.*.input.?.handle); + } else { + break :handle extra.*.input.?.handle; + } + }; + s_chpipe_h = std.fmt.bufPrint( + buf[0..], + "{d}", + .{handle}, + ) catch unreachable; + try self.stdin.?.writer().writeAll(s_chpipe_h); + try self.stdin.?.writer().writeAll("\n"); + } + } +} + +pub fn main() !void { + const testing = std.testing; + // TODO add proper allocator with cleaning up at end + // we cant rely on the testing allocator doing the leak check for us + const alloc = testing.allocator; + const child_binary = "child"; + var child_process = ChildProcess.init( + &[_][]const u8{child_binary}, + alloc, + ); + child_process.stdin_behavior = .Pipe; + var extra_streams = [_]ChildProcess.ExtraStream{ + .{ + .direction = .parent_to_child, + .input = null, + .output = null, + }, + }; + child_process.extra_streams = &extra_streams; + + try child_process.spawn(testPipeInfo); + try std.testing.expect(child_process.extra_streams.?[0].input == null); + if (builtin.target.os.tag == .windows) { + var handle_flags = windows.DWORD; + try windows.GetHandleInformation(child_process.extra_streams.?[0].output.?.handle, &handle_flags); + std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); + } else { + const fcntl_flags = try os.fcntl(child_process.extra_streams.?[0].output.?.handle, os.F.GETFD, 0); + try std.testing.expect((fcntl_flags & os.FD_CLOEXEC) != 0); + } + + const extra_str_wr = child_process.extra_streams.?[0].output.?.writer(); + try extra_str_wr.writeAll("test123\x17"); // ETB = \x17 + const ret_val = try child_process.wait(); + try testing.expectEqual(ret_val, .{ .Exited = 0 }); +} From 448d375fb3a13058a3285f686752a02e800c0e19 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Tue, 31 May 2022 21:48:40 +0200 Subject: [PATCH 05/16] remove stuff --- .../childprocess_pipetest/build.zig | 12 --- .../childprocess_pipetest/child.zig | 34 -------- .../childprocess_pipetest/parent.zig | 77 ------------------- 3 files changed, 123 deletions(-) delete mode 100644 test/standalone/childprocess_pipetest/build.zig delete mode 100644 test/standalone/childprocess_pipetest/child.zig delete mode 100644 test/standalone/childprocess_pipetest/parent.zig diff --git a/test/standalone/childprocess_pipetest/build.zig b/test/standalone/childprocess_pipetest/build.zig deleted file mode 100644 index 9acbd9e04ce4..000000000000 --- a/test/standalone/childprocess_pipetest/build.zig +++ /dev/null @@ -1,12 +0,0 @@ -const Builder = @import("std").build.Builder; - -pub fn build(b: *Builder) void { - const child = b.addExecutable("child", "child.zig"); - child.install(); - const parent = b.addExecutable("parent", "parent.zig"); - parent.install(); - const run_cmd = parent.run(); - run_cmd.step.dependOn(b.getInstallStep()); - const test_step = b.step("test", "Test it"); - test_step.dependOn(&run_cmd.step); -} diff --git a/test/standalone/childprocess_pipetest/child.zig b/test/standalone/childprocess_pipetest/child.zig deleted file mode 100644 index fd740c186a28..000000000000 --- a/test/standalone/childprocess_pipetest/child.zig +++ /dev/null @@ -1,34 +0,0 @@ -const std = @import("std"); -const builtin = @import("builtin"); -const windows = std.os.windows; -pub fn main() !void { - const alloc = std.testing.allocator; - const stdin = std.io.getStdIn(); - const stdin_reader = stdin.reader(); - const stdin_cont = try stdin_reader.readUntilDelimiterAlloc(alloc, '\n', 2_000); - defer alloc.free(stdin_cont); - var file_handle = file_handle: { - if (builtin.target.os.tag == .windows) { - var handle_int = try std.fmt.parseInt(usize, stdin_cont, 10); - break :file_handle @intToPtr(windows.HANDLE, handle_int); - } else { - break :file_handle try std.fmt.parseInt(std.os.fd_t, stdin_cont, 10); - } - }; - if (builtin.target.os.tag == .windows) { - var handle_flags = windows.DWORD; - try windows.GetHandleInformation(file_handle, &handle_flags); - std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); - try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0); - } else { - var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0); - try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0); - _ = try std.os.fcntl(file_handle, std.os.F.SETFD, std.os.FD_CLOEXEC); - } - var extra_stream_in = std.fs.File{ .handle = file_handle }; - defer extra_stream_in.close(); - const extra_str_in_rd = extra_stream_in.reader(); - const all_extra_str_in = try extra_str_in_rd.readUntilDelimiterAlloc(alloc, '\x17', 20_000); - defer alloc.free(all_extra_str_in); - try std.testing.expectEqualSlices(u8, all_extra_str_in, "test123"); -} diff --git a/test/standalone/childprocess_pipetest/parent.zig b/test/standalone/childprocess_pipetest/parent.zig deleted file mode 100644 index ca18835ca3e6..000000000000 --- a/test/standalone/childprocess_pipetest/parent.zig +++ /dev/null @@ -1,77 +0,0 @@ -const builtin = @import("builtin"); -const std = @import("std"); -const ChildProcess = std.ChildProcess; -const math = std.math; -const os = std.os; -const windows = os.windows; - -fn testPipeInfo(self: *ChildProcess) ChildProcess.SpawnError!void { - const windowsPtrDigits: usize = std.math.log10(math.maxInt(usize)); - const otherPtrDigits: usize = std.math.log10(math.maxInt(u32)) + 1; // +1 for sign - if (self.extra_streams) |extra_streams| { - for (extra_streams) |*extra| { - const size = comptime size: { - if (builtin.target.os.tag == .windows) { - break :size windowsPtrDigits; - } else { - break :size otherPtrDigits; - } - }; - var buf = comptime [_]u8{0} ** size; - var s_chpipe_h: []u8 = undefined; - std.debug.assert(extra.direction == .parent_to_child); - const handle = handle: { - if (builtin.target.os.tag == .windows) { - // handle is *anyopaque and there is no other way to cast - break :handle @ptrToInt(extra.*.input.?.handle); - } else { - break :handle extra.*.input.?.handle; - } - }; - s_chpipe_h = std.fmt.bufPrint( - buf[0..], - "{d}", - .{handle}, - ) catch unreachable; - try self.stdin.?.writer().writeAll(s_chpipe_h); - try self.stdin.?.writer().writeAll("\n"); - } - } -} - -pub fn main() !void { - const testing = std.testing; - // TODO add proper allocator with cleaning up at end - // we cant rely on the testing allocator doing the leak check for us - const alloc = testing.allocator; - const child_binary = "child"; - var child_process = ChildProcess.init( - &[_][]const u8{child_binary}, - alloc, - ); - child_process.stdin_behavior = .Pipe; - var extra_streams = [_]ChildProcess.ExtraStream{ - .{ - .direction = .parent_to_child, - .input = null, - .output = null, - }, - }; - child_process.extra_streams = &extra_streams; - - try child_process.spawn(testPipeInfo); - try std.testing.expect(child_process.extra_streams.?[0].input == null); - if (builtin.target.os.tag == .windows) { - var handle_flags = windows.DWORD; - try windows.GetHandleInformation(child_process.extra_streams.?[0].output.?.handle, &handle_flags); - std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); - } else { - const fcntl_flags = try os.fcntl(child_process.extra_streams.?[0].output.?.handle, os.F.GETFD, 0); - try std.testing.expect((fcntl_flags & os.FD_CLOEXEC) != 0); - } - - const extra_str_wr = child_process.extra_streams.?[0].output.?.writer(); - try extra_str_wr.writeAll("test123\x17"); // ETB = \x17 - const ret_val = try child_process.wait(); - try testing.expectEqual(ret_val, .{ .Exited = 0 }); -} From 2ef7caec29937d2e1065837a480855729498e0d6 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Tue, 31 May 2022 22:33:14 +0200 Subject: [PATCH 06/16] use struct for extensibility as suggested by @daurnimator --- lib/std/build.zig | 2 +- lib/std/build/RunStep.zig | 2 +- lib/std/child_process.zig | 41 +++++++++++-------- src/Compilation.zig | 4 +- src/link/Elf.zig | 4 +- src/link/Wasm.zig | 4 +- src/main.zig | 4 +- src/mingw.zig | 2 +- .../childprocess_extrapipe/parent.zig | 2 +- test/tests.zig | 2 +- 10 files changed, 36 insertions(+), 31 deletions(-) diff --git a/lib/std/build.zig b/lib/std/build.zig index 39123e19631c..d5fe9a4d4db4 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -1194,7 +1194,7 @@ pub const Builder = struct { child.stderr_behavior = stderr_behavior; child.env_map = self.env_map; - try child.spawn(null); + try child.spawn(.{}); const stdout = child.stdout.?.reader().readAllAlloc(self.allocator, max_output_size) catch { return error.ReadFailure; diff --git a/lib/std/build/RunStep.zig b/lib/std/build/RunStep.zig index d867e7c0cc62..eb1383146436 100644 --- a/lib/std/build/RunStep.zig +++ b/lib/std/build/RunStep.zig @@ -224,7 +224,7 @@ pub fn runCommand( if (print) printCmd(cwd, argv); - child.spawn(null) catch |err| { + child.spawn(.{}) catch |err| { std.debug.print("Unable to spawn {s}: {s}\n", .{ argv[0], @errorName(err) }); return err; }; diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 9270891069de..5f5f4a46ab25 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -172,27 +172,32 @@ pub const ChildProcess = struct { else => ?*const fn (self: *ChildProcess) ChildProcess.SpawnError!void, }; + pub const SpawnOptions = struct { + /// Use this field to execute custom code in parent process before the clone call. + /// For example, the child process must be told about non-standard pipes this way. + /// See field of ChildProcess 'extra_streams' for more information. + pipe_info_fn: ExPipeInfoProto = null, + }; + /// On success must call `kill` or `wait`. - /// Use null xor function pointer to tell child process about extra pipes - /// See field extra_streams for more information - pub fn spawn(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { + pub fn spawn(self: *ChildProcess, opts: SpawnOptions) SpawnError!void { if (!std.process.can_spawn) { @compileError("the target operating system cannot spawn processes"); } if (comptime builtin.target.isDarwin()) { - return self.spawnMacos(pipe_info_fn); + return self.spawnMacos(opts); } if (builtin.os.tag == .windows) { - return self.spawnWindows(pipe_info_fn); + return self.spawnWindows(opts); } else { - return self.spawnPosix(pipe_info_fn); + return self.spawnPosix(opts); } } - pub fn spawnAndWait(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!Term { - try self.spawn(pipe_info_fn); + pub fn spawnAndWait(self: *ChildProcess, opts: SpawnOptions) SpawnError!Term { + try self.spawn(opts); return self.wait(); } @@ -436,7 +441,7 @@ pub const ChildProcess = struct { child.env_map = args.env_map; child.expand_arg0 = args.expand_arg0; - try child.spawn(null); + try child.spawn(.{}); if (builtin.os.tag == .haiku) { const stdout_in = child.stdout.?.reader(); @@ -609,7 +614,7 @@ pub const ChildProcess = struct { Term{ .Unknown = status }; } - fn spawnMacos(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { + fn spawnMacos(self: *ChildProcess, opts: SpawnOptions) SpawnError!void { const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0; const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined; errdefer if (self.stdin_behavior == StdIo.Pipe) destroyPipe(stdin_pipe); @@ -722,8 +727,8 @@ pub const ChildProcess = struct { // user must communicate extra pipes to child process either via // environment variables or stdin if (self.extra_streams != null) { - std.debug.assert(pipe_info_fn != null); - try pipe_info_fn.?(self); + std.debug.assert(opts.pipe_info_fn != null); + try opts.pipe_info_fn.?(self); } const pid = try os.posix_spawn.spawnp(self.argv[0], actions, attr, argv_buf, envp); @@ -780,7 +785,7 @@ pub const ChildProcess = struct { } } - fn spawnPosix(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { + fn spawnPosix(self: *ChildProcess, opts: SpawnOptions) SpawnError!void { const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0; const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined; errdefer if (self.stdin_behavior == StdIo.Pipe) { @@ -887,8 +892,8 @@ pub const ChildProcess = struct { // user must communicate extra pipes to child process either via // environment variables or stdin if (self.extra_streams != null) { - std.debug.assert(pipe_info_fn != null); - try pipe_info_fn.?(self); + std.debug.assert(opts.pipe_info_fn != null); + try opts.pipe_info_fn.?(self); } // This pipe is used to communicate errors between the time of fork @@ -999,7 +1004,7 @@ pub const ChildProcess = struct { } } - fn spawnWindows(self: *ChildProcess, pipe_info_fn: ExPipeInfoProto) SpawnError!void { + fn spawnWindows(self: *ChildProcess, opts: SpawnOptions) SpawnError!void { const saAttr = windows.SECURITY_ATTRIBUTES{ .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES), .bInheritHandle = windows.TRUE, @@ -1231,8 +1236,8 @@ pub const ChildProcess = struct { // user must communicate extra pipes to child process either via // environment variables or stdin if (self.extra_streams != null) { - std.debug.assert(pipe_info_fn != null); - try pipe_info_fn.?(self); + std.debug.assert(opts.pipe_info_fn != null); + try opts.pipe_info_fn.?(self); } exec: { diff --git a/src/Compilation.zig b/src/Compilation.zig index 00c097e73368..6a9b1120d5b3 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -4052,7 +4052,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait(null) catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { return comp.failCObj(c_object, "unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); }; switch (term) { @@ -4070,7 +4070,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(null); + try child.spawn(.{}); const stderr_reader = child.stderr.?.reader(); diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 67a4437837bd..ff4958255edc 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1870,7 +1870,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait(null) catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); return error.UnableToSpawnSelf; }; @@ -1887,7 +1887,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(null); + try child.spawn(.{}); const stderr = try child.stderr.?.reader().readAllAlloc(arena, 10 * 1024 * 1024); diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index a1b1fe8058f4..b85bedb425db 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -3548,7 +3548,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait(null) catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); return error.UnableToSpawnWasm; }; @@ -3565,7 +3565,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(null); + try child.spawn(.{}); const stderr = try child.stderr.?.reader().readAllAlloc(arena, 10 * 1024 * 1024); diff --git a/src/main.zig b/src/main.zig index 40e8fea311f4..d8dabc8ed2f5 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3376,7 +3376,7 @@ fn runOrTest( comp_destroyed.* = true; } - const term = child.spawnAndWait(null) catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { try warnAboutForeignBinaries(arena, arg_mode, target_info, link_libc); const cmd = try std.mem.join(arena, " ", argv.items); fatal("the following command failed with '{s}':\n{s}", .{ @errorName(err), cmd }); @@ -4059,7 +4059,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = try child.spawnAndWait(null); + const term = try child.spawnAndWait(.{}); switch (term) { .Exited => |code| { if (code == 0) return cleanExit(); diff --git a/src/mingw.zig b/src/mingw.zig index a6813aa0c6c3..4d9b03a720f5 100644 --- a/src/mingw.zig +++ b/src/mingw.zig @@ -376,7 +376,7 @@ pub fn buildImportLib(comp: *Compilation, lib_name: []const u8) !void { child.stdout_behavior = .Pipe; child.stderr_behavior = .Pipe; - try child.spawn(null); + try child.spawn(.{}); const stderr_reader = child.stderr.?.reader(); diff --git a/test/standalone/childprocess_extrapipe/parent.zig b/test/standalone/childprocess_extrapipe/parent.zig index 5e3c085fbafe..7c049f519718 100644 --- a/test/standalone/childprocess_extrapipe/parent.zig +++ b/test/standalone/childprocess_extrapipe/parent.zig @@ -59,7 +59,7 @@ pub fn main() !void { }; child_process.extra_streams = &extra_streams; - try child_process.spawn(testPipeInfo); + try child_process.spawn(.{ .pipe_info_fn = testPipeInfo }); try std.testing.expect(child_process.extra_streams.?[0].input == null); if (builtin.target.os.tag == .windows) { var handle_flags = windows.DWORD; diff --git a/test/tests.zig b/test/tests.zig index ab5c8252e178..96005ca5776e 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -887,7 +887,7 @@ pub const StackTracesContext = struct { if (b.verbose) { printInvocation(args.items); } - child.spawn(null) catch |err| debug.panic("Unable to spawn {s}: {s}\n", .{ full_exe_path, @errorName(err) }); + child.spawn(.{}) catch |err| debug.panic("Unable to spawn {s}: {s}\n", .{ full_exe_path, @errorName(err) }); const stdout = child.stdout.?.reader().readAllAlloc(b.allocator, max_stdout_size) catch unreachable; defer b.allocator.free(stdout); From 03cf3b2ef5ee313aea952926b14aa6ea700dc3b9 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Tue, 31 May 2022 22:59:36 +0200 Subject: [PATCH 07/16] switch case everywhere --- lib/std/child_process.zig | 137 +++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 62 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 5f5f4a46ab25..d75fedeff034 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -548,16 +548,19 @@ pub const ChildProcess = struct { } if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - if (extra.*.output) |*outpipe| { - outpipe.close(); - extra.*.output = null; - } - } else { - if (extra.*.input) |*inpipe| { - inpipe.close(); - extra.*.input = null; - } + switch (extra.*.direction) { + .parent_to_child => { + if (extra.*.output) |*outpipe| { + outpipe.close(); + extra.*.output = null; + } + }, + .child_to_parent => { + if (extra.*.input) |*inpipe| { + inpipe.close(); + extra.*.input = null; + } + }, } } } @@ -698,10 +701,9 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - try actions.close(extra.*.output.?.handle); - } else { - try actions.close(extra.*.input.?.handle); + switch (extra.*.direction) { + .parent_to_child => try actions.close(extra.*.output.?.handle), + .child_to_parent => try actions.close(extra.*.input.?.handle), } } } @@ -747,20 +749,23 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - extra.*.input.?.close(); // parent does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); - var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); - std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); - } else { - extra.*.output.?.close(); // parent does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); - var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); - std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + switch (extra.*.direction) { + .parent_to_child => { + extra.*.input.?.close(); // parent does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + }, + .child_to_parent => { + extra.*.output.?.close(); // parent does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + }, } } } @@ -932,14 +937,17 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - extra.*.output.?.close(); // child does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); - } else { - extra.*.input.?.close(); // child does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); + switch (extra.*.direction) { + .parent_to_child => { + extra.*.output.?.close(); // child does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + }, + .child_to_parent => { + extra.*.input.?.close(); // child does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + }, } } } @@ -985,20 +993,23 @@ pub const ChildProcess = struct { // to prevent non-standard streams to leak into children if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - extra.*.input.?.close(); // parent does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); - var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); - std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); - } else { - extra.*.output.?.close(); // parent does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); - var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); - std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + switch (extra.*.direction) { + .parent_to_child => { + extra.*.input.?.close(); // parent does not read + extra.*.input = null; + std.debug.assert(extra.*.output != null); + var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + }, + .child_to_parent => { + extra.*.output.?.close(); // parent does not write + extra.*.output = null; + std.debug.assert(extra.*.input != null); + var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); + _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + }, } } } @@ -1336,14 +1347,17 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - if (extra.*.direction == .parent_to_child) { - extra.*.input.?.close(); // reading end - extra.*.input = null; - try windows.SetHandleInformation(extra.*.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); - } else { - extra.*.output.?.close(); // writing end - extra.*.output = null; - try windows.SetHandleInformation(extra.*.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + switch (extra.*.direction) { + .parent_to_child => { + extra.*.input.?.close(); // reading end + extra.*.input = null; + try windows.SetHandleInformation(extra.*.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + }, + .child_to_parent => { + extra.*.output.?.close(); // writing end + extra.*.output = null; + try windows.SetHandleInformation(extra.*.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + }, } } } @@ -1788,10 +1802,9 @@ fn windowsMakeAsyncPipe( } errdefer os.close(write_handle); - if (direction == .child_to_parent) { - try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); - } else { - try windows.SetHandleInformation(write_handle, windows.HANDLE_FLAG_INHERIT, 0); + switch (direction) { + .child_to_parent => try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0), + .parent_to_child => try windows.SetHandleInformation(write_handle, windows.HANDLE_FLAG_INHERIT, 0), } rd.* = read_handle; wr.* = write_handle; From 76113daf5c3f92496493a5f9857905c7484d06df Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Tue, 31 May 2022 23:21:37 +0200 Subject: [PATCH 08/16] remove all unnecessary `.*` --- lib/std/child_process.zig | 138 +++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index d75fedeff034..734618cda9d1 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -548,17 +548,17 @@ pub const ChildProcess = struct { } if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { + switch (extra.direction) { .parent_to_child => { - if (extra.*.output) |*outpipe| { + if (extra.output) |*outpipe| { outpipe.close(); - extra.*.output = null; + extra.output = null; } }, .child_to_parent => { - if (extra.*.input) |*inpipe| { + if (extra.input) |*inpipe| { inpipe.close(); - extra.*.input = null; + extra.input = null; } }, } @@ -663,21 +663,21 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra, i| { - std.debug.assert(extra.*.input == null); - std.debug.assert(extra.*.output == null); + std.debug.assert(extra.input == null); + std.debug.assert(extra.output == null); const tmp_pipe = os.pipe() catch |err| { for (extra_streams[0..i]) |*extra_fail| { - extra_fail.*.input.?.close(); - extra_fail.*.output.?.close(); - extra_fail.*.input = null; - extra_fail.*.output = null; + extra_fail.input.?.close(); + extra_fail.output.?.close(); + extra_fail.input = null; + extra_fail.output = null; } return err; }; - extra.*.input = File{ .handle = tmp_pipe[0] }; - extra.*.output = File{ .handle = tmp_pipe[1] }; + extra.input = File{ .handle = tmp_pipe[0] }; + extra.output = File{ .handle = tmp_pipe[1] }; } } @@ -701,9 +701,9 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { - .parent_to_child => try actions.close(extra.*.output.?.handle), - .child_to_parent => try actions.close(extra.*.input.?.handle), + switch (extra.direction) { + .parent_to_child => try actions.close(extra.output.?.handle), + .child_to_parent => try actions.close(extra.input.?.handle), } } } @@ -749,22 +749,22 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { + switch (extra.direction) { .parent_to_child => { - extra.*.input.?.close(); // parent does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); - var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + extra.input.?.close(); // parent does not read + extra.input = null; + std.debug.assert(extra.output != null); + var fcntl_flags = try os.fcntl(extra.output.?.handle, os.F.GETFD, 0); std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + _ = try os.fcntl(extra.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); }, .child_to_parent => { - extra.*.output.?.close(); // parent does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); - var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + extra.output.?.close(); // parent does not write + extra.output = null; + std.debug.assert(extra.input != null); + var fcntl_flags = try os.fcntl(extra.input.?.handle, os.F.GETFD, 0); std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + _ = try os.fcntl(extra.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); }, } } @@ -844,21 +844,21 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra, i| { - std.debug.assert(extra.*.input == null); - std.debug.assert(extra.*.output == null); + std.debug.assert(extra.input == null); + std.debug.assert(extra.output == null); const tmp_pipe = os.pipe() catch |err| { for (extra_streams[0..i]) |*extra_fail| { - extra_fail.*.input.?.close(); - extra_fail.*.output.?.close(); - extra_fail.*.input = null; - extra_fail.*.output = null; + extra_fail.input.?.close(); + extra_fail.output.?.close(); + extra_fail.input = null; + extra_fail.output = null; } return err; }; - extra.*.input = File{ .handle = tmp_pipe[0] }; - extra.*.output = File{ .handle = tmp_pipe[1] }; + extra.input = File{ .handle = tmp_pipe[0] }; + extra.output = File{ .handle = tmp_pipe[1] }; } } @@ -937,16 +937,16 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { + switch (extra.direction) { .parent_to_child => { - extra.*.output.?.close(); // child does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); + extra.output.?.close(); // child does not write + extra.output = null; + std.debug.assert(extra.input != null); }, .child_to_parent => { - extra.*.input.?.close(); // child does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); + extra.input.?.close(); // child does not read + extra.input = null; + std.debug.assert(extra.output != null); }, } } @@ -993,22 +993,22 @@ pub const ChildProcess = struct { // to prevent non-standard streams to leak into children if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { + switch (extra.direction) { .parent_to_child => { - extra.*.input.?.close(); // parent does not read - extra.*.input = null; - std.debug.assert(extra.*.output != null); - var fcntl_flags = try os.fcntl(extra.*.output.?.handle, os.F.GETFD, 0); + extra.input.?.close(); // parent does not read + extra.input = null; + std.debug.assert(extra.output != null); + var fcntl_flags = try os.fcntl(extra.output.?.handle, os.F.GETFD, 0); std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); + _ = try os.fcntl(extra.output.?.handle, os.F.SETFD, os.FD_CLOEXEC); }, .child_to_parent => { - extra.*.output.?.close(); // parent does not write - extra.*.output = null; - std.debug.assert(extra.*.input != null); - var fcntl_flags = try os.fcntl(extra.*.input.?.handle, os.F.GETFD, 0); + extra.output.?.close(); // parent does not write + extra.output = null; + std.debug.assert(extra.input != null); + var fcntl_flags = try os.fcntl(extra.input.?.handle, os.F.GETFD, 0); std.debug.assert((fcntl_flags & os.FD_CLOEXEC) == 0); - _ = try os.fcntl(extra.*.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); + _ = try os.fcntl(extra.input.?.handle, os.F.SETFD, os.FD_CLOEXEC); }, } } @@ -1143,8 +1143,8 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra, i| { - std.debug.assert(extra.*.input == null); - std.debug.assert(extra.*.output == null); + std.debug.assert(extra.input == null); + std.debug.assert(extra.output == null); var g_hChildExtra_Stream_Rd: ?windows.HANDLE = null; var g_hChildExtra_Stream_Wr: ?windows.HANDLE = null; @@ -1152,18 +1152,18 @@ pub const ChildProcess = struct { &g_hChildExtra_Stream_Rd, &g_hChildExtra_Stream_Wr, &saAttr, - extra.*.direction, + extra.direction, ) catch |err| { for (extra_streams[0..i]) |*extra_fail| { - extra_fail.*.input.?.close(); - extra_fail.*.output.?.close(); - extra_fail.*.input = null; - extra_fail.*.output = null; + extra_fail.input.?.close(); + extra_fail.output.?.close(); + extra_fail.input = null; + extra_fail.output = null; } return err; }; - extra.*.input = File{ .handle = g_hChildExtra_Stream_Rd.? }; - extra.*.output = File{ .handle = g_hChildExtra_Stream_Wr.? }; + extra.input = File{ .handle = g_hChildExtra_Stream_Rd.? }; + extra.output = File{ .handle = g_hChildExtra_Stream_Wr.? }; } } @@ -1347,16 +1347,16 @@ pub const ChildProcess = struct { if (self.extra_streams) |extra_streams| { for (extra_streams) |*extra| { - switch (extra.*.direction) { + switch (extra.direction) { .parent_to_child => { - extra.*.input.?.close(); // reading end - extra.*.input = null; - try windows.SetHandleInformation(extra.*.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + extra.input.?.close(); // reading end + extra.input = null; + try windows.SetHandleInformation(extra.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); }, .child_to_parent => { - extra.*.output.?.close(); // writing end - extra.*.output = null; - try windows.SetHandleInformation(extra.*.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + extra.output.?.close(); // writing end + extra.output = null; + try windows.SetHandleInformation(extra.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); }, } } From 9aeadfdccc3264a9e419243ee823c8d8a873179f Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Tue, 6 Sep 2022 00:35:58 +0200 Subject: [PATCH 09/16] track latest master changes --- lib/std/build.zig | 2 +- src/link/Coff/lld.zig | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/std/build.zig b/lib/std/build.zig index d5fe9a4d4db4..83316dc5ac76 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -978,7 +978,7 @@ pub const Builder = struct { child.cwd = cwd; child.env_map = env_map; - const term = child.spawnAndWait(null) catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { log.err("Unable to spawn {s}: {s}\n", .{ argv[0], @errorName(err) }); return err; }; diff --git a/src/link/Coff/lld.zig b/src/link/Coff/lld.zig index 46b013054289..27342abc6536 100644 --- a/src/link/Coff/lld.zig +++ b/src/link/Coff/lld.zig @@ -505,7 +505,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; - const term = child.spawnAndWait() catch |err| { + const term = child.spawnAndWait(.{}) catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); return error.UnableToSpawnSelf; }; @@ -522,7 +522,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod child.stdout_behavior = .Ignore; child.stderr_behavior = .Pipe; - try child.spawn(); + try child.spawn(.{}); const stderr = try child.stderr.?.reader().readAllAlloc(arena, 10 * 1024 * 1024); From 801ffd01067e6b045582770d26d044b5418c713e Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Sun, 27 Nov 2022 12:09:22 +0100 Subject: [PATCH 10/16] smol fixup --- test/standalone/childprocess_extrapipe/parent.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/childprocess_extrapipe/parent.zig b/test/standalone/childprocess_extrapipe/parent.zig index 7c049f519718..a3019cda4e69 100644 --- a/test/standalone/childprocess_extrapipe/parent.zig +++ b/test/standalone/childprocess_extrapipe/parent.zig @@ -2,7 +2,7 @@ const builtin = @import("builtin"); const std = @import("std"); const ChildProcess = std.ChildProcess; const math = std.math; -const windows = std.windows; +const windows = std.os.windows; const os = std.os; const testing = std.testing; From a8d3094fa0d34452a6f12a09acf0aa6a5765ce2e Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Sun, 27 Nov 2022 15:05:03 +0100 Subject: [PATCH 11/16] another fixup --- test/standalone/childprocess_extrapipe/parent.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/childprocess_extrapipe/parent.zig b/test/standalone/childprocess_extrapipe/parent.zig index a3019cda4e69..53e97bd47e68 100644 --- a/test/standalone/childprocess_extrapipe/parent.zig +++ b/test/standalone/childprocess_extrapipe/parent.zig @@ -62,7 +62,7 @@ pub fn main() !void { try child_process.spawn(.{ .pipe_info_fn = testPipeInfo }); try std.testing.expect(child_process.extra_streams.?[0].input == null); if (builtin.target.os.tag == .windows) { - var handle_flags = windows.DWORD; + var handle_flags: windows.DWORD = undefined; try windows.GetHandleInformation(child_process.extra_streams.?[0].output.?.handle, &handle_flags); std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); } else { From 7bb0b343c15df86c2875e3cc0dbc6caa7e016cd5 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Sun, 27 Nov 2022 21:29:12 +0100 Subject: [PATCH 12/16] add missing kernel32 bindings for GetHandleInformation --- lib/std/os/windows.zig | 11 +++++++++++ lib/std/os/windows/kernel32.zig | 2 ++ 2 files changed, 13 insertions(+) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index ea09631719af..b6cbe8e39f0d 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -225,6 +225,17 @@ pub fn DeviceIoControl( } } +pub const GetHandleInformationError = error{Unexpected}; + +pub fn GetHandleInformation(h: HANDLE, flags: *DWORD) GetHandleInformationError!DWORD { + if (kernel32.GetHandleInformation(h, flags) == 0) { + switch (kernel32.GetLastError()) { + else => |err| return unexpectedError(err), + } + } + return flags.*; +} + pub fn GetOverlappedResult(h: HANDLE, overlapped: *OVERLAPPED, wait: bool) !DWORD { var bytes: DWORD = undefined; if (kernel32.GetOverlappedResult(h, overlapped, &bytes, @boolToInt(wait)) == 0) { diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index e1cb7f333a28..75b95217f799 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -218,6 +218,8 @@ pub extern "kernel32" fn GetFullPathNameW( lpFilePart: ?*?[*:0]u16, ) callconv(@import("std").os.windows.WINAPI) u32; +pub extern "kernel32" fn GetHandleInformation(hObject: HANDLE, dwFlags: *DWORD) callconv(WINAPI) BOOL; + pub extern "kernel32" fn GetOverlappedResult(hFile: HANDLE, lpOverlapped: *OVERLAPPED, lpNumberOfBytesTransferred: *DWORD, bWait: BOOL) callconv(WINAPI) BOOL; pub extern "kernel32" fn GetProcessHeap() callconv(WINAPI) ?HANDLE; From b16eeee40242a1f3908ce7d5a4f7da8e78b6bfb6 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Sun, 27 Nov 2022 22:19:54 +0100 Subject: [PATCH 13/16] remove unused return value in os.windows --- lib/std/os/windows.zig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index b6cbe8e39f0d..d56a1e953d7e 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -227,13 +227,12 @@ pub fn DeviceIoControl( pub const GetHandleInformationError = error{Unexpected}; -pub fn GetHandleInformation(h: HANDLE, flags: *DWORD) GetHandleInformationError!DWORD { +pub fn GetHandleInformation(h: HANDLE, flags: *DWORD) GetHandleInformationError!void { if (kernel32.GetHandleInformation(h, flags) == 0) { switch (kernel32.GetLastError()) { else => |err| return unexpectedError(err), } } - return flags.*; } pub fn GetOverlappedResult(h: HANDLE, overlapped: *OVERLAPPED, wait: bool) !DWORD { From 3399aa6cca08a7095b0f7b58025963db55e40076 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 28 Nov 2022 00:05:05 +0100 Subject: [PATCH 14/16] smol fixup --- test/standalone/childprocess_extrapipe/child.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/standalone/childprocess_extrapipe/child.zig b/test/standalone/childprocess_extrapipe/child.zig index 742e9d8bf060..24eaf25ae623 100644 --- a/test/standalone/childprocess_extrapipe/child.zig +++ b/test/standalone/childprocess_extrapipe/child.zig @@ -19,7 +19,7 @@ pub fn main() !void { }; if (builtin.target.os.tag == .windows) { // windows.HANDLE_FLAG_INHERIT is enabled - var handle_flags = windows.DWORD; + var handle_flags: windows.DWORD = undefined; try windows.GetHandleInformation(file_handle, &handle_flags); try std.testing.expect(handle_flags & windows.HANDLE_FLAG_INHERIT != 0); } else { From f94de5ac1af853b5aef586fa32363fb80fd90913 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 28 Nov 2022 01:33:25 +0100 Subject: [PATCH 15/16] dont rely on install step with racy file system --- test/standalone/childprocess_extrapipe/build.zig | 10 +++++++--- test/standalone/childprocess_extrapipe/parent.zig | 13 +++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/test/standalone/childprocess_extrapipe/build.zig b/test/standalone/childprocess_extrapipe/build.zig index 9acbd9e04ce4..6a18f3f6d169 100644 --- a/test/standalone/childprocess_extrapipe/build.zig +++ b/test/standalone/childprocess_extrapipe/build.zig @@ -1,12 +1,16 @@ const Builder = @import("std").build.Builder; pub fn build(b: *Builder) void { + const mode = b.standardReleaseOptions(); + const child = b.addExecutable("child", "child.zig"); - child.install(); + child.setBuildMode(mode); + const parent = b.addExecutable("parent", "parent.zig"); - parent.install(); + parent.setBuildMode(mode); const run_cmd = parent.run(); - run_cmd.step.dependOn(b.getInstallStep()); + run_cmd.addArtifactArg(child); + const test_step = b.step("test", "Test it"); test_step.dependOn(&run_cmd.step); } diff --git a/test/standalone/childprocess_extrapipe/parent.zig b/test/standalone/childprocess_extrapipe/parent.zig index 53e97bd47e68..df5c2c99567d 100644 --- a/test/standalone/childprocess_extrapipe/parent.zig +++ b/test/standalone/childprocess_extrapipe/parent.zig @@ -41,12 +41,17 @@ fn testPipeInfo(self: *ChildProcess) ChildProcess.SpawnError!void { } pub fn main() !void { - var general_purpose_allocator = std.heap.GeneralPurposeAllocator(.{}){}; - defer std.debug.assert(!general_purpose_allocator.deinit()); - const gpa = general_purpose_allocator.allocator(); + var gpa_state = std.heap.GeneralPurposeAllocator(.{}){}; + defer if (gpa_state.deinit()) @panic("found memory leaks"); + const gpa = gpa_state.allocator(); + + var it = try std.process.argsWithAllocator(gpa); + defer it.deinit(); + _ = it.next() orelse unreachable; // skip binary name + const child_path = it.next() orelse unreachable; var child_process = ChildProcess.init( - &[_][]const u8{"zig-out/bin/child"}, + &.{child_path}, gpa, ); child_process.stdin_behavior = .Pipe; From 76b9978d725f5f96f7227e4cb438074fcbeefa17 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 28 Nov 2022 23:23:43 +0100 Subject: [PATCH 16/16] SetHandleInformation wants handle, mask, flags applied_flags = mask & flags On the upside, I have a simple Zig dev setup and it seems to work on Win10. --- lib/std/child_process.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 734618cda9d1..85f7273c3e76 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -1351,12 +1351,12 @@ pub const ChildProcess = struct { .parent_to_child => { extra.input.?.close(); // reading end extra.input = null; - try windows.SetHandleInformation(extra.output.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + try windows.SetHandleInformation(extra.output.?.handle, windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT); }, .child_to_parent => { extra.output.?.close(); // writing end extra.output = null; - try windows.SetHandleInformation(extra.input.?.handle, windows.HANDLE_FLAG_INHERIT, 0); + try windows.SetHandleInformation(extra.input.?.handle, windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT); }, } } @@ -1803,8 +1803,8 @@ fn windowsMakeAsyncPipe( errdefer os.close(write_handle); switch (direction) { - .child_to_parent => try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0), - .parent_to_child => try windows.SetHandleInformation(write_handle, windows.HANDLE_FLAG_INHERIT, 0), + .child_to_parent => try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT), + .parent_to_child => try windows.SetHandleInformation(write_handle, windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT), } rd.* = read_handle; wr.* = write_handle;