From c257f892fdeda9c738d4a3afdf80a12775269884 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Dec 2019 10:56:53 +0100 Subject: [PATCH 1/2] Revert "Revert "Use eventfd in ChildProcess on Linux"" This reverts commit 2c7a2aefbfd0dbab190f912b4fbcbda96fb5ac44. --- lib/std/child_process.zig | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index 62b134bcf..a7ca89c6a 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -280,10 +280,7 @@ pub const ChildProcess = struct { } fn cleanupAfterWait(self: *ChildProcess, status: u32) !Term { - defer { - os.close(self.err_pipe[0]); - os.close(self.err_pipe[1]); - } + defer destroyPipe(self.err_pipe); // Write maxInt(ErrInt) to the write end of the err_pipe. This is after // waitpid, so this write is guaranteed to be after the child @@ -359,7 +356,16 @@ pub const ChildProcess = struct { // 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 = try os.pipe(); + const err_pipe = blk: { + if (builtin.os == .linux) { + const fd = try os.eventfd(0, 0); + // There's no distinction between the readable and the writeable + // end with eventfd + break :blk [2]os.fd_t{ fd, fd }; + } else { + break :blk try os.pipe(); + } + }; errdefer destroyPipe(err_pipe); const pid_result = try os.fork(); @@ -773,7 +779,7 @@ fn windowsMakePipeOut(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const fn destroyPipe(pipe: [2]os.fd_t) void { os.close(pipe[0]); - os.close(pipe[1]); + if (pipe[0] != pipe[1]) os.close(pipe[1]); } // Child of fork calls this to report an error to the fork parent. @@ -787,12 +793,12 @@ const ErrInt = @IntType(false, @sizeOf(anyerror) * 8); fn writeIntFd(fd: i32, value: ErrInt) !void { const stream = &File.openHandle(fd).outStream().stream; - stream.writeIntNative(ErrInt, value) catch return error.SystemResources; + stream.writeIntNative(u64, @intCast(u64, value)) catch return error.SystemResources; } fn readIntFd(fd: i32) !ErrInt { const stream = &File.openHandle(fd).inStream().stream; - return stream.readIntNative(ErrInt) catch return error.SystemResources; + return @intCast(ErrInt, stream.readIntNative(u64) catch return error.SystemResources); } /// Caller must free result. From 9d9b0720f52059a5b18fdf313cb80fca6379e54d Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Dec 2019 10:57:29 +0100 Subject: [PATCH 2/2] Fix for the error codepath in ChildProcess --- lib/std/child_process.zig | 41 ++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/std/child_process.zig b/lib/std/child_process.zig index a7ca89c6a..a76dfd352 100644 --- a/lib/std/child_process.zig +++ b/lib/std/child_process.zig @@ -282,17 +282,36 @@ pub const ChildProcess = struct { fn cleanupAfterWait(self: *ChildProcess, status: u32) !Term { defer destroyPipe(self.err_pipe); - // Write maxInt(ErrInt) to the write end of the err_pipe. This is after - // waitpid, so this write is guaranteed to be after the child - // pid potentially wrote an error. This way we can do a blocking - // read on the error pipe and either get maxInt(ErrInt) (no error) or - // an error code. - try writeIntFd(self.err_pipe[1], maxInt(ErrInt)); - const err_int = try readIntFd(self.err_pipe[0]); - // Here we potentially return the fork child's error - // from the parent pid. - if (err_int != maxInt(ErrInt)) { - return @errSetCast(SpawnError, @intToError(err_int)); + if (builtin.os == .linux) { + var fd = [1]std.os.pollfd{std.os.pollfd{ + .fd = self.err_pipe[0], + .events = std.os.POLLIN, + .revents = undefined, + }}; + + // Check if the eventfd buffer stores a non-zero value by polling + // it, that's the error code returned by the child process. + _ = std.os.poll(&fd, 0) catch unreachable; + + // According to eventfd(2) the descriptro is readable if the counter + // has a value greater than 0 + if ((fd[0].revents & std.os.POLLIN) != 0) { + const err_int = try readIntFd(self.err_pipe[0]); + return @errSetCast(SpawnError, @intToError(err_int)); + } + } else { + // Write maxInt(ErrInt) to the write end of the err_pipe. This is after + // waitpid, so this write is guaranteed to be after the child + // pid potentially wrote an error. This way we can do a blocking + // read on the error pipe and either get maxInt(ErrInt) (no error) or + // an error code. + try writeIntFd(self.err_pipe[1], maxInt(ErrInt)); + const err_int = try readIntFd(self.err_pipe[0]); + // Here we potentially return the fork child's error from the parent + // pid. + if (err_int != maxInt(ErrInt)) { + return @errSetCast(SpawnError, @intToError(err_int)); + } } return statusToTerm(status);