Simplify logic, check glibc runtime version

Author: grynspan

Sources/Testing/ExitTests/ExitTest.swift

@@ -801,30 +801,6 @@ extension ExitTest {
           childEnvironment["SWT_EXPERIMENTAL_CAPTURED_VALUES"] = capturedValuesEnvironmentVariable
         }
 
-#if !SWT_TARGET_OS_APPLE
-        // Set inherited those file handles that the child process needs. On
-        // Darwin, this isn't needed because we use POSIX_SPAWN_CLOEXEC_DEFAULT.
-        var setFileHandlesInherited = false
-#if os(Windows)
-        // Always set the file handles to be inherited on Windows. Our calls to
-        // CreateProcessW() specify which file handles are actually inherited on
-        // a per-child basis.
-        setFileHandlesInherited = true
-#elseif canImport(Glibc)
-        // On Glibc ≥ 2.29, posix_spawn_file_actions_adddup2() automatically
-        // clears FD_CLOEXEC for us in the child process.
-        var glibcVersion: (major: CInt, minor: CInt) = (0, 0)
-        swt_getGlibcVersion(&glibcVersion.major, &glibcVersion.minor)
-        setFileHandlesInherited = glibcVersion.major < 2 || (glibcVersion.major == 2 && glibcVersion.minor < 29)
-#endif
-        if setFileHandlesInherited {
-          try stdoutWriteEnd?.setInherited(true)
-          try stderrWriteEnd?.setInherited(true)
-          try backChannelWriteEnd.setInherited(true)
-          try capturedValuesReadEnd.setInherited(true)
-        }
-#endif
-
         // Spawn the child process.
         let processID = try withUnsafePointer(to: backChannelWriteEnd) { backChannelWriteEnd in
           try withUnsafePointer(to: capturedValuesReadEnd) { capturedValuesReadEnd in

Sources/Testing/ExitTests/SpawnProcess.swift

@@ -118,6 +118,9 @@ func spawnExecutable(
 
       // Forward standard I/O streams and any explicitly added file handles.
       var highestFD = max(STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO)
+#if canImport(Glibc)
+      lazy var glibcVersion = glibcVersion
+#endif
       func inherit(_ fileHandle: borrowing FileHandle, as standardFD: CInt? = nil) throws {
         try fileHandle.withUnsafePOSIXFileDescriptor { fd in
           guard let fd else {
@@ -130,6 +133,13 @@ func spawnExecutable(
             _ = posix_spawn_file_actions_addinherit_np(fileActions, fd)
 #else
             _ = posix_spawn_file_actions_adddup2(fileActions, fd, fd)
+#if canImport(Glibc)
+            if glibcVersion.major < 2 || (glibcVersion.major == 2 && glibcVersion.minor < 29) {
+              // This system is using an older version of glibc that does not
+              // implement FD_CLOEXEC clearing in posix_spawn_file_actions_adddup2().
+              try setFD_CLOEXEC(false, onFileDescriptor: fd)
+            }
+#endif
 #endif
             highestFD = max(highestFD, fd)
           }
@@ -218,36 +228,42 @@ func spawnExecutable(
   }
 #elseif os(Windows)
   return try _withStartupInfoEx(attributeCount: 1) { startupInfo in
-    func inherit(_ fileHandle: borrowing FileHandle, as outWindowsHANDLE: inout HANDLE?) throws {
+    func inherit(_ fileHandle: borrowing FileHandle) throws -> HANDLE {
       try fileHandle.withUnsafeWindowsHANDLE { windowsHANDLE in
         guard let windowsHANDLE else {
           throw SystemError(description: "A child process cannot inherit a file handle without an associated Windows handle. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
         }
-        outWindowsHANDLE = windowsHANDLE
+
+        // Ensure the file handle can be inherited by the child process.
+        guard SetHandleInformation(windowsHANDLE, DWORD(HANDLE_FLAG_INHERIT), DWORD(HANDLE_FLAG_INHERIT)) else {
+          throw Win32Error(rawValue: GetLastError())
+        }
+
+        return windowsHANDLE
       }
     }
-    func inherit(_ fileHandle: borrowing FileHandle?, as outWindowsHANDLE: inout HANDLE?) throws {
+    func inherit(_ fileHandle: borrowing FileHandle?) throws -> HANDLE? {
       if fileHandle != nil {
-        try inherit(fileHandle!, as: &outWindowsHANDLE)
+        return try inherit(fileHandle!)
       } else {
-        outWindowsHANDLE = nil
+        return nil
       }
     }
 
     // Forward standard I/O streams.
-    try inherit(standardInput, as: &startupInfo.pointee.StartupInfo.hStdInput)
-    try inherit(standardOutput, as: &startupInfo.pointee.StartupInfo.hStdOutput)
-    try inherit(standardError, as: &startupInfo.pointee.StartupInfo.hStdError)
+    startupInfo.pointee.StartupInfo.hStdInput = try inherit(standardInput)
+    startupInfo.pointee.StartupInfo.hStdOutput = try inherit(standardOutput)
+    startupInfo.pointee.StartupInfo.hStdError = try inherit(standardError)
     startupInfo.pointee.StartupInfo.dwFlags |= STARTF_USESTDHANDLES
 
     // Ensure standard I/O streams and any explicitly added file handles are
     // inherited by the child process.
     var inheritedHandles = [HANDLE?](repeating: nil, count: additionalFileHandles.count + 3)
-    try inherit(standardInput, as: &inheritedHandles[0])
-    try inherit(standardOutput, as: &inheritedHandles[1])
-    try inherit(standardError, as: &inheritedHandles[2])
+    inheritedHandles[0] = startupInfo.pointee.StartupInfo.hStdInput
+    inheritedHandles[1] = startupInfo.pointee.StartupInfo.hStdOutput
+    inheritedHandles[2] = startupInfo.pointee.StartupInfo.hStdError
     for i in 0 ..< additionalFileHandles.count {
-      try inherit(additionalFileHandles[i].pointee, as: &inheritedHandles[i + 3])
+      inheritedHandles[i + 3] = try inherit(additionalFileHandles[i].pointee)
     }
     inheritedHandles = inheritedHandles.compactMap(\.self)
 

Sources/Testing/Support/FileHandle.swift

@@ -108,8 +108,7 @@ struct FileHandle: ~Copyable, Sendable {
   ///
   /// By default, the resulting file handle is not inherited by any child
   /// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
-  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.) To make it inheritable, call
-  /// ``setInherited()``.
+  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
   init(forReadingAtPath path: String) throws {
     try self.init(atPath: path, mode: "reb")
   }
@@ -123,8 +122,7 @@ struct FileHandle: ~Copyable, Sendable {
   ///
   /// By default, the resulting file handle is not inherited by any child
   /// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
-  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.) To make it inheritable, call
-  /// ``setInherited()``.
+  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
   init(forWritingAtPath path: String) throws {
     try self.init(atPath: path, mode: "web")
   }
@@ -492,8 +490,7 @@ extension FileHandle {
   ///
   /// By default, the resulting file handles are not inherited by any child
   /// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
-  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.) To make them inheritable,
-  /// call ``setInherited()``.
+  /// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
   static func makePipe(readEnd: inout FileHandle?, writeEnd: inout FileHandle?) throws {
 #if !os(Windows)
     var pipe2Called = false
@@ -533,8 +530,8 @@ extension FileHandle {
     if !pipe2Called {
       // pipe2() is not available. Use pipe() instead and simulate O_CLOEXEC
       // to the best of our ability.
-      try _setFileDescriptorInherited(fdReadEnd, false)
-      try _setFileDescriptorInherited(fdWriteEnd, false)
+      try setFD_CLOEXEC(true, onFileDescriptor: fdReadEnd)
+      try setFD_CLOEXEC(true, onFileDescriptor: fdWriteEnd)
     }
 #endif
 
@@ -612,72 +609,6 @@ extension FileHandle {
 #endif
   }
 #endif
-
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android)
-  /// Set whether or not the given file descriptor is inherited by child processes.
-  ///
-  /// - Parameters:
-  ///   - fd: The file descriptor.
-  ///   - inherited: Whether or not `fd` is inherited by child processes
-  ///     (ignoring overriding functionality such as Apple's
-  ///     `POSIX_SPAWN_CLOEXEC_DEFAULT` flag.)
-  ///
-  /// - Throws: Any error that occurred while setting the flag.
-  private static func _setFileDescriptorInherited(_ fd: CInt, _ inherited: Bool) throws {
-    switch swt_getfdflags(fd) {
-    case -1:
-      // An error occurred reading the flags for this file descriptor.
-      throw CError(rawValue: swt_errno())
-    case let oldValue:
-      let newValue = if inherited {
-        oldValue & ~FD_CLOEXEC
-      } else {
-        oldValue | FD_CLOEXEC
-      }
-      if oldValue == newValue {
-        // No need to make a second syscall as nothing has changed.
-        return
-      }
-      if -1 == swt_setfdflags(fd, newValue) {
-        // An error occurred setting the flags for this file descriptor.
-        throw CError(rawValue: swt_errno())
-      }
-    }
-  }
-#endif
-
-  /// Set whether or not this file handle is inherited by child processes.
-  ///
-  /// - Parameters:
-  ///   - inherited: Whether or not this file handle is inherited by child
-  ///     processes (ignoring overriding functionality such as Apple's
-  ///     `POSIX_SPAWN_CLOEXEC_DEFAULT` flag.)
-  ///
-  /// - Throws: Any error that occurred while setting the flag.
-  func setInherited(_ inherited: Bool) throws {
-#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android)
-    try withUnsafePOSIXFileDescriptor { fd in
-      guard let fd else {
-        throw SystemError(description: "Cannot set whether a file handle is inherited unless it is backed by a file descriptor. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
-      }
-      try withLock {
-        try Self._setFileDescriptorInherited(fd, inherited)
-      }
-    }
-#elseif os(Windows)
-    return try withUnsafeWindowsHANDLE { handle in
-      guard let handle else {
-        throw SystemError(description: "Cannot set whether a file handle is inherited unless it is backed by a Windows file handle. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
-      }
-      let newValue = inherited ? DWORD(HANDLE_FLAG_INHERIT) : 0
-      guard SetHandleInformation(handle, DWORD(HANDLE_FLAG_INHERIT), newValue) else {
-        throw Win32Error(rawValue: GetLastError())
-      }
-    }
-#else
-#warning("Platform-specific implementation missing: cannot set whether a file handle is inherited")
-#endif
-  }
 }
 
 // MARK: - General path utilities
@@ -757,4 +688,35 @@ func canonicalizePath(_ path: String) -> String? {
   return nil
 #endif
 }
+
+#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD) || os(Android)
+/// Set the given file descriptor's `FD_CLOEXEC` flag.
+///
+/// - Parameters:
+///   - flag: The new value of `fd`'s `FD_CLOEXEC` flag.
+///   - fd: The file descriptor.
+///
+/// - Throws: Any error that occurred while setting the flag.
+func setFD_CLOEXEC(_ flag: Bool, onFileDescriptor fd: CInt) throws {
+  switch swt_getfdflags(fd) {
+  case -1:
+    // An error occurred reading the flags for this file descriptor.
+    throw CError(rawValue: swt_errno())
+  case let oldValue:
+    let newValue = if flag {
+      oldValue & ~FD_CLOEXEC
+    } else {
+      oldValue | FD_CLOEXEC
+    }
+    if oldValue == newValue {
+      // No need to make a second syscall as nothing has changed.
+      return
+    }
+    if -1 == swt_setfdflags(fd, newValue) {
+      // An error occurred setting the flags for this file descriptor.
+      throw CError(rawValue: swt_errno())
+    }
+  }
+}
+#endif
 #endif

Sources/Testing/Support/Versions.swift

@@ -153,6 +153,30 @@ let swiftStandardLibraryVersion: String = {
   return "unknown"
 }()
 
+#if canImport(Glibc)
+/// Get the (runtime, not compile-time) version of glibc in use on this system.
+///
+/// - Returns: The version of glibc currently in use on this system.
+var glibcVersion: (major: Int, minor: Int) {
+  // Default to the statically available version number if the function call
+  // fails for some reason.
+  var major = Int(clamping: __GLIBC__)
+  var minor = Int(clamping: __GLIBC_MINOR__)
+
+  if let strVersion = gnu_get_libc_version() {
+    withUnsafeMutablePointer(to: &major) { major in
+      withUnsafeMutablePointer(to: &minor) { minor in
+        withVaList([major, minor]) { args in
+          _ = vsscanf(strVersion, "%zd.%zd", args)
+        }
+      }
+    }
+  }
+
+  return (major, minor)
+}
+#endif
+
 // MARK: - sysctlbyname() Wrapper
 
 #if !SWT_NO_SYSCTL && SWT_TARGET_OS_APPLE

Sources/_TestingInternals/include/Versions.h

@@ -48,17 +48,6 @@ static const char *_Nullable swt_getWASIVersion(void) {
 }
 #endif
 
-#if defined(__GLIBC__)
-/// Get the version of glibc used when compiling the testing library.
-///
-/// This function is provided because `__GLIBC__` and `__GLIBC_MINOR__` are not
-/// available in Swift.
-static void swt_getGlibcVersion(int *outMajor, int *outMinor) {
-  *outMajor = __GLIBC__;
-  *outMinor = __GLIBC_MINOR__;
-}
-#endif
-
 SWT_ASSUME_NONNULL_END
 
 #endif