more cleanups

Author: matu3ba

lib/std/os.zig

@@ -4871,21 +4871,21 @@ pub fn lseek_CUR_get(fd: fd_t) SeekError!u64 {
     }
 }
 
-const EnableFileInheritanceError = FcntlError || windows.SetHandleInformationError;
+const EnableInheritanceError = FcntlError || windows.SetHandleInformationError;
 
-pub inline fn enableFileInheritance(file_handle: fd_t) EnableFileInheritanceError!void {
+pub inline fn enableInheritance(file_handle: fd_t) EnableInheritanceError!void {
     if (builtin.os.tag == .windows) {
         try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 1);
     } else {
-        var flags = fcntl(file_handle, F.GETFD, 0);
+        var flags = try fcntl(file_handle, F.GETFD, 0);
         flags &= ~@as(u32, FD_CLOEXEC);
         _ = try fcntl(file_handle, F.SETFD, flags);
     }
 }
 
-const DisableFileInheritanceError = FcntlError || windows.SetHandleInformationError;
+const DisableInheritanceError = FcntlError || windows.SetHandleInformationError;
 
-pub inline fn disableFileInheritance(file_handle: fd_t) DisableFileInheritanceError!void {
+pub inline fn disableInheritance(file_handle: fd_t) DisableInheritanceError!void {
     if (builtin.os.tag == .windows) {
         try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0);
     } else {

test/standalone/childprocess_extrapipe/child.zig

@@ -23,7 +23,7 @@ pub fn main() !void {
         var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0);
         try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0);
     }
-    try std.os.disableFileInheritance(file_handle);
+    try std.os.disableInheritance(file_handle);
     var file_in = std.fs.File{ .handle = file_handle }; // read side of pipe
     defer file_in.close();
     const file_in_reader = file_in.reader();

test/standalone/childprocess_extrapipe/parent.zig

@@ -27,10 +27,13 @@ pub fn main() !void {
             .bInheritHandle = windows.TRUE,
             .lpSecurityDescriptor = null,
         };
-        // create pipe and enable inheritance for the read end, which will be given to the child
-        try child_process.windowsMakeAsyncPipe(&pipe[pipe_rd], &pipe[pipe_wr], &saAttr, .parent_to_child);
+        // create pipe without inheritance
+        try child_process.windowsMakeAsyncPipe(&pipe[pipe_rd], &pipe[pipe_wr], &saAttr);
     } else {
-        pipe = try os.pipe(); // leaks on default, but more portable, TODO: use pip2 and close earlier?
+        // we could save setting and and unsetting 1 pipe end, but this would
+        // 1. allow more leak time and 2. makes things less consistent with windows
+        // TODO: benchmarks
+        pipe = try os.pipe2(@as(u32, os.O.CLOEXEC));
     }
 
     // write read side of pipe to string + add to spawn command
@@ -40,21 +43,20 @@ pub fn main() !void {
         &.{ child_path, s_handle },
         gpa,
     );
+
+    // enabling of file inheritance directly before and directly after spawn
+    // less time to leak => better
     {
-        // close read side of pipe, less time for leaking, if closed immediately with posix_spawn
         if (os.hasPosixSpawn) child_proc.posix_actions = try os.posix_spawn.Actions.init();
         defer if (os.hasPosixSpawn) child_proc.posix_actions.?.deinit();
         if (os.hasPosixSpawn) try child_proc.posix_actions.?.close(pipe[pipe_wr]);
+
+        try os.enableInheritance(pipe[pipe_rd]);
         defer os.close(pipe[pipe_rd]);
 
         try child_proc.spawn();
     }
 
-    // call fcntl on Unixes to disable handle inheritance (windows one is per default not enabled)
-    if (builtin.os.tag != .windows) {
-        try std.os.disableFileInheritance(pipe[pipe_wr]);
-    }
-
     // windows does have inheritance disabled on default, but we check to be sure
     if (builtin.target.os.tag == .windows) {
         var handle_flags: windows.DWORD = undefined;