std.child_process: enable non-standard streams

lib/std/child_process.zig

@@ -830,7 +830,7 @@ pub const ChildProcess = struct {
     fn spawnWindows(self: *ChildProcess) SpawnError!void {
         const saAttr = windows.SECURITY_ATTRIBUTES{
             .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES),
-            .bInheritHandle = windows.TRUE,
+            .bInheritHandle = windows.FALSE,
             .lpSecurityDescriptor = null,
         };
 
@@ -881,11 +881,24 @@ pub const ChildProcess = struct {
             windowsDestroyPipe(g_hChildStd_IN_Rd, g_hChildStd_IN_Wr);
         };
 
+        var tmp_hChildStd_Rd: windows.HANDLE = undefined;
+        var tmp_hChildStd_Wr: windows.HANDLE = undefined;
         var g_hChildStd_OUT_Rd: ?windows.HANDLE = null;
         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(
+                    &tmp_hChildStd_Rd,
+                    &tmp_hChildStd_Wr,
+                    &saAttr,
+                );
+                errdefer {
+                    os.close(tmp_hChildStd_Rd);
+                    os.close(tmp_hChildStd_Wr);
+                }
+                try windows.SetHandleInformation(tmp_hChildStd_Wr, windows.HANDLE_FLAG_INHERIT, 1);
+                g_hChildStd_OUT_Rd = tmp_hChildStd_Rd;
+                g_hChildStd_OUT_Wr = tmp_hChildStd_Wr;
             },
             StdIo.Ignore => {
                 g_hChildStd_OUT_Wr = nul_handle;
@@ -905,7 +918,18 @@ 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(
+                    &tmp_hChildStd_Rd,
+                    &tmp_hChildStd_Wr,
+                    &saAttr,
+                );
+                errdefer {
+                    os.close(tmp_hChildStd_Rd);
+                    os.close(tmp_hChildStd_Wr);
+                }
+                try windows.SetHandleInformation(tmp_hChildStd_Wr, windows.HANDLE_FLAG_INHERIT, 1);
+                g_hChildStd_ERR_Rd = tmp_hChildStd_Rd;
+                g_hChildStd_ERR_Wr = tmp_hChildStd_Wr;
             },
             StdIo.Ignore => {
                 g_hChildStd_ERR_Wr = nul_handle;
@@ -1103,6 +1127,28 @@ pub const ChildProcess = struct {
     }
 };
 
+/// Pipe read side
+pub const pipe_rd = 0;
+/// Pipe write side
+pub const pipe_wr = 1;
+const PortPipeT = if (builtin.os.tag == .windows) [2]windows.HANDLE else [2]os.fd_t;
+
+/// Portable pipe creation with disabled inheritance
+pub inline fn portablePipe() !PortPipeT {
+    var pipe_new: PortPipeT = undefined;
+    if (builtin.os.tag == .windows) {
+        const saAttr = windows.SECURITY_ATTRIBUTES{
+            .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES),
+            .bInheritHandle = windows.FALSE,
+            .lpSecurityDescriptor = null,
+        };
+        try windowsMakeAsyncPipe(&pipe_new[pipe_rd], &pipe_new[pipe_wr], &saAttr);
+    } else {
+        pipe_new = try os.pipe2(@as(u32, os.O.CLOEXEC));
+    }
+    return pipe_new;
+}
+
 /// Expects `app_buf` to contain exactly the app name, and `dir_buf` to contain exactly the dir path.
 /// After return, `app_buf` will always contain exactly the app name and `dir_buf` will always contain exactly the dir path.
 /// Note: `app_buf` should not contain any leading path separators.
