authorgravatar for thatlemon@gmail.comLemonBoy <thatlemon@gmail.com> 2020-10-20 08:51:21+02:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-29 10:40:00-07:00
logdc810eb73b08edfc445f2ce043806be00a236abf
tree6062dc5dbf4ebf5948ba1bde1317e5d01487e12c
parent1590ed9d6aea95e5a21e3455e8edba4cdb374f2c

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.

3 files changed, 208 insertions(+), 11 deletions(-)

lib/std/child_process.zig+162-11
......@@ -186,6 +186,106 @@ pub const ChildProcess = struct {
186186
187187 pub const exec2 = @compileError("deprecated: exec2 is renamed to exec");
188188
189 fn collectOutputPosix(child: *const ChildProcess, stdout: *std.ArrayList(u8), stderr: *std.ArrayList(u8), max_output_bytes: usize) !void {
190 var poll_fds = [_]os.pollfd{
191 .{ .fd = child.stdout.?.handle, .events = os.POLLIN, .revents = undefined },
192 .{ .fd = child.stderr.?.handle, .events = os.POLLIN, .revents = undefined },
193 };
194
195 var dead_fds: usize = 0;
196 var loop_buf: [4096]u8 = undefined;
197
198 while (dead_fds < poll_fds.len) {
199 const events = try os.poll(&poll_fds, std.math.maxInt(i32));
200 if (events == 0) continue;
201
202 // Try reading whatever is available before checking the error
203 // conditions.
204 if (poll_fds[0].revents & os.POLLIN != 0) {
205 // stdout is ready.
206 const n = try os.read(poll_fds[0].fd, &loop_buf);
207 try stdout.appendSlice(loop_buf[0..n]);
208 }
209 if (poll_fds[1].revents & os.POLLIN != 0) {
210 // stderr is ready.
211 const n = try os.read(poll_fds[1].fd, &loop_buf);
212 try stderr.appendSlice(loop_buf[0..n]);
213 }
214
215 // Exclude the fds that signaled an error.
216 if (poll_fds[0].revents & (os.POLLERR | os.POLLNVAL | os.POLLHUP) != 0) {
217 poll_fds[0].fd = -1;
218 dead_fds += 1;
219 }
220 if (poll_fds[1].revents & (os.POLLERR | os.POLLNVAL | os.POLLHUP) != 0) {
221 poll_fds[1].fd = -1;
222 dead_fds += 1;
223 }
224 }
225 }
226
227 fn collectOutputWindows(child: *const ChildProcess, stdout: *std.ArrayList(u8), stderr: *std.ArrayList(u8), max_output_bytes: usize) !void {
228 // The order of the objects here is important, WaitForMultipleObjects
229 // uses the same order when scanning the events.
230 var wait_objects = [_]windows.kernel32.HANDLE{
231 child.handle, child.stdout.?.handle, child.stderr.?.handle,
232 };
233 // XXX: Calling zeroes([2]windows.OVERLAPPED) causes the stage1 compiler
234 // to crash and burn.
235 var overlapped = [_]windows.OVERLAPPED{
236 mem.zeroes(windows.OVERLAPPED),
237 mem.zeroes(windows.OVERLAPPED),
238 };
239 var temp_buf: [2][4096]u8 = undefined;
240
241 // Kickstart the loop by issuing two async reads.
242 // ReadFile returns false and GetLastError returns ERROR_IO_PENDING if
243 // everything is ok.
244 _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]);
245 _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]);
246
247 while (true) {
248 const status = windows.kernel32.WaitForMultipleObjects(wait_objects.len, &wait_objects, 0, windows.INFINITE);
249 std.debug.print("status {x}\n", .{status});
250 switch (status) {
251 windows.WAIT_OBJECT_0 + 0 => {
252 // The child process was terminated.
253 break;
254 },
255 windows.WAIT_OBJECT_0 + 1 => {
256 // stdout is ready.
257 var read_bytes: u32 = undefined;
258 if (windows.kernel32.GetOverlappedResult(wait_objects[1], &overlapped[0], &read_bytes, 0) == 0) {
259 switch (windows.kernel32.GetLastError()) {
260 else => |err| return windows.unexpectedError(err),
261 }
262 }
263 try stdout.appendSlice(temp_buf[0][0..read_bytes]);
264 _ = windows.kernel32.ReadFile(wait_objects[1], &temp_buf[0], temp_buf[0].len, null, &overlapped[0]);
265 },
266 windows.WAIT_OBJECT_0 + 2 => {
267 // stderr is ready.
268 var read_bytes: u32 = undefined;
269 if (windows.kernel32.GetOverlappedResult(wait_objects[2], &overlapped[1], &read_bytes, 0) == 0) {
270 switch (windows.kernel32.GetLastError()) {
271 else => |err| return windows.unexpectedError(err),
272 }
273 }
274 try stdout.appendSlice(temp_buf[1][0..read_bytes]);
275 _ = windows.kernel32.ReadFile(wait_objects[2], &temp_buf[1], temp_buf[1].len, null, &overlapped[1]);
276 },
277 windows.WAIT_FAILED => {
278 switch (windows.kernel32.GetLastError()) {
279 else => |err| return windows.unexpectedError(err),
280 }
281 },
282 // We're waiting with an infinite timeout
283 windows.WAIT_TIMEOUT => unreachable,
284 else => unreachable,
285 }
286 }
287 }
288
189289 /// Spawns a child process, waits for it, collecting stdout and stderr, and then returns.
190290 /// If it succeeds, the caller owns result.stdout and result.stderr memory.
191291 pub fn exec(args: struct {
......@@ -210,19 +310,21 @@ pub const ChildProcess = struct {
210310
211311 try child.spawn();
212312
213 const stdout_in = child.stdout.?.reader();
214 const stderr_in = child.stderr.?.reader();
313 var stdout = std.ArrayList(u8).init(args.allocator);
314 var stderr = std.ArrayList(u8).init(args.allocator);
215315
216 // TODO https://github.com/ziglang/zig/issues/6343
217 const stdout = try stdout_in.readAllAlloc(args.allocator, args.max_output_bytes);
218 errdefer args.allocator.free(stdout);
219 const stderr = try stderr_in.readAllAlloc(args.allocator, args.max_output_bytes);
220 errdefer args.allocator.free(stderr);
316 // XXX: Respect max_output_bytes
317 // XXX: Smarter reading logic, read directly into the ArrayList
318 if (builtin.os.tag == .windows) {
319 try collectOutputWindows(child, &stdout, &stderr, args.max_output_bytes);
320 } else {
321 try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes);
322 }
221323
222324 return ExecResult{
223325 .term = try child.wait(),
224 .stdout = stdout,
225 .stderr = stderr,
326 .stdout = stdout.toOwnedSlice(),
327 .stderr = stderr.toOwnedSlice(),
226328 };
227329 }
228330
......@@ -555,7 +657,7 @@ pub const ChildProcess = struct {
555657 var g_hChildStd_OUT_Wr: ?windows.HANDLE = null;
556658 switch (self.stdout_behavior) {
557659 StdIo.Pipe => {
558 try windowsMakePipeOut(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr);
660 try windowsMakePipe(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr);
559661 },
560662 StdIo.Ignore => {
561663 g_hChildStd_OUT_Wr = nul_handle;
......@@ -575,7 +677,7 @@ pub const ChildProcess = struct {
575677 var g_hChildStd_ERR_Wr: ?windows.HANDLE = null;
576678 switch (self.stderr_behavior) {
577679 StdIo.Pipe => {
578 try windowsMakePipeOut(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr);
680 try windowsMakePipe(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr);
579681 },
580682 StdIo.Ignore => {
581683 g_hChildStd_ERR_Wr = nul_handle;
......@@ -808,6 +910,55 @@ fn windowsDestroyPipe(rd: ?windows.HANDLE, wr: ?windows.HANDLE) void {
808910 if (wr) |h| os.close(h);
809911}
810912
913var pipe_name_counter = std.atomic.Int(u32).init(1);
914
915fn windowsMakePipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void {
916 var tmp_buf: [128]u8 = undefined;
917 // Forge a random path for the pipe.
918 const pipe_path = std.fmt.bufPrintZ(
919 &tmp_buf,
920 "\\\\.\\pipe\\zig-childprocess-{d}-{d}",
921 .{ windows.kernel32.GetCurrentProcessId(), pipe_name_counter.fetchAdd(1) },
922 ) catch unreachable;
923
924 // Create the read handle that can be used with overlapped IO ops.
925 const read_handle = windows.kernel32.CreateNamedPipeA(
926 pipe_path,
927 windows.PIPE_ACCESS_INBOUND | windows.FILE_FLAG_OVERLAPPED,
928 windows.PIPE_TYPE_BYTE,
929 1,
930 0x1000,
931 0x1000,
932 0,
933 sattr,
934 );
935 if (read_handle == windows.INVALID_HANDLE_VALUE) {
936 switch (windows.kernel32.GetLastError()) {
937 else => |err| return windows.unexpectedError(err),
938 }
939 }
940
941 const write_handle = windows.kernel32.CreateFileA(
942 pipe_path,
943 windows.GENERIC_WRITE,
944 0,
945 sattr,
946 windows.OPEN_EXISTING,
947 windows.FILE_ATTRIBUTE_NORMAL,
948 null,
949 );
950 if (write_handle == windows.INVALID_HANDLE_VALUE) {
951 switch (windows.kernel32.GetLastError()) {
952 else => |err| return windows.unexpectedError(err),
953 }
954 }
955
956 try windows.SetHandleInformation(read_handle, windows.HANDLE_FLAG_INHERIT, 0);
957
958 rd.* = read_handle;
959 wr.* = write_handle;
960}
961
811962fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const windows.SECURITY_ATTRIBUTES) !void {
812963 var rd_h: windows.HANDLE = undefined;
813964 var wr_h: windows.HANDLE = undefined;
lib/std/os/windows/bits.zig+13
......@@ -438,6 +438,19 @@ pub const SECURITY_ATTRIBUTES = extern struct {
438438pub const PSECURITY_ATTRIBUTES = *SECURITY_ATTRIBUTES;
439439pub const LPSECURITY_ATTRIBUTES = *SECURITY_ATTRIBUTES;
440440
441pub const PIPE_ACCESS_INBOUND = 0x00000001;
442pub const PIPE_ACCESS_OUTBOUND = 0x00000002;
443pub const PIPE_ACCESS_DUPLEX = 0x00000003;
444
445pub const PIPE_TYPE_BYTE = 0x00000000;
446pub const PIPE_TYPE_MESSAGE = 0x00000004;
447
448pub const PIPE_READMODE_BYTE = 0x00000000;
449pub const PIPE_READMODE_MESSAGE = 0x00000002;
450
451pub const PIPE_WAIT = 0x00000000;
452pub const PIPE_NOWAIT = 0x00000001;
453
441454pub const GENERIC_READ = 0x80000000;
442455pub const GENERIC_WRITE = 0x40000000;
443456pub const GENERIC_EXECUTE = 0x20000000;
lib/std/os/windows/kernel32.zig+33
......@@ -15,6 +15,29 @@ pub extern "kernel32" fn CloseHandle(hObject: HANDLE) callconv(WINAPI) BOOL;
1515pub extern "kernel32" fn CreateDirectoryW(lpPathName: [*:0]const u16, lpSecurityAttributes: ?*SECURITY_ATTRIBUTES) callconv(WINAPI) BOOL;
1616pub extern "kernel32" fn SetEndOfFile(hFile: HANDLE) callconv(WINAPI) BOOL;
1717
18pub extern "kernel32" fn GetCurrentProcessId() callconv(.Stdcall) DWORD;
19
20pub extern "kernel32" fn CreateNamedPipeA(
21 lpName: [*:0]const u8,
22 dwOpenMode: DWORD,
23 dwPipeMode: DWORD,
24 nMaxInstances: DWORD,
25 nOutBufferSize: DWORD,
26 nInBufferSize: DWORD,
27 nDefaultTimeOut: DWORD,
28 lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES,
29) callconv(.Stdcall) HANDLE;
30pub extern "kernel32" fn CreateNamedPipeW(
31 lpName: LPCWSTR,
32 dwOpenMode: DWORD,
33 dwPipeMode: DWORD,
34 nMaxInstances: DWORD,
35 nOutBufferSize: DWORD,
36 nInBufferSize: DWORD,
37 nDefaultTimeOut: DWORD,
38 lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES,
39) callconv(.Stdcall) HANDLE;
40
1841pub extern "kernel32" fn CreateEventExW(
1942 lpEventAttributes: ?*SECURITY_ATTRIBUTES,
2043 lpName: [*:0]const u16,
......@@ -32,6 +55,16 @@ pub extern "kernel32" fn CreateFileW(
3255 hTemplateFile: ?HANDLE,
3356) callconv(WINAPI) HANDLE;
3457
58pub extern "kernel32" fn CreateFileA(
59 lpFileName: [*:0]const u8,
60 dwDesiredAccess: DWORD,
61 dwShareMode: DWORD,
62 lpSecurityAttributes: ?*const SECURITY_ATTRIBUTES,
63 dwCreationDisposition: DWORD,
64 dwFlagsAndAttributes: DWORD,
65 hTemplateFile: ?HANDLE,
66) callconv(.Stdcall) HANDLE;
67
3568pub extern "kernel32" fn CreatePipe(
3669 hReadPipe: *HANDLE,
3770 hWritePipe: *HANDLE,