From 34c00ecf57a50e19b31fce420044311c4f2e9c7a Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Tue, 20 Oct 2020 08:51:21 +0200 Subject: [PATCH 1/4] std: Avoid deadlocking in ChildProcess.exec Reading stdin&stderr at different times may lead to nasty deadlocks (eg. when stdout is read before stderr and the child process doesn't write anything onto stdout). Implement a polling mechanism to make sure this won't happen: we read data from stderr/stdout as it becomes ready and then it's copied into an ArrayList provided by the user, avoiding any kind of blocking read. --- lib/std/child_process.zig | 123 +++++++++++++++++++++++++++++++- lib/std/os/windows/bits.zig | 13 ++++ lib/std/os/windows/kernel32.zig | 33 +++++++++ 3 files changed, 167 insertions(+), 2 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 1cdb3ef34c..936609d309 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -257,6 +257,68 @@ pub const ChildProcess = struct { } } + fn collectOutputWindows(child: *const ChildProcess, stdout: *std.ArrayList(u8), stderr: *std.ArrayList(u8), max_output_bytes: usize) !void { + // The order of the objects here is important, WaitForMultipleObjects + // uses the same order when scanning the events. + var wait_objects = [_]windows.kernel32.HANDLE{ + child.handle, child.stdout.?.handle, child.stderr.?.handle, + }; + // XXX: Calling zeroes([2]windows.OVERLAPPED) causes the stage1 compiler + // to crash and burn. + var overlapped = [_]windows.OVERLAPPED{ + mem.zeroes(windows.OVERLAPPED), + mem.zeroes(windows.OVERLAPPED), + }; + var temp_buf: [2][4096]u8 = undefined; + + // Kickstart the loop by issuing two async reads. + // ReadFile returns false and GetLastError returns ERROR_IO_PENDING if + // everything is ok. + _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); + _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); + + while (true) { + const status = windows.kernel32.WaitForMultipleObjects(wait_objects.len, &wait_objects, 0, windows.INFINITE); + std.debug.print("status {x}\n", .{status}); + switch (status) { + windows.WAIT_OBJECT_0 + 0 => { + // The child process was terminated. + break; + }, + windows.WAIT_OBJECT_0 + 1 => { + // stdout is ready. + var read_bytes: u32 = undefined; + if (windows.kernel32.GetOverlappedResult(wait_objects[1], &overlapped[0], &read_bytes, 0) == 0) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + try stdout.appendSlice(temp_buf[0][0..read_bytes]); + _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); + }, + windows.WAIT_OBJECT_0 + 2 => { + // stderr is ready. + var read_bytes: u32 = undefined; + if (windows.kernel32.GetOverlappedResult(wait_objects[2], &overlapped[1], &read_bytes, 0) == 0) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + try stdout.appendSlice(temp_buf[1][0..read_bytes]); + _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); + }, + windows.WAIT_FAILED => { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + }, + // We're waiting with an infinite timeout + windows.WAIT_TIMEOUT => unreachable, + else => unreachable, + } + } + } + /// 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. pub fn exec(args: struct { @@ -308,6 +370,14 @@ pub const ChildProcess = struct { try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes); + // XXX: Respect max_output_bytes + // XXX: Smarter reading logic, read directly into the ArrayList + if (builtin.os.tag == .windows) { + try collectOutputWindows(child, &stdout, &stderr, args.max_output_bytes); + } else { + try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes); + } + return ExecResult{ .term = try child.wait(), .stdout = stdout.toOwnedSlice(), @@ -644,7 +714,7 @@ pub const ChildProcess = struct { var g_hChildStd_OUT_Wr: ?windows.HANDLE = null; switch (self.stdout_behavior) { StdIo.Pipe => { - try windowsMakePipeOut(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr); + try windowsMakePipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr); }, StdIo.Ignore => { g_hChildStd_OUT_Wr = nul_handle; @@ -664,7 +734,7 @@ pub const ChildProcess = struct { var g_hChildStd_ERR_Wr: ?windows.HANDLE = null; switch (self.stderr_behavior) { StdIo.Pipe => { - try windowsMakePipeOut(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr); + try windowsMakePipe(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr); }, StdIo.Ignore => { g_hChildStd_ERR_Wr = nul_handle; @@ -897,6 +967,55 @@ fn windowsDestroyPipe(rd: ?windows.HANDLE, wr: ?windows.HANDLE) void { if (wr) |h| os.close(h); } +var pipe_name_counter = std.atomic.Int(u32).init(1); + +fn windowsMakePipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { + var tmp_buf: [128]u8 = undefined; + // Forge a random path for the pipe. + const pipe_path = std.fmt.bufPrintZ( + &tmp_buf, + "\\\\.\\pipe\\zig-childprocess-{d}-{d}", + .{ windows.kernel32.GetCurrentProcessId(), pipe_name_counter.fetchAdd(1) }, + ) catch unreachable; + + // Create the read handle that can be used with overlapped IO ops. + const read_handle = windows.kernel32.CreateNamedPipeA( + pipe_path, + windows.PIPE_ACCESS_INBOUND | windows.FILE_FLAG_OVERLAPPED, + windows.PIPE_TYPE_BYTE, + 1, + 0x1000, + 0x1000, + 0, + sattr, + ); + if (read_handle == windows.INVALID_HANDLE_VALUE) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + + const write_handle = windows.kernel32.CreateFileA( + pipe_path, + windows.GENERIC_WRITE, + 0, + sattr, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL, + null, + ); + if (write_handle == windows.INVALID_HANDLE_VALUE) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + + try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); + + rd.* = read_handle; + wr.* = write_handle; +} + fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { var rd_h: windows.HANDLE = undefined; var wr_h: windows.HANDLE = undefined; diff --git a/lib/std/os/windows/bits.zig b/lib/std/os/windows/bits.zig index 977c63cc5b..59a2a87415 100644 --- a/lib/std/os/windows/bits.zig +++ b/lib/std/os/windows/bits.zig @@ -448,6 +448,19 @@ pub const SECURITY_ATTRIBUTES = extern struct { pub const PSECURITY_ATTRIBUTES = *SECURITY_ATTRIBUTES; pub const LPSECURITY_ATTRIBUTES = *SECURITY_ATTRIBUTES; +pub const PIPE_ACCESS_INBOUND = 0x00000001; +pub const PIPE_ACCESS_OUTBOUND = 0x00000002; +pub const PIPE_ACCESS_DUPLEX = 0x00000003; + +pub const PIPE_TYPE_BYTE = 0x00000000; +pub const PIPE_TYPE_MESSAGE = 0x00000004; + +pub const PIPE_READMODE_BYTE = 0x00000000; +pub const PIPE_READMODE_MESSAGE = 0x00000002; + +pub const PIPE_WAIT = 0x00000000; +pub const PIPE_NOWAIT = 0x00000001; + pub const GENERIC_READ = 0x80000000; pub const GENERIC_WRITE = 0x40000000; pub const GENERIC_EXECUTE = 0x20000000; diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index 8f100a9511..1ce3bd004c 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -15,6 +15,29 @@ pub extern "kernel32" fn CloseHandle(hObject: HANDLE) callconv(WINAPI) BOOL; pub extern "kernel32" fn CreateDirectoryW(lpPathName: [*:0]const u16, lpSecurityAttributes: ?*SECURITY_ATTRIBUTES) callconv(WINAPI) BOOL; pub extern "kernel32" fn SetEndOfFile(hFile: HANDLE) callconv(WINAPI) BOOL; +pub extern "kernel32" fn GetCurrentProcessId() callconv(.Stdcall) DWORD; + +pub extern "kernel32" fn CreateNamedPipeA( + lpName: [*:0]const u8, + dwOpenMode: DWORD, + dwPipeMode: DWORD, + nMaxInstances: DWORD, + nOutBufferSize: DWORD, + nInBufferSize: DWORD, + nDefaultTimeOut: DWORD, + lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, +) callconv(.Stdcall) HANDLE; +pub extern "kernel32" fn CreateNamedPipeW( + lpName: LPCWSTR, + dwOpenMode: DWORD, + dwPipeMode: DWORD, + nMaxInstances: DWORD, + nOutBufferSize: DWORD, + nInBufferSize: DWORD, + nDefaultTimeOut: DWORD, + lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, +) callconv(.Stdcall) HANDLE; + pub extern "kernel32" fn CreateEventExW( lpEventAttributes: ?*SECURITY_ATTRIBUTES, lpName: [*:0]const u16, @@ -32,6 +55,16 @@ pub extern "kernel32" fn CreateFileW( hTemplateFile: ?HANDLE, ) callconv(WINAPI) HANDLE; +pub extern "kernel32" fn CreateFileA( + lpFileName: [*:0]const u8, + dwDesiredAccess: DWORD, + dwShareMode: DWORD, + lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, + dwCreationDisposition: DWORD, + dwFlagsAndAttributes: DWORD, + hTemplateFile: ?HANDLE, +) callconv(.Stdcall) HANDLE; + pub extern "kernel32" fn CreatePipe( hReadPipe: *HANDLE, hWritePipe: *HANDLE, From b590195222ba560b8201f2344d7fcb398a43a504 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 21 Oct 2020 16:54:38 +0200 Subject: [PATCH 2/4] std: Uniform polling logic for Windows and Unix Keep polling until there are enough open handles, if the child process terminates closing the handles or explicitly closes them we just quit polling and wait for the process handle to signal the termination condition. --- lib/std/child_process.zig | 49 +++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 936609d309..644a17c1dc 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -258,11 +258,11 @@ pub const ChildProcess = struct { } fn collectOutputWindows(child: *const ChildProcess, stdout: *std.ArrayList(u8), stderr: *std.ArrayList(u8), max_output_bytes: usize) !void { - // The order of the objects here is important, WaitForMultipleObjects - // uses the same order when scanning the events. var wait_objects = [_]windows.kernel32.HANDLE{ - child.handle, child.stdout.?.handle, child.stderr.?.handle, + child.stdout.?.handle, child.stderr.?.handle, }; + var waiting_objects: u32 = wait_objects.len; + // XXX: Calling zeroes([2]windows.OVERLAPPED) causes the stage1 compiler // to crash and burn. var overlapped = [_]windows.OVERLAPPED{ @@ -274,38 +274,31 @@ pub const ChildProcess = struct { // Kickstart the loop by issuing two async reads. // ReadFile returns false and GetLastError returns ERROR_IO_PENDING if // everything is ok. - _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); - _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); + _ = windows.kernel32.ReadFile(wait_objects[0], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); + _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); - while (true) { - const status = windows.kernel32.WaitForMultipleObjects(wait_objects.len, &wait_objects, 0, windows.INFINITE); - std.debug.print("status {x}\n", .{status}); + poll: while (waiting_objects > 0) { + const status = windows.kernel32.WaitForMultipleObjects(waiting_objects, &wait_objects, 0, windows.INFINITE); switch (status) { - windows.WAIT_OBJECT_0 + 0 => { - // The child process was terminated. - break; - }, - windows.WAIT_OBJECT_0 + 1 => { - // stdout is ready. + windows.WAIT_OBJECT_0 + 0...windows.WAIT_OBJECT_0 + 1 => { + // stdout (or stderr) is ready. + const object = status - windows.WAIT_OBJECT_0; + var read_bytes: u32 = undefined; - if (windows.kernel32.GetOverlappedResult(wait_objects[1], &overlapped[0], &read_bytes, 0) == 0) { + if (windows.kernel32.GetOverlappedResult(wait_objects[object], &overlapped[object], &read_bytes, 0) == 0) { switch (windows.kernel32.GetLastError()) { + .BROKEN_PIPE => { + // Move it to the end to remove it. + if (object != waiting_objects - 1) + mem.swap(windows.kernel32.HANDLE, &wait_objects[object], &wait_objects[waiting_objects - 1]); + waiting_objects -= 1; + continue :poll; + }, else => |err| return windows.unexpectedError(err), } } - try stdout.appendSlice(temp_buf[0][0..read_bytes]); - _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); - }, - windows.WAIT_OBJECT_0 + 2 => { - // stderr is ready. - var read_bytes: u32 = undefined; - if (windows.kernel32.GetOverlappedResult(wait_objects[2], &overlapped[1], &read_bytes, 0) == 0) { - switch (windows.kernel32.GetLastError()) { - else => |err| return windows.unexpectedError(err), - } - } - try stdout.appendSlice(temp_buf[1][0..read_bytes]); - _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); + try stdout.appendSlice(temp_buf[object][0..read_bytes]); + _ = windows.kernel32.ReadFile(wait_objects[object], &temp_buf[object], temp_buf[object].len, null, &overlapped[object]); }, windows.WAIT_FAILED => { switch (windows.kernel32.GetLastError()) { From 1e0d68e6fbfe6d5d759ad773b656c5a5acba7a5e Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 6 Dec 2020 10:50:06 +0100 Subject: [PATCH 3/4] std: Use WINAPI instead of .Stdcall --- lib/std/os/windows/kernel32.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index 1ce3bd004c..f847a34162 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -15,7 +15,7 @@ pub extern "kernel32" fn CloseHandle(hObject: HANDLE) callconv(WINAPI) BOOL; pub extern "kernel32" fn CreateDirectoryW(lpPathName: [*:0]const u16, lpSecurityAttributes: ?*SECURITY_ATTRIBUTES) callconv(WINAPI) BOOL; pub extern "kernel32" fn SetEndOfFile(hFile: HANDLE) callconv(WINAPI) BOOL; -pub extern "kernel32" fn GetCurrentProcessId() callconv(.Stdcall) DWORD; +pub extern "kernel32" fn GetCurrentProcessId() callconv(WINAPI) DWORD; pub extern "kernel32" fn CreateNamedPipeA( lpName: [*:0]const u8, @@ -26,7 +26,7 @@ pub extern "kernel32" fn CreateNamedPipeA( nInBufferSize: DWORD, nDefaultTimeOut: DWORD, lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, -) callconv(.Stdcall) HANDLE; +) callconv(WINAPI) HANDLE; pub extern "kernel32" fn CreateNamedPipeW( lpName: LPCWSTR, dwOpenMode: DWORD, @@ -36,7 +36,7 @@ pub extern "kernel32" fn CreateNamedPipeW( nInBufferSize: DWORD, nDefaultTimeOut: DWORD, lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, -) callconv(.Stdcall) HANDLE; +) callconv(WINAPI) HANDLE; pub extern "kernel32" fn CreateEventExW( lpEventAttributes: ?*SECURITY_ATTRIBUTES, @@ -63,7 +63,7 @@ pub extern "kernel32" fn CreateFileA( dwCreationDisposition: DWORD, dwFlagsAndAttributes: DWORD, hTemplateFile: ?HANDLE, -) callconv(.Stdcall) HANDLE; +) callconv(WINAPI) HANDLE; pub extern "kernel32" fn CreatePipe( hReadPipe: *HANDLE, From 9e0338b82e45f672975bf7daa1ade5f4b2de4c01 Mon Sep 17 00:00:00 2001 From: Jonathan Marler Date: Thu, 17 Jun 2021 17:36:42 -0600 Subject: [PATCH 4/4] finish ChildProcess collectOutputWindows This finishes LemonBoy's Draft PR ziglang#6750. It updates ChildProcess to collect the output from stdout/stderr asynchronously using Overlapped IO and named pipes. --- lib/std/child_process.zig | 228 +++++++++++++++++--------------- lib/std/os/windows/kernel32.zig | 47 ++----- 2 files changed, 136 insertions(+), 139 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 644a17c1dc..b63153e904 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -13,6 +13,7 @@ const process = std.process; const File = std.fs.File; const windows = os.windows; const mem = std.mem; +const math = std.math; const debug = std.debug; const BufMap = std.BufMap; const builtin = std.builtin; @@ -257,58 +258,76 @@ pub const ChildProcess = struct { } } - fn collectOutputWindows(child: *const ChildProcess, stdout: *std.ArrayList(u8), stderr: *std.ArrayList(u8), max_output_bytes: usize) !void { - var wait_objects = [_]windows.kernel32.HANDLE{ - child.stdout.?.handle, child.stderr.?.handle, + fn collectOutputWindows(child: *const ChildProcess, outs: [2]*std.ArrayList(u8), max_output_bytes: usize) !void { + const bump_amt = 512; + const handles = [_]windows.HANDLE{ + child.stdout.?.handle, + child.stderr.?.handle, }; - var waiting_objects: u32 = wait_objects.len; - // XXX: Calling zeroes([2]windows.OVERLAPPED) causes the stage1 compiler - // to crash and burn. var overlapped = [_]windows.OVERLAPPED{ mem.zeroes(windows.OVERLAPPED), mem.zeroes(windows.OVERLAPPED), }; - var temp_buf: [2][4096]u8 = undefined; - // Kickstart the loop by issuing two async reads. - // ReadFile returns false and GetLastError returns ERROR_IO_PENDING if - // everything is ok. - _ = windows.kernel32.ReadFile(wait_objects[0], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]); - _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]); + var wait_objects: [2]windows.HANDLE = undefined; + var wait_object_count: u2 = 0; - poll: while (waiting_objects > 0) { - const status = windows.kernel32.WaitForMultipleObjects(waiting_objects, &wait_objects, 0, windows.INFINITE); - switch (status) { - windows.WAIT_OBJECT_0 + 0...windows.WAIT_OBJECT_0 + 1 => { - // stdout (or stderr) is ready. - const object = status - windows.WAIT_OBJECT_0; + // we need to cancel all pending IO before returning so our OVERLAPPED values don't go out of scope + defer for (wait_objects[0..wait_object_count]) |o| { + _ = windows.kernel32.CancelIo(o); + }; - var read_bytes: u32 = undefined; - if (windows.kernel32.GetOverlappedResult(wait_objects[object], &overlapped[object], &read_bytes, 0) == 0) { - switch (windows.kernel32.GetLastError()) { - .BROKEN_PIPE => { - // Move it to the end to remove it. - if (object != waiting_objects - 1) - mem.swap(windows.kernel32.HANDLE, &wait_objects[object], &wait_objects[waiting_objects - 1]); - waiting_objects -= 1; - continue :poll; - }, - else => |err| return windows.unexpectedError(err), - } - } - try stdout.appendSlice(temp_buf[object][0..read_bytes]); - _ = windows.kernel32.ReadFile(wait_objects[object], &temp_buf[object], temp_buf[object].len, null, &overlapped[object]); - }, - windows.WAIT_FAILED => { - switch (windows.kernel32.GetLastError()) { - else => |err| return windows.unexpectedError(err), - } - }, - // We're waiting with an infinite timeout - windows.WAIT_TIMEOUT => unreachable, - else => unreachable, + // Windows Async IO requires an initial call to ReadFile before waiting on the handle + for ([_]u1{ 0, 1 }) |i| { + try outs[i].ensureCapacity(bump_amt); + const buf = outs[i].unusedCapacitySlice(); + _ = windows.kernel32.ReadFile(handles[i], buf.ptr, math.cast(u32, buf.len) catch maxInt(u32), null, &overlapped[i]); + wait_objects[wait_object_count] = handles[i]; + wait_object_count += 1; + } + + while (true) { + const status = windows.kernel32.WaitForMultipleObjects(wait_object_count, &wait_objects, 0, windows.INFINITE); + if (status == windows.WAIT_FAILED) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } } + if (status < windows.WAIT_OBJECT_0 or status > windows.WAIT_OBJECT_0 + wait_object_count - 1) + unreachable; + + const wait_idx = status - windows.WAIT_OBJECT_0; + + // this extra `i` index is needed to map the wait handle back to the stdout or stderr + // values since the wait_idx can change which handle it corresponds with + const i: u1 = if (wait_objects[wait_idx] == handles[0]) 0 else 1; + + // remove completed event from the wait list + wait_object_count -= 1; + if (wait_idx == 0) + wait_objects[0] = wait_objects[1]; + + var read_bytes: u32 = undefined; + if (windows.kernel32.GetOverlappedResult(handles[i], &overlapped[i], &read_bytes, 0) == 0) { + switch (windows.kernel32.GetLastError()) { + .BROKEN_PIPE => { + if (wait_object_count == 0) + break; + continue; + }, + else => |err| return windows.unexpectedError(err), + } + } + + outs[i].items.len += read_bytes; + const new_capacity = std.math.min(outs[i].items.len + bump_amt, max_output_bytes); + try outs[i].ensureCapacity(new_capacity); + const buf = outs[i].unusedCapacitySlice(); + if (buf.len == 0) return if (i == 0) error.StdoutStreamTooLong else error.StderrStreamTooLong; + _ = windows.kernel32.ReadFile(handles[i], buf.ptr, math.cast(u32, buf.len) catch maxInt(u32), null, &overlapped[i]); + wait_objects[wait_object_count] = handles[i]; + wait_object_count += 1; } } @@ -361,12 +380,8 @@ pub const ChildProcess = struct { stderr.deinit(); } - try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes); - - // XXX: Respect max_output_bytes - // XXX: Smarter reading logic, read directly into the ArrayList if (builtin.os.tag == .windows) { - try collectOutputWindows(child, &stdout, &stderr, args.max_output_bytes); + try collectOutputWindows(child, [_]*std.ArrayList(u8){ &stdout, &stderr }, args.max_output_bytes); } else { try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes); } @@ -707,7 +722,7 @@ pub const ChildProcess = struct { var g_hChildStd_OUT_Wr: ?windows.HANDLE = null; switch (self.stdout_behavior) { StdIo.Pipe => { - try windowsMakePipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr); + try windowsMakeAsyncPipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr); }, StdIo.Ignore => { g_hChildStd_OUT_Wr = nul_handle; @@ -727,7 +742,7 @@ pub const ChildProcess = struct { var g_hChildStd_ERR_Wr: ?windows.HANDLE = null; switch (self.stderr_behavior) { StdIo.Pipe => { - try windowsMakePipe(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr); + try windowsMakeAsyncPipe(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr); }, StdIo.Ignore => { g_hChildStd_ERR_Wr = nul_handle; @@ -960,55 +975,6 @@ fn windowsDestroyPipe(rd: ?windows.HANDLE, wr: ?windows.HANDLE) void { if (wr) |h| os.close(h); } -var pipe_name_counter = std.atomic.Int(u32).init(1); - -fn windowsMakePipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { - var tmp_buf: [128]u8 = undefined; - // Forge a random path for the pipe. - const pipe_path = std.fmt.bufPrintZ( - &tmp_buf, - "\\\\.\\pipe\\zig-childprocess-{d}-{d}", - .{ windows.kernel32.GetCurrentProcessId(), pipe_name_counter.fetchAdd(1) }, - ) catch unreachable; - - // Create the read handle that can be used with overlapped IO ops. - const read_handle = windows.kernel32.CreateNamedPipeA( - pipe_path, - windows.PIPE_ACCESS_INBOUND | windows.FILE_FLAG_OVERLAPPED, - windows.PIPE_TYPE_BYTE, - 1, - 0x1000, - 0x1000, - 0, - sattr, - ); - if (read_handle == windows.INVALID_HANDLE_VALUE) { - switch (windows.kernel32.GetLastError()) { - else => |err| return windows.unexpectedError(err), - } - } - - const write_handle = windows.kernel32.CreateFileA( - pipe_path, - windows.GENERIC_WRITE, - 0, - sattr, - windows.OPEN_EXISTING, - windows.FILE_ATTRIBUTE_NORMAL, - null, - ); - if (write_handle == windows.INVALID_HANDLE_VALUE) { - switch (windows.kernel32.GetLastError()) { - else => |err| return windows.unexpectedError(err), - } - } - - try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); - - rd.* = read_handle; - wr.* = write_handle; -} - fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { var rd_h: windows.HANDLE = undefined; var wr_h: windows.HANDLE = undefined; @@ -1019,14 +985,64 @@ fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const w wr.* = wr_h; } -fn windowsMakePipeOut(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { - var rd_h: windows.HANDLE = undefined; - var wr_h: windows.HANDLE = undefined; - try windows.CreatePipe(&rd_h, &wr_h, sattr); - errdefer windowsDestroyPipe(rd_h, wr_h); - try windows.SetHandleInformation(rd_h, windows.HANDLE_FLAG_INHERIT, 0); - rd.* = rd_h; - wr.* = wr_h; +var pipe_name_counter = std.atomic.Atomic(u32).init(1); + +fn windowsMakeAsyncPipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void { + var tmp_bufw: [128]u16 = undefined; + + // We must make a named pipe on windows because anonymous pipes do not support async IO + const pipe_path = blk: { + var tmp_buf: [128]u8 = undefined; + // Forge a random path for the pipe. + const pipe_path = std.fmt.bufPrintZ( + &tmp_buf, + "\\\\.\\pipe\\zig-childprocess-{d}-{d}", + .{ windows.kernel32.GetCurrentProcessId(), pipe_name_counter.fetchAdd(1, .Monotonic) }, + ) catch unreachable; + const len = std.unicode.utf8ToUtf16Le(&tmp_bufw, pipe_path) catch unreachable; + tmp_bufw[len] = 0; + break :blk tmp_bufw[0..len :0]; + }; + + // Create the read handle that can be used with overlapped IO ops. + const read_handle = windows.kernel32.CreateNamedPipeW( + pipe_path.ptr, + windows.PIPE_ACCESS_INBOUND | windows.FILE_FLAG_OVERLAPPED, + windows.PIPE_TYPE_BYTE, + 1, + 4096, + 4096, + 0, + sattr, + ); + if (read_handle == windows.INVALID_HANDLE_VALUE) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + errdefer os.close(read_handle); + + var sattr_copy = sattr.*; + const write_handle = windows.kernel32.CreateFileW( + pipe_path.ptr, + windows.GENERIC_WRITE, + 0, + &sattr_copy, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL, + null, + ); + if (write_handle == windows.INVALID_HANDLE_VALUE) { + switch (windows.kernel32.GetLastError()) { + else => |err| return windows.unexpectedError(err), + } + } + errdefer os.close(write_handle); + + try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0); + + rd.* = read_handle; + wr.* = write_handle; } fn destroyPipe(pipe: [2]os.fd_t) void { diff --git a/lib/std/os/windows/kernel32.zig b/lib/std/os/windows/kernel32.zig index f847a34162..971273ef3a 100644 --- a/lib/std/os/windows/kernel32.zig +++ b/lib/std/os/windows/kernel32.zig @@ -8,6 +8,7 @@ usingnamespace @import("bits.zig"); pub extern "kernel32" fn AddVectoredExceptionHandler(First: c_ulong, Handler: ?VECTORED_EXCEPTION_HANDLER) callconv(WINAPI) ?*c_void; pub extern "kernel32" fn RemoveVectoredExceptionHandler(Handle: HANDLE) callconv(WINAPI) c_ulong; +pub extern "kernel32" fn CancelIo(hFile: HANDLE) callconv(WINAPI) BOOL; pub extern "kernel32" fn CancelIoEx(hFile: HANDLE, lpOverlapped: ?LPOVERLAPPED) callconv(WINAPI) BOOL; pub extern "kernel32" fn CloseHandle(hObject: HANDLE) callconv(WINAPI) BOOL; @@ -15,29 +16,6 @@ pub extern "kernel32" fn CloseHandle(hObject: HANDLE) callconv(WINAPI) BOOL; pub extern "kernel32" fn CreateDirectoryW(lpPathName: [*:0]const u16, lpSecurityAttributes: ?*SECURITY_ATTRIBUTES) callconv(WINAPI) BOOL; pub extern "kernel32" fn SetEndOfFile(hFile: HANDLE) callconv(WINAPI) BOOL; -pub extern "kernel32" fn GetCurrentProcessId() callconv(WINAPI) DWORD; - -pub extern "kernel32" fn CreateNamedPipeA( - lpName: [*:0]const u8, - dwOpenMode: DWORD, - dwPipeMode: DWORD, - nMaxInstances: DWORD, - nOutBufferSize: DWORD, - nInBufferSize: DWORD, - nDefaultTimeOut: DWORD, - lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, -) callconv(WINAPI) HANDLE; -pub extern "kernel32" fn CreateNamedPipeW( - lpName: LPCWSTR, - dwOpenMode: DWORD, - dwPipeMode: DWORD, - nMaxInstances: DWORD, - nOutBufferSize: DWORD, - nInBufferSize: DWORD, - nDefaultTimeOut: DWORD, - lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, -) callconv(WINAPI) HANDLE; - pub extern "kernel32" fn CreateEventExW( lpEventAttributes: ?*SECURITY_ATTRIBUTES, lpName: [*:0]const u16, @@ -55,16 +33,6 @@ pub extern "kernel32" fn CreateFileW( hTemplateFile: ?HANDLE, ) callconv(WINAPI) HANDLE; -pub extern "kernel32" fn CreateFileA( - lpFileName: [*:0]const u8, - dwDesiredAccess: DWORD, - dwShareMode: DWORD, - lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, - dwCreationDisposition: DWORD, - dwFlagsAndAttributes: DWORD, - hTemplateFile: ?HANDLE, -) callconv(WINAPI) HANDLE; - pub extern "kernel32" fn CreatePipe( hReadPipe: *HANDLE, hWritePipe: *HANDLE, @@ -72,6 +40,17 @@ pub extern "kernel32" fn CreatePipe( nSize: DWORD, ) callconv(WINAPI) BOOL; +pub extern "kernel32" fn CreateNamedPipeW( + lpName: LPCWSTR, + dwOpenMode: DWORD, + dwPipeMode: DWORD, + nMaxInstances: DWORD, + nOutBufferSize: DWORD, + nInBufferSize: DWORD, + nDefaultTimeOut: DWORD, + lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES, +) callconv(WINAPI) HANDLE; + pub extern "kernel32" fn CreateProcessW( lpApplicationName: ?LPWSTR, lpCommandLine: LPWSTR, @@ -132,6 +111,8 @@ pub extern "kernel32" fn GetCurrentDirectoryW(nBufferLength: DWORD, lpBuffer: ?[ pub extern "kernel32" fn GetCurrentThread() callconv(WINAPI) HANDLE; pub extern "kernel32" fn GetCurrentThreadId() callconv(WINAPI) DWORD; +pub extern "kernel32" fn GetCurrentProcessId() callconv(WINAPI) DWORD; + pub extern "kernel32" fn GetCurrentProcess() callconv(WINAPI) HANDLE; pub extern "kernel32" fn GetEnvironmentStringsW() callconv(WINAPI) ?[*:0]u16;