Disable inheritance of file descriptors created by Swift Testing by default. (swiftlang#1145)

This PR makes file descriptors created by Swift Testing `FD_CLOEXEC` by default (on Windows, `~HANDLE_FLAG_INHERIT`.) We then clear `FD_CLOEXEC` for specific file descriptors that should be inherited. On Darwin, this is effectively ignored because we use `POSIX_SPAWN_CLOEXEC_DEFAULT`, and on Windows we use `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` which has [much the same effect](https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873). (On non-Darwin POSIX platforms, there's no reliable way to ensure only one child process inherits a particular file descriptor.) This change is speculative. No additional unit tests are added because existing test coverage _should be_ sufficient; the reported issue is on a platform (Ubuntu 20.04) where we don't have any CI jobs. Resolves swiftlang#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/Attachments/Attachment.swift

@@ -462,7 +462,7 @@ extension Attachment where AttachableValue: ~Copyable {
  // file exists at this path (note "x" in the mode string), an error will
  // be thrown and we'll try again by adding a suffix.
  let preferredPath = appendPathComponent(preferredName, to: directoryPath)
- file = try FileHandle(atPath: preferredPath, mode: "wxb")
+ file = try FileHandle(atPath: preferredPath, mode: "wxeb")
  result = preferredPath
  } catch {
  // Split the extension(s) off the preferred name. The first component in
@@ -478,7 +478,7 @@ extension Attachment where AttachableValue: ~Copyable {
  // Propagate any error *except* EEXIST, which would indicate that the
  // name was already in use (so we should try again with a new suffix.)
  do {
- file = try FileHandle(atPath: preferredPath, mode: "wxb")
+ file = try FileHandle(atPath: preferredPath, mode: "wxeb")
  result = preferredPath
  break
  } catch let error as CError where error.rawValue == swt_EEXIST() {

Sources/Testing/ExitTests/ExitTest.swift

@@ -801,6 +801,15 @@ 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/Support/FileHandle.swift

@@ -73,6 +73,13 @@ struct FileHandle: ~Copyable, Sendable {
  return
  }
 
+ // On Windows, "N" is used rather than "e" to signify that a file handle is
+ // not inherited.
+ var mode = mode
+ if let eIndex = mode.firstIndex(of: "e") {
+ mode.replaceSubrange(eIndex ... eIndex, with: "N")
+ }
+
  // Windows deprecates fopen() as insecure, so call _wfopen_s() instead.
  let fileHandle = try path.withCString(encodedAs: UTF16.self) { path in
  try mode.withCString(encodedAs: UTF16.self) { mode in
@@ -98,8 +105,13 @@ struct FileHandle: ~Copyable, Sendable {
  /// - path: The path to read from.
  ///
  /// - Throws: Any error preventing the stream from being opened.
+ ///
+ /// 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()``.
  init(forReadingAtPath path: String) throws {
- try self.init(atPath: path, mode: "rb")
+ try self.init(atPath: path, mode: "reb")
  }
 
  /// Initialize an instance of this type to write to the given path.
@@ -108,8 +120,13 @@ struct FileHandle: ~Copyable, Sendable {
  /// - path: The path to write to.
  ///
  /// - Throws: Any error preventing the stream from being opened.
+ ///
+ /// 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()``.
  init(forWritingAtPath path: String) throws {
- try self.init(atPath: path, mode: "wb")
+ try self.init(atPath: path, mode: "web")
  }
 
  /// Initialize an instance of this type with an existing C file handle.
@@ -445,6 +462,17 @@ extension FileHandle {
 #if !SWT_NO_PIPES
 // MARK: - Pipes
 
+#if !SWT_TARGET_OS_APPLE && !os(Windows) && !SWT_NO_DYNAMIC_LINKING
+/// Create a pipe with flags.
+///
+/// This function declaration is provided because `pipe2()` is only declared if
+/// `_GNU_SOURCE` is set, but setting it causes build errors due to conflicts
+/// with Swift's Glibc module.
+private let _pipe2 = symbol(named: "pipe2").map {
+ castCFunction(at: $0, to: (@convention(c) (UnsafeMutablePointer<CInt>, CInt) -> CInt).self)
+}
+#endif
+
 extension FileHandle {
  /// Make a pipe connecting two new file handles.
  ///
@@ -461,15 +489,37 @@ extension FileHandle {
  /// - Bug: This function should return a tuple containing the file handles
  /// instead of returning them via `inout` arguments. Swift does not support
  /// tuples with move-only elements. ([104669935](rdar://104669935))
+ ///
+ /// 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()``.
  static func makePipe(readEnd: inout FileHandle?, writeEnd: inout FileHandle?) throws {
+#if !os(Windows)
+ var pipe2Called = false
+#endif
+
  var (fdReadEnd, fdWriteEnd) = try withUnsafeTemporaryAllocation(of: CInt.self, capacity: 2) { fds in
 #if os(Windows)
- guard 0 == _pipe(fds.baseAddress, 0, _O_BINARY) else {
+ guard 0 == _pipe(fds.baseAddress, 0, _O_BINARY | _O_NOINHERIT) else {
  throw CError(rawValue: swt_errno())
  }
 #else
- guard 0 == pipe(fds.baseAddress!) else {
- throw CError(rawValue: swt_errno())
+#if !SWT_TARGET_OS_APPLE && !os(Windows) && !SWT_NO_DYNAMIC_LINKING
+ if let _pipe2 {
+ guard 0 == _pipe2(fds.baseAddress!, O_CLOEXEC) else {
+ throw CError(rawValue: swt_errno())
+ }
+ pipe2Called = true
+ }
+#endif
+
+ if !pipe2Called {
+ // pipe2() is not available. Use pipe() instead and simulate O_CLOEXEC
+ // to the best of our ability.
+ guard 0 == pipe(fds.baseAddress!) else {
+ throw CError(rawValue: swt_errno())
+ }
  }
 #endif
  return (fds[0], fds[1])
@@ -479,6 +529,15 @@ extension FileHandle {
  Self._close(fdWriteEnd)
  }
 
+#if !os(Windows)
+ 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)
+ }
+#endif
+
  do {
  defer {
  fdReadEnd = -1
@@ -553,6 +612,72 @@ 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

Sources/_TestingInternals/include/Stubs.h

@@ -151,6 +151,26 @@ static int swt_EEXIST(void) {
  return EEXIST;
 }
 
+#if defined(F_GETFD)
+/// Call `fcntl(F_GETFD)`.
+///
+/// This function is provided because `fcntl()` is a variadic function and
+/// cannot be imported directly into Swift.
+static int swt_getfdflags(int fd) {
+ return fcntl(fd, F_GETFD);
+}
+#endif
+
+#if defined(F_SETFD)
+/// Call `fcntl(F_SETFD)`.
+///
+/// This function is provided because `fcntl()` is a variadic function and
+/// cannot be imported directly into Swift.
+static int swt_setfdflags(int fd, int flags) {
+ return fcntl(fd, F_SETFD, flags);
+}
+#endif
+
 SWT_ASSUME_NONNULL_END
 
 #endif