@@ -1333,30 +1379,25 @@ 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.
     return windows.CreateProcessW(
         app_name,
         cmd_line,
         null,
         null,
         windows.TRUE,
-        windows.CREATE_UNICODE_ENVIRONMENT,
+        @enumToInt(windows.PROCESS_CREATION_FLAGS.CREATE_UNICODE_ENVIRONMENT),
         @ptrCast(?*anyopaque, envp_ptr),
         cwd_ptr,
         lpStartupInfo,
@@ -1473,14 +1514,22 @@ fn windowsMakePipeIn(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *const w
     var wr_h: windows.HANDLE = undefined;
     try windows.CreatePipe(&rd_h, &wr_h, sattr);
     errdefer windowsDestroyPipe(rd_h, wr_h);
-    try windows.SetHandleInformation(wr_h, windows.HANDLE_FLAG_INHERIT, 0);
+    try windows.SetHandleInformation(rd_h, windows.HANDLE_FLAG_INHERIT, 1);
     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 {
+/// To enable/disable inheritance parent and child process, use
+/// os.enableInheritance() and os.disableInheritance() on the handle.
+/// convention: sattr uses bInheritHandle = windows.FALSE and only used pipe end
+/// is enabled.
+pub fn windowsMakeAsyncPipe(
+    rd: *windows.HANDLE,
+    wr: *windows.HANDLE,
+    sattr: *const windows.SECURITY_ATTRIBUTES,
+) !void {
     var tmp_bufw: [128]u16 = undefined;
 
     // Anonymous pipes are built upon Named pipes.
@@ -1533,9 +1582,6 @@ fn windowsMakeAsyncPipe(rd: *?windows.HANDLE, wr: *?windows.HANDLE, sattr: *cons
             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;

lib/std/os.zig

@@ -288,6 +288,27 @@ pub fn close(fd: fd_t) void {
     }
 }
 
+pub const windowsPtrDigits = 19; // log10(max(usize))
+pub const unixoidPtrDigits = 10; // log10(max(u32)) + 1 for sign
+pub const handleCharSize = if (builtin.target.os.tag == .windows) windowsPtrDigits else unixoidPtrDigits;
+
+pub fn handleToString(handle: fd_t, buf: []u8) std.fmt.BufPrintError![]u8 {
+    var s_handle: []u8 = undefined;
+    const handle_int =
+        // handle is *anyopaque or an integer on unix-likes Kernels.
+        if (builtin.target.os.tag == .windows) @ptrToInt(handle) else handle;
+    s_handle = try std.fmt.bufPrint(buf[0..], "{d}", .{handle_int});
+    return s_handle;
+}
+
+pub fn stringToHandle(s_handle: []const u8) std.fmt.ParseIntError!std.os.fd_t {
+    var handle: std.os.fd_t = if (builtin.target.os.tag == .windows)
+        @intToPtr(windows.HANDLE, try std.fmt.parseInt(usize, s_handle, 10))
+    else
+        try std.fmt.parseInt(std.os.fd_t, s_handle, 10);
+    return handle;
+}
+
 pub const FChmodError = error{
     AccessDenied,
     InputOutput,
@@ -4866,6 +4887,44 @@ pub fn lseek_CUR_get(fd: fd_t) SeekError!u64 {
     }
 }
 
+const IsInheritableError = FcntlError || windows.GetHandleInformationError;
+
+/// Whether inheritence is enabled or CLOEXEC is not set.
+pub inline fn isInheritable(handle: fd_t) IsInheritableError!bool {
+    if (builtin.os.tag == .windows) {
+        var handle_flags: windows.DWORD = undefined;
+        try windows.GetHandleInformation(handle, &handle_flags);
+        return handle_flags & windows.HANDLE_FLAG_INHERIT != 0;
+    } else {
+        const fcntl_flags = try fcntl(handle, F.GETFD, 0);
+        return fcntl_flags & FD_CLOEXEC == 0;
+    }
+}
+
+const EnableInheritanceError = FcntlError || windows.SetHandleInformationError;
+
+/// Enables inheritence or sets CLOEXEC.
+pub inline fn enableInheritance(handle: fd_t) EnableInheritanceError!void {
+    if (builtin.os.tag == .windows) {
+        try windows.SetHandleInformation(handle, windows.HANDLE_FLAG_INHERIT, 1);
+    } else {
+        var flags = try fcntl(handle, F.GETFD, 0);
+        flags &= ~@as(u32, FD_CLOEXEC);
+        _ = try fcntl(handle, F.SETFD, flags);
+    }
+}
+
+const DisableInheritanceError = FcntlError || windows.SetHandleInformationError;
+
+/// Disables inheritence or unsets CLOEXEC.
+pub inline fn disableInheritance(handle: fd_t) DisableInheritanceError!void {
+    if (builtin.os.tag == .windows) {
+        try windows.SetHandleInformation(handle, windows.HANDLE_FLAG_INHERIT, 0);
+    } else {
+        _ = try fcntl(handle, F.SETFD, FD_CLOEXEC);
+    }
+}
+
 pub const FcntlError = error{
     PermissionDenied,
     FileBusy,

lib/std/os/windows.zig

@@ -228,6 +228,16 @@ pub fn DeviceIoControl(
     }
 }
 
+pub const GetHandleInformationError = error{Unexpected};
+
+pub fn GetHandleInformation(h: HANDLE, flags: *DWORD) GetHandleInformationError!void {
+    if (kernel32.GetHandleInformation(h, flags) == 0) {
+        switch (kernel32.GetLastError()) {
+            else => |err| return unexpectedError(err),
+        }
+    }
+}
+
 pub fn GetOverlappedResult(h: HANDLE, overlapped: *OVERLAPPED, wait: bool) !DWORD {
     var bytes: DWORD = undefined;
     if (kernel32.GetOverlappedResult(h, overlapped, &bytes, @boolToInt(wait)) == 0) {
@@ -1590,6 +1600,45 @@ pub fn GetEnvironmentVariableW(lpName: LPWSTR, lpBuffer: [*]u16, nSize: DWORD) G
     return rc;
 }
 
+// zig fmt: off
+pub const PROCESS_CREATION_FLAGS = enum(u32) {
+    // <- gap here ->
+    DEBUG_PROCESS                       = 0x0000_0001,
+    DEBUG_ONLY_THIS_PROCESS             = 0x0000_0002,
+    CREATE_SUSPENDED                    = 0x0000_0004,
+    DETACHED_PROCESS                    = 0x0000_0008,
+    CREATE_NEW_CONSOLE                  = 0x0000_0010,
+    NORMAL_PRIORITY_CLASS               = 0x0000_0020,
+    IDLE_PRIORITY_CLASS                 = 0x0000_0040,
+    HIGH_PRIORITY_CLASS                 = 0x0000_0080,
+    REALTIME_PRIORITY_CLASS             = 0x0000_0100,
+    CREATE_NEW_PROCESS_GROUP            = 0x0000_0200,
+    CREATE_UNICODE_ENVIRONMENT          = 0x0000_0400,
+    CREATE_SEPARATE_WOW_VDM             = 0x0000_0800,
+    CREATE_SHARED_WOW_VDM               = 0x0000_1000,
+    CREATE_FORCEDOS                     = 0x0000_2000,
+    BELOW_NORMAL_PRIORITY_CLASS         = 0x0000_4000,
+    ABOVE_NORMAL_PRIORITY_CLASS         = 0x0000_8000,
+    INHERIT_PARENT_AFFINITY             = 0x0001_0000,
+    INHERIT_CALLER_PRIORITY             = 0x0002_0000,
+    CREATE_PROTECTED_PROCESS            = 0x0004_0000,
+    EXTENDED_STARTUPINFO_PRESENT        = 0x0008_0000,
+    PROCESS_MODE_BACKGROUND_BEGIN       = 0x0010_0000,
+    PROCESS_MODE_BACKGROUND_END         = 0x0020_0000,
+    CREATE_SECURE_PROCESS               = 0x0040_0000,
+    // <- gap here ->
+    CREATE_BREAKAWAY_FROM_JOB           = 0x0100_0000,
+    CREATE_PRESERVE_CODE_AUTHZ_LEVEL    = 0x0200_0000,
+    CREATE_DEFAULT_ERROR_MODE           = 0x0400_0000,
+    CREATE_NO_WINDOW                    = 0x0800_0000,
+    PROFILE_USER                        = 0x1000_0000,
+    PROFILE_KERNEL                      = 0x2000_0000,
+    PROFILE_SERVER                      = 0x4000_0000,
+    CREATE_IGNORE_SYSTEM_DEFAULT        = 0x8000_0000,
+    _,
+};
+// zig fmt: on
+
 pub const CreateProcessError = error{
     FileNotFound,
     AccessDenied,
@@ -2940,8 +2989,6 @@ pub const COORD = extern struct {
     Y: SHORT,
 };
 
-pub const CREATE_UNICODE_ENVIRONMENT = 1024;
-
 pub const TLS_OUT_OF_INDEXES = 4294967295;
 pub const IMAGE_TLS_DIRECTORY = extern struct {
     StartAddressOfRawData: usize,

lib/std/os/windows/kernel32.zig

@@ -227,6 +227,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;

lib/std/std.zig

@@ -53,6 +53,7 @@ pub const base64 = @import("base64.zig");
 pub const bit_set = @import("bit_set.zig");
 pub const builtin = @import("builtin.zig");
 pub const c = @import("c.zig");
+pub const child_process = @import("child_process.zig");
 pub const coff = @import("coff.zig");
 pub const compress = @import("compress.zig");
 pub const crypto = @import("crypto.zig");

test/standalone.zig

@@ -65,6 +65,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) {

test/standalone/childprocess_extrapipe/build.zig

@@ -0,0 +1,25 @@
+const Builder = @import("std").build.Builder;
+
+pub fn build(b: *Builder) void {
+    const target = b.standardTargetOptions(.{});
+    const optimize = b.standardOptimizeOption(.{});
+
+    const child = b.addExecutable(.{
+        .name = "child",
+        .root_source_file = .{ .path = "child.zig" },
+        .target = target,
+        .optimize = optimize,
+    });
+
+    const parent = b.addExecutable(.{
+        .name = "parent",
+        .root_source_file = .{ .path = "parent.zig" },
+        .target = target,
+        .optimize = optimize,
+    });
+    const run_cmd = parent.run();
+    run_cmd.addArtifactArg(child);
+
+    const test_step = b.step("test", "Test it");
+    test_step.dependOn(&run_cmd.step);
+}

test/standalone/childprocess_extrapipe/child.zig

@@ -0,0 +1,26 @@
+const std = @import("std");
+const builtin = @import("builtin");
+const windows = std.os.windows;
+pub fn main() !void {
+    var general_purpose_allocator = std.heap.GeneralPurposeAllocator(.{}){};
+    defer if (general_purpose_allocator.deinit()) @panic("found memory leaks");
+    const gpa = general_purpose_allocator.allocator();
+
+    var it = try std.process.argsWithAllocator(gpa);
+    defer it.deinit();
+    _ = it.next() orelse unreachable; // skip binary name
+    const s_handle = it.next() orelse unreachable;
+    var file_handle = try std.os.stringToHandle(s_handle);
+    defer std.os.close(file_handle);
+
+    // child inherited the handle, so inheritance must be enabled
+    const is_inheritable = try std.os.isInheritable(file_handle);
+    std.debug.assert(is_inheritable);
+
+    try std.os.disableInheritance(file_handle);
+    var file_in = std.fs.File{ .handle = file_handle }; // read side of pipe
+    const file_in_reader = file_in.reader();
+    const message = try file_in_reader.readUntilDelimiterAlloc(gpa, '\x17', 20_000);
+    defer gpa.free(message);
+    try std.testing.expectEqualSlices(u8, message, "test123");
+}