From 1aeee80344387509bc898abf4789593bcfa5353a Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 5 Jun 2021 20:26:59 +0200 Subject: [PATCH] std: Better handing of POLLHUP in ChildProcess (#8988) * std: Better handing of POLLHUP in ChildProcess Upon hitting the EOF condition there are two main differences between how Linux and the *BSD-derived systems behave: the former sets POLLHUP and POLLIN and, after reading any residual data, only POLLHUP remains set. The latter signal the EOF condition by setting both flags thus requiring some extra checks to determine if the stream is "done". DragonFly workaround/hack for POLLHUP is no longer required. Closes #8969 --- lib/std/child_process.zig | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 56f3e9b1df..10bfe6d3f2 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -203,16 +203,18 @@ pub const ChildProcess = struct { // of space an ArrayList will allocate grows exponentially. const bump_amt = 512; - // TODO https://github.com/ziglang/zig/issues/8724 - // parent process does not receive POLLHUP events - const dragonfly_workaround = builtin.os.tag == .dragonfly; + const err_mask = os.POLLERR | os.POLLNVAL | os.POLLHUP; while (dead_fds < poll_fds.len) { const events = try os.poll(&poll_fds, std.math.maxInt(i32)); if (events == 0) continue; + var remove_stdout = false; + var remove_stderr = false; // Try reading whatever is available before checking the error // conditions. + // It's still possible to read after a POLLHUP is received, always + // check if there's some data waiting to be read first. if (poll_fds[0].revents & os.POLLIN != 0) { // stdout is ready. const new_capacity = std.math.min(stdout.items.len + bump_amt, max_output_bytes); @@ -222,9 +224,12 @@ pub const ChildProcess = struct { const nread = try os.read(poll_fds[0].fd, buf); stdout.items.len += nread; - // insert POLLHUP event because dragonfly fails to do so - if (dragonfly_workaround and nread == 0) poll_fds[0].revents |= os.POLLHUP; + // Remove the fd when the EOF condition is met. + remove_stdout = nread == 0; + } else { + remove_stdout = poll_fds[0].revents & err_mask != 0; } + if (poll_fds[1].revents & os.POLLIN != 0) { // stderr is ready. const new_capacity = std.math.min(stderr.items.len + bump_amt, max_output_bytes); @@ -234,16 +239,18 @@ pub const ChildProcess = struct { const nread = try os.read(poll_fds[1].fd, buf); stderr.items.len += nread; - // insert POLLHUP event because dragonfly fails to do so - if (dragonfly_workaround and nread == 0) poll_fds[1].revents |= os.POLLHUP; + // Remove the fd when the EOF condition is met. + remove_stderr = nread == 0; + } else { + remove_stderr = poll_fds[1].revents & err_mask != 0; } // Exclude the fds that signaled an error. - if (poll_fds[0].revents & (os.POLLERR | os.POLLNVAL | os.POLLHUP) != 0) { + if (remove_stdout) { poll_fds[0].fd = -1; dead_fds += 1; } - if (poll_fds[1].revents & (os.POLLERR | os.POLLNVAL | os.POLLHUP) != 0) { + if (remove_stderr) { poll_fds[1].fd = -1; dead_fds += 1; } @@ -294,6 +301,10 @@ pub const ChildProcess = struct { var stdout = std.ArrayList(u8).init(args.allocator); var stderr = std.ArrayList(u8).init(args.allocator); + errdefer { + stdout.deinit(); + stderr.deinit(); + } try collectOutputPosix(child, &stdout, &stderr, args.max_output_bytes);