Use posix_spawn_file_actions_adddup2() to clear FD_CLOEXEC. (#1147)

On
[FreeBSD](https://man.freebsd.org/cgi/man.cgi?query=posix_spawn_file_actions_adddup2),
[OpenBSD](https://github.com/openbsd/src/blob/master/lib/libc/gen/posix_spawn.c#L155),
[Android](https://android.googlesource.com/platform/bionic/+/master/libc/bionic/spawn.cpp#103),
and [Glibc ≥
2.29](https://sourceware.org/bugzilla/show_bug.cgi?id=23640),
`posix_spawn_file_actions_adddup2()` automatically clears `FD_CLOEXEC`
if the file descriptors passed to it are equal. Relying on this
behaviour eliminates a race condition when spawning child processes.
This functionality is standardized in
[POSIX.1-2024](https://pubs.opengroup.org/onlinepubs/9799919799/) thanks
to [Austin Group Defect
#411](https://www.austingroupbugs.net/view.php?id=411).

Some older Linuxes (Amazon Linux 2 in particular) don't have this
functionality, so we do a runtime check of the Glibc version.

This PR is a follow-up to #1145.
Resolves #1140.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Author: grynspan

Sources/Testing/ExitTests/ExitTest.swift

@@ -803,15 +803,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 is a no-op because we use POSIX_SPAWN_CLOEXEC_DEFAULT.
-        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

@@ -123,11 +123,27 @@ func spawnExecutable(
           guard let fd else {
             throw SystemError(description: "A child process cannot inherit a file handle without an associated file descriptor. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
           }
-          if let standardFD {
+          if let standardFD, standardFD != fd {
             _ = posix_spawn_file_actions_adddup2(fileActions, fd, standardFD)
           } else {
 #if SWT_TARGET_OS_APPLE
             _ = posix_spawn_file_actions_addinherit_np(fileActions, fd)
+#else
+            // posix_spawn_file_actions_adddup2() will automatically clear
+            // FD_CLOEXEC after forking but before execing even if the old and
+            // new file descriptors are equal. This behavior is supported by
+            // Glibc ≥ 2.29, FreeBSD, OpenBSD, and Android (Bionic) and is
+            // standardized in POSIX.1-2024 (see https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_adddup2.html
+            // and https://www.austingroupbugs.net/view.php?id=411).
+            _ = posix_spawn_file_actions_adddup2(fileActions, fd, fd)
+#if canImport(Glibc)
+            if _slowPath(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(),
+              // so we must clear it here in the parent process.
+              try setFD_CLOEXEC(false, onFileDescriptor: fd)
+            }
+#endif
 #endif
             highestFD = max(highestFD, fd)
           }
@@ -156,8 +172,6 @@ func spawnExecutable(
 #if !SWT_NO_DYNAMIC_LINKING
       // This platform doesn't have POSIX_SPAWN_CLOEXEC_DEFAULT, but we can at
       // least close all file descriptors higher than the highest inherited one.
-      // We are assuming here that the caller didn't set FD_CLOEXEC on any of
-      // these file descriptors.
       _ = _posix_spawn_file_actions_addclosefrom_np?(fileActions, highestFD + 1)
 #endif
 #elseif os(FreeBSD)
@@ -216,36 +230,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)
+/// The (runtime, not compile-time) version of glibc in use on this system.
+///
+/// This value is not part of the public interface of the testing library.
+let 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/Includes.h

@@ -53,6 +53,10 @@
 #include <sys/fcntl.h>
 #endif
 
+#if __has_include(<gnu/libc-version.h>)
+#include <gnu/libc-version.h>
+#endif
+
 #if __has_include(<sys/resource.h>) && !defined(__wasi__)
 #include <sys/resource.h>
 #endif