From bc432e15dc1dfe7172adcf042f2b22400dd4ba6c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 16 Sep 2024 12:01:25 -0400 Subject: [PATCH 1/9] Record issues generated within exit tests. This PR introduces a "back channel" file handle to exit tests, allowing us to record issues that occur within exit test bodies. For example: ```swift await #expect(exitsWith: .failure) { let context = try #require(possiblyMissingContext) ... } ``` In this example, if the call to `try #require()` finds `nil`, it will record an issue, but that issue today will be lost because there's no mechanism to forward the issue back to the parent process hosting the exit test. This PR fixes that! Issues are converted to JSON using the same schema we use for event handling, then written over a pipe back to the parent process where they are decoded. This decoding is lossy, so there will be further refinement needed here to try to preserve more information about the recorded issues. That said, "it's got good bones" right? On Darwin, Linux, and FreeBSD, the pipe's write end is allowed to survive into the child process (i.e. no `FD_CLOEXEC`). On Windows, the equivalent is to tell `CreateProcessW()` to explicitly inherit a `HANDLE`. The identity of this file descriptor or handle is passed to the child process via environment variable. The child process then parses the file descriptor or handle out of the environment and converts it back to a `FileHandle` that is then connected to an instance of `Configuration` with an event handler set, and off we go. Because we can now report these issues back to the parent process, I've removed the compile-time diagnostic in the `#expect(exitsWith:)` macro implementation that we emit when we see a nested `#expect()` or `#require()` call. --- Sources/Testing/ExitTests/ExitTest.swift | 155 +++++++++++- Sources/Testing/ExitTests/SpawnProcess.swift | 237 +++++++++++++----- Sources/Testing/ExitTests/WaitFor.swift | 12 +- Sources/TestingMacros/ConditionMacro.swift | 32 --- Sources/_TestingInternals/CMakeLists.txt | 1 + Sources/_TestingInternals/Stubs.cpp | 45 ++++ Sources/_TestingInternals/include/Stubs.h | 22 +- .../ConditionMacroTests.swift | 16 -- Tests/TestingTests/ExitTestTests.swift | 21 ++ 9 files changed, 424 insertions(+), 117 deletions(-) create mode 100644 Sources/_TestingInternals/Stubs.cpp diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 727b40549..6c98f7824 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -66,6 +66,32 @@ public struct ExitTest: Sendable, ~Copyable { #endif } + /// Find a back channel file handle set up by the parent process. + /// + /// - Returns: A file handle open for writing to which events should be + /// written, or `nil` if the file handle could not be resolved. + private static func _findBackChannel() -> FileHandle? { + guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL_FD") else { + return nil + } + + var fd: CInt? +#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) + fd = CInt(backChannelEnvironmentVariable).map(dup) +#elseif os(Windows) + if let handle = UInt(backChannelEnvironmentVariable).flatMap(HANDLE.init(bitPattern:)) { + fd = _open_osfhandle(Int(bitPattern: handle), _O_WRONLY | _O_BINARY) + } +#else +#warning("Platform-specific implementation missing: back-channel pipe unavailable") +#endif + guard let fd, fd >= 0 else { + return nil + } + + return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: "wb") + } + /// Call the exit test in the current process. /// /// This function invokes the closure originally passed to @@ -76,8 +102,27 @@ public struct ExitTest: Sendable, ~Copyable { public consuming func callAsFunction() async -> Never { Self._disableCrashReporting() + // Set up the configuration for this process. + var configuration = Configuration() + if let backChannel = Self._findBackChannel() { + // Encode events as JSON and write them to the back channel file handle. + var eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in + try? backChannel.write(json) + } + + // Only forward issue-recorded events. (If we start handling other kinds + // of event in the future, we can forward them too.) + eventHandler = { [eventHandler] event, eventContext in + if case .issueRecorded = event.kind { + eventHandler(event, eventContext) + } + } + + configuration.eventHandler = eventHandler + } + do { - try await body() + try await Configuration.withCurrent(configuration, perform: body) } catch { _errorInMain(error) } @@ -343,11 +388,109 @@ extension ExitTest { childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION"] = String(decoding: json, as: UTF8.self) } - return try await spawnAndWait( - forExecutableAtPath: childProcessExecutablePath, - arguments: childArguments, - environment: childEnvironment - ) + return try await withThrowingTaskGroup(of: ExitCondition?.self) { taskGroup in + // Create a "back channel" pipe to handle events from the child process. + let backChannel = try FileHandle.Pipe() + + // Let the child process know how to find the back channel by setting a + // known environment variable to the corresponding file descriptor + // (HANDLE on Windows.) + var backChannelEnvironmentVariable: String? +#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) + backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafePOSIXFileDescriptor { fd in + fd.map(String.init(describing:)) + } +#elseif os(Windows) + backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafeWindowsHANDLE { handle in + handle.flatMap { String(describing: UInt(bitPattern: $0)) } + } +#else +#warning("Platform-specific implementation missing: back-channel pipe unavailable") +#endif + if let backChannelEnvironmentVariable { + childEnvironment["SWT_EXPERIMENTAL_BACKCHANNEL_FD"] = backChannelEnvironmentVariable + } + + // Spawn the child process. + let processID = try withUnsafePointer(to: backChannel.writeEnd) { writeEnd in + try spawnExecutable( + atPath: childProcessExecutablePath, + arguments: childArguments, + environment: childEnvironment, + additionalFileHandles: .init(start: writeEnd, count: 1) + ) + } + + // Await termination of the child process. + taskGroup.addTask { + try await wait(for: processID) + } + + // Read back all data written to the back channel by the child process + // and process it as a (minimal) event stream. + let readEnd = backChannel.closeWriteEnd() + taskGroup.addTask { + Self._processRecordsFromBackChannel(readEnd) + return nil + } + + // This is a roundabout way of saying "and return the exit condition + // yielded by wait(for:)". + return try await taskGroup.compactMap { $0 }.first { _ in true }! + } + } + } + + /// Read lines from the given back channel file handle and process them as + /// event records. + /// + /// - Parameters: + /// - backChannel: The file handle to read from. Reading continues until an + /// error is encountered or the end of the file is reached. + private static func _processRecordsFromBackChannel(_ backChannel: borrowing FileHandle) { + let bytes: [UInt8] + do { + bytes = try backChannel.readToEnd() + } catch { + // NOTE: an error caught here indicates an I/O problem. + // TODO: should we record these issues as systemic instead? + Issue.record(error) + return + } + + for recordJSON in bytes.split(whereSeparator: \.isASCIINewline) where !recordJSON.isEmpty { + do { + try recordJSON.withUnsafeBufferPointer { recordJSON in + try Self._processRecord(.init(recordJSON), fromBackChannel: backChannel) + } + } catch { + // NOTE: an error caught here indicates a decoding problem. + // TODO: should we record these issues as systemic instead? + Issue.record(error) + } + } + } + + /// Decode a line of JSON read from a back channel file handle and handle it + /// as if the corresponding event occurred locally. + /// + /// - Parameters: + /// - recordJSON: The JSON to decode and process. + /// - backChannel: The file handle that `recordJSON` was read from. + /// + /// - Throws: Any error encountered attempting to decode or process the JSON. + private static func _processRecord(_ recordJSON: UnsafeRawBufferPointer, fromBackChannel backChannel: borrowing FileHandle) throws { + let record = try JSON.decode(ABIv0.Record.self, from: recordJSON) + + if case let .event(event) = record.kind, let issue = event.issue { + // Translate the issue back into a "real" issue and record it + // in the parent process. This translation is, of course, lossy + // due to the process boundary, but we make a best effort. + let comments: [Comment] = event.messages.compactMap { message in + message.symbol == .details ? Comment(rawValue: message.text) : nil + } + let issue = Issue(kind: .unconditional, comments: comments, sourceContext: .init(backtrace: nil, sourceLocation: issue.sourceLocation)) + issue.record() } } } diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index ae7eb0bd7..368f13bf1 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -8,9 +8,20 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // -private import _TestingInternals +internal import _TestingInternals #if !SWT_NO_EXIT_TESTS +/// A platform-specific value identifying a process running on the current +/// system. +#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) +typealias ProcessID = pid_t +#elseif os(Windows) +typealias ProcessID = HANDLE +#else +#warning("Platform-specific implementation missing: process IDs unavailable") +typealias ProcessID = Never +#endif + /// Spawn a process and wait for it to terminate. /// /// - Parameters: @@ -18,16 +29,21 @@ private import _TestingInternals /// - arguments: The arguments to pass to the executable, not including the /// executable path. /// - environment: The environment block to pass to the executable. +/// - additionalFileHandles: A collection of file handles to inherit in the +/// child process. /// -/// - Returns: The exit condition of the spawned process. +/// - Returns: A value identifying the process that was spawned. The caller must +/// eventually pass this value to ``wait(for:)`` to avoid leaking system +/// resources. /// /// - Throws: Any error that prevented the process from spawning or its exit /// condition from being read. -func spawnAndWait( - forExecutableAtPath executablePath: String, +func spawnExecutable( + atPath executablePath: String, arguments: [String], - environment: [String: String] -) async throws -> ExitCondition { + environment: [String: String], + additionalFileHandles: UnsafeBufferPointer = .init(start: nil, count: 0) +) throws -> ProcessID { // Darwin and Linux differ in their optionality for the posix_spawn types we // use, so use this typealias to paper over the differences. #if SWT_TARGET_OS_APPLE @@ -37,7 +53,7 @@ func spawnAndWait( #endif #if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) - let pid = try withUnsafeTemporaryAllocation(of: P.self, capacity: 1) { fileActions in + return try withUnsafeTemporaryAllocation(of: P.self, capacity: 1) { fileActions in guard 0 == posix_spawn_file_actions_init(fileActions.baseAddress!) else { throw CError(rawValue: swt_errno()) } @@ -45,11 +61,6 @@ func spawnAndWait( _ = posix_spawn_file_actions_destroy(fileActions.baseAddress!) } - // Do not forward standard I/O. - _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDIN_FILENO, "/dev/null", O_RDONLY, 0) - _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDOUT_FILENO, "/dev/null", O_WRONLY, 0) - _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDERR_FILENO, "/dev/null", O_WRONLY, 0) - return try withUnsafeTemporaryAllocation(of: P.self, capacity: 1) { attrs in guard 0 == posix_spawnattr_init(attrs.baseAddress!) else { throw CError(rawValue: swt_errno()) @@ -57,11 +68,39 @@ func spawnAndWait( defer { _ = posix_spawnattr_destroy(attrs.baseAddress!) } + + // Do not forward standard I/O. + _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDIN_FILENO, "/dev/null", O_RDONLY, 0) + _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDOUT_FILENO, "/dev/null", O_WRONLY, 0) + _ = posix_spawn_file_actions_addopen(fileActions.baseAddress!, STDERR_FILENO, "/dev/null", O_WRONLY, 0) + +#if os(Linux) || os(FreeBSD) + var highestFD = CInt(0) +#endif + for i in 0 ..< additionalFileHandles.count { + try additionalFileHandles[i].withUnsafePOSIXFileDescriptor { fd in + guard let fd else { + throw SystemError(description: "A child process inherit a file handle without an associated file descriptor. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") + } #if SWT_TARGET_OS_APPLE - // Close all other file descriptors open in the parent. Note that Linux - // does not support this flag and, unlike Foundation.Process, we do not - // attempt to emulate it. + _ = posix_spawn_file_actions_addinherit_np(fileActions.baseAddress!, fd) +#elseif os(Linux) || os(FreeBSD) + highestFD = max(highestFD, fd) +#endif + } + } + +#if SWT_TARGET_OS_APPLE + // Close all other file descriptors open in the parent. _ = posix_spawnattr_setflags(attrs.baseAddress!, CShort(POSIX_SPAWN_CLOEXEC_DEFAULT)) +#elseif os(Linux) || os(FreeBSD) + // 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. + _ = swt_posix_spawn_file_actions_addclosefrom_np(fileActions.baseAddress!, highestFD + 1) +#else +#warning("Platform-specific implementation missing: cannot close unused file descriptors") #endif var argv: [UnsafeMutablePointer?] = [strdup(executablePath)] @@ -88,15 +127,130 @@ func spawnAndWait( return pid } } - - return try await wait(for: pid) #elseif os(Windows) - // NOTE: Windows processes are responsible for handling their own - // command-line escaping. This code is adapted from the code in - // swift-corelibs-foundation (SEE: quoteWindowsCommandLine()) which was - // itself adapted from the code published by Microsoft at - // https://learn.microsoft.com/en-gb/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way - let commandLine = (CollectionOfOne(executablePath) + arguments).lazy + return try _withStartupInfoEx(attributeCount: 1) { startupInfo in + // Forward the back channel's write end to the child process so that it can + // send information back to us. Note that we don't keep the pipe open as + // bidirectional, though we could if we find we need to in the future. + let inheritedHandlesBuffer = UnsafeMutableBufferPointer.allocate(capacity: additionalFileHandles.count) + defer { + inheritedHandlesBuffer.deallocate() + } + for i in 0 ..< additionalFileHandles.count { + try additionalFileHandles[i].withUnsafeWindowsHANDLE { handle in + guard let handle else { + throw SystemError(description: "A child process inherit a file handle without an associated Windows handle. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") + } + inheritedHandlesBuffer[i] = handle + } + } + + // Update the attribute list to hold the handle buffer. + _ = UpdateProcThreadAttribute( + startupInfo.pointee.lpAttributeList, + 0, + swt_PROC_THREAD_ATTRIBUTE_HANDLE_LIST(), + inheritedHandlesBuffer.baseAddress!, + SIZE_T(MemoryLayout.stride * inheritedHandlesBuffer.count), + nil, + nil + ) + + let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments) + let environ = environment.map { "\($0.key)=\($0.value)"}.joined(separator: "\0") + "\0\0" + + return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in + try environ.withCString(encodedAs: UTF16.self) { environ in + var processInfo = PROCESS_INFORMATION() + + guard CreateProcessW( + nil, + .init(mutating: commandLine), + nil, + nil, + true, // bInheritHandles + DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT), + .init(mutating: environ), + nil, + startupInfo.pointer(to: \.StartupInfo)!, + &processInfo + ) else { + throw Win32Error(rawValue: GetLastError()) + } + _ = CloseHandle(processInfo.hThread) + + return processInfo.hProcess! + } + } + } +#else +#warning("Platform-specific implementation missing: process spawning unavailable") + throw SystemError(description: "Exit tests are unimplemented on this platform.") +#endif +} + +// MARK: - + +#if os(Windows) +/// Create a temporary instance of `STARTUPINFOEXW` to pass to +/// `CreateProcessW()`. +/// +/// - Parameters: +/// - attributeCount: The number of attributes to make space for in the +/// resulting structure's attribute list. +/// - body: A function to invoke. A temporary, mutable pointer to an instance +/// of `STARTUPINFOEXW` is passed to this function. +/// +/// - Returns: Whatever is returned by `body`. +/// +/// - Throws: Whatever is thrown while creating the startup info structure or +/// its attribute list, or whatever is thrown by `body`. +private func _withStartupInfoEx(attributeCount: Int = 0, _ body: (UnsafeMutablePointer) throws -> R) throws -> R { + // Initialize the startup info structure. + var startupInfo = STARTUPINFOEXW() + startupInfo.StartupInfo.cb = DWORD(MemoryLayout.size(ofValue: startupInfo)) + + guard attributeCount > 0 else { + return try body(&startupInfo) + } + + // Initialize an attribute list of sufficient size for the specified number of + // attributes. Alignment is a problem because LPPROC_THREAD_ATTRIBUTE_LIST is + // an opaque pointer and we don't know the alignment of the underlying data. + // We *should* use the alignment of C's max_align_t, but it is defined using a + // C++ using statement on Windows and isn't imported into Swift. So, 16 it is. + var attributeListByteCount = SIZE_T(0) + _ = InitializeProcThreadAttributeList(nil, DWORD(attributeCount), 0, &attributeListByteCount) + return try withUnsafeTemporaryAllocation(byteCount: Int(attributeListByteCount), alignment: 16) { attributeList in + let attributeList = LPPROC_THREAD_ATTRIBUTE_LIST(attributeList.baseAddress!) + guard InitializeProcThreadAttributeList(attributeList, DWORD(attributeCount), 0, &attributeListByteCount) else { + throw Win32Error(rawValue: GetLastError()) + } + defer { + DeleteProcThreadAttributeList(attributeList) + } + startupInfo.lpAttributeList = attributeList + + return try body(&startupInfo) + } +} + +/// Construct an escaped command line string suitable for passing to +/// `CreateProcessW()`. +/// +/// - Parameters: +/// - arguments: The arguments, including the executable path, to include in +/// the command line string. +/// +/// - Returns: A command line string. This string can later be parsed with +/// `CommandLineToArgvW()`. +/// +/// Windows processes are responsible for handling their own command-line +/// escaping. This function is adapted from the code in +/// swift-corelibs-foundation (see `quoteWindowsCommandLine()`) which was +/// itself adapted from code [published by Microsoft](https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way). +private func _escapeCommandLine(_ arguments: [String]) -> String { + return arguments.lazy .map { arg in if !arg.contains(where: {" \t\n\"".contains($0)}) { return arg @@ -123,41 +277,6 @@ func spawnAndWait( quoted.append("\"") return quoted }.joined(separator: " ") - let environ = environment.map { "\($0.key)=\($0.value)"}.joined(separator: "\0") + "\0\0" - - let processHandle: HANDLE! = try commandLine.withCString(encodedAs: UTF16.self) { commandLine in - try environ.withCString(encodedAs: UTF16.self) { environ in - var processInfo = PROCESS_INFORMATION() - - var startupInfo = STARTUPINFOW() - startupInfo.cb = DWORD(MemoryLayout.size(ofValue: startupInfo)) - guard CreateProcessW( - nil, - .init(mutating: commandLine), - nil, - nil, - false, - DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT), - .init(mutating: environ), - nil, - &startupInfo, - &processInfo - ) else { - throw Win32Error(rawValue: GetLastError()) - } - _ = CloseHandle(processInfo.hThread) - - return processInfo.hProcess - } - } - defer { - CloseHandle(processHandle) - } - - return try await wait(for: processHandle) -#else -#warning("Platform-specific implementation missing: process spawning unavailable") - throw SystemError(description: "Exit tests are unimplemented on this platform.") -#endif } #endif +#endif diff --git a/Sources/Testing/ExitTests/WaitFor.swift b/Sources/Testing/ExitTests/WaitFor.swift index 15fed9918..865322345 100644 --- a/Sources/Testing/ExitTests/WaitFor.swift +++ b/Sources/Testing/ExitTests/WaitFor.swift @@ -109,7 +109,7 @@ private let _createWaitThreadImpl: Void = { #if SWT_TARGET_OS_APPLE _ = pthread_setname_np("Swift Testing exit test monitor") #elseif os(Linux) - _ = pthread_setname_np(pthread_self(), "SWT ExT monitor") + _ = swt_pthread_setname_np(pthread_self(), "SWT ExT monitor") #elseif os(FreeBSD) _ = pthread_set_name_np(pthread_self(), "SWT ex test monitor") #else @@ -181,13 +181,19 @@ func wait(for pid: pid_t) async throws -> ExitCondition { /// Wait for a given process handle to exit and report its status. /// /// - Parameters: -/// - processHandle: The handle to wait for. +/// - processHandle: The handle to wait for. This function takes ownership of +/// this handle and closes it when done. /// /// - Returns: The exit condition of `processHandle`. /// /// - Throws: Any error encountered calling `WaitForSingleObject()` or /// `GetExitCodeProcess()`. -func wait(for processHandle: HANDLE) async throws -> ExitCondition { +func wait(for processHandle: consuming HANDLE) async throws -> ExitCondition { + let processHandle = copy processHandle + defer { + _ = CloseHandle(processHandle) + } + // Once the continuation resumes, it will need to unregister the wait, so // yield the wait handle back to the calling scope. var waitHandle: HANDLE? diff --git a/Sources/TestingMacros/ConditionMacro.swift b/Sources/TestingMacros/ConditionMacro.swift index adb940950..e761dcd00 100644 --- a/Sources/TestingMacros/ConditionMacro.swift +++ b/Sources/TestingMacros/ConditionMacro.swift @@ -348,34 +348,6 @@ public struct RequireThrowsNeverMacro: RefinedConditionMacro { } } -// MARK: - - -/// A syntax visitor that looks for uses of `#expect()` and `#require()` nested -/// within another macro invocation and diagnoses them as unsupported. -private final class _NestedConditionFinder: SyntaxVisitor where M: FreestandingMacroExpansionSyntax, C: MacroExpansionContext { - /// The enclosing macro invocation. - private var _macro: M - - /// The macro context in which the expression is being parsed. - private var _context: C - - init(viewMode: SyntaxTreeViewMode, macro: M, context: C) { - _macro = macro - _context = context - super.init(viewMode: viewMode) - } - - override func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind { - switch node.macroName.tokenKind { - case .identifier("expect"), .identifier("require"): - _context.diagnose(.checkUnsupported(node, inExitTest: _macro)) - default: - break - } - return .visitChildren - } -} - // MARK: - Exit test condition macros public protocol ExitTestConditionMacro: RefinedConditionMacro {} @@ -404,10 +376,6 @@ extension ExitTestConditionMacro { let bodyArgumentExpr = arguments[trailingClosureIndex].expression - // Diagnose any nested conditions in the exit test body. - let conditionFinder = _NestedConditionFinder(viewMode: .sourceAccurate, macro: macro, context: context) - conditionFinder.walk(bodyArgumentExpr) - // Create a local type that can be discovered at runtime and which contains // the exit test body. let enumName = context.makeUniqueName("__🟠$exit_test_body__") diff --git a/Sources/_TestingInternals/CMakeLists.txt b/Sources/_TestingInternals/CMakeLists.txt index b0bde88ed..8841ba50b 100644 --- a/Sources/_TestingInternals/CMakeLists.txt +++ b/Sources/_TestingInternals/CMakeLists.txt @@ -12,6 +12,7 @@ include(LibraryVersion) include(TargetTriple) add_library(_TestingInternals STATIC Discovery.cpp + Stubs.cpp Versions.cpp WillThrow.cpp) target_include_directories(_TestingInternals PUBLIC diff --git a/Sources/_TestingInternals/Stubs.cpp b/Sources/_TestingInternals/Stubs.cpp new file mode 100644 index 000000000..5fb8b4ff4 --- /dev/null +++ b/Sources/_TestingInternals/Stubs.cpp @@ -0,0 +1,45 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for Swift project authors +// + +/// This source file includes implementations of functions that _should_ simply +/// be `static` stubs in Stubs.h, but which for technical reasons cannot be +/// imported into Swift when defined in a header. +/// +/// Do not, as a rule, add function implementations in this file. Prefer to add +/// them to Stubs.h so that they can be inlined at compile- or link-time. Only +/// include functions here if Swift cannot successfully import and call them +/// otherwise. + +#undef _DEFAULT_SOURCE +#define _DEFAULT_SOURCE 1 +#undef _GNU_SOURCE +#define _GNU_SOURCE 1 + +#include "Stubs.h" + +#if defined(__linux__) +int swt_pthread_setname_np(pthread_t thread, const char *name) { + return pthread_setname_np(thread, name); +} +#endif + +#if defined(__GLIBC__) +int swt_posix_spawn_file_actions_addclosefrom_np(posix_spawn_file_actions_t *fileActions, int from) { + int result = 0; + +#if defined(__GLIBC_PREREQ) +#if __GLIBC_PREREQ(2, 34) + result = posix_spawn_file_actions_addclosefrom_np(fileActions, from); +#endif +#endif + + return result; +} +#endif diff --git a/Sources/_TestingInternals/include/Stubs.h b/Sources/_TestingInternals/include/Stubs.h index 47d97681c..87b023393 100644 --- a/Sources/_TestingInternals/include/Stubs.h +++ b/Sources/_TestingInternals/include/Stubs.h @@ -83,6 +83,14 @@ static mach_port_t swt_mach_task_self(void) { static LANGID swt_MAKELANGID(int p, int s) { return MAKELANGID(p, s); } + +/// Get the value of `PROC_THREAD_ATTRIBUTE_HANDLE_LIST`. +/// +/// This function is provided because `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` is a +/// complex macro and cannot be imported directly into Swift. +static DWORD_PTR swt_PROC_THREAD_ATTRIBUTE_HANDLE_LIST(void) { + return PROC_THREAD_ATTRIBUTE_HANDLE_LIST; +} #endif #if defined(__linux__) || defined(__FreeBSD__) || defined(__ANDROID__) @@ -99,13 +107,25 @@ SWT_EXTERN char *_Nullable *_Null_unspecified environ; static char *_Nullable *_Null_unspecified swt_environ(void) { return environ; } +#endif +#if defined(__linux__) /// Set the name of the current thread. /// /// This function declaration is provided because `pthread_setname_np()` is /// only declared if `_GNU_SOURCE` is set, but setting it causes build errors /// due to conflicts with Swift's Glibc module. -SWT_IMPORT_FROM_STDLIB int pthread_setname_np(pthread_t, const char *); +SWT_EXTERN int swt_pthread_setname_np(pthread_t thread, const char *name); +#endif + +#if defined(__GLIBC__) +/// Close file descriptors above a given value when spawing a new process. +/// +/// This symbol is provided because the underlying function was added to glibc +/// relatively recently and may not be available on all targets. Checking +/// `__GLIBC_PREREQ()` is insufficient because `_DEFAULT_SOURCE` may not be +/// defined at the point spawn.h is first included. +SWT_EXTERN int swt_posix_spawn_file_actions_addclosefrom_np(posix_spawn_file_actions_t *fileActions, int from); #endif #if !defined(__ANDROID__) diff --git a/Tests/TestingMacrosTests/ConditionMacroTests.swift b/Tests/TestingMacrosTests/ConditionMacroTests.swift index e29a4a33d..7ede6233c 100644 --- a/Tests/TestingMacrosTests/ConditionMacroTests.swift +++ b/Tests/TestingMacrosTests/ConditionMacroTests.swift @@ -365,22 +365,6 @@ struct ConditionMacroTests { #expect(diagnostic.message.contains("is redundant")) } -#if !SWT_NO_EXIT_TESTS - @Test("Expectation inside an exit test diagnoses", - arguments: [ - "#expectExitTest(exitsWith: .failure) { #expect(1 > 2) }", - "#requireExitTest(exitsWith: .success) { #expect(1 > 2) }", - ] - ) - func expectationInExitTest(input: String) throws { - let (_, diagnostics) = try parse(input) - let diagnostic = try #require(diagnostics.first) - #expect(diagnostic.diagMessage.severity == .error) - #expect(diagnostic.message.contains("record an issue")) - #expect(diagnostic.message.contains("(exitsWith:")) - } -#endif - @Test("Macro expansion is performed within a test function") func macroExpansionInTestFunction() throws { let input = ##""" diff --git a/Tests/TestingTests/ExitTestTests.swift b/Tests/TestingTests/ExitTestTests.swift index 5c4ab7b47..3d0e37e20 100644 --- a/Tests/TestingTests/ExitTestTests.swift +++ b/Tests/TestingTests/ExitTestTests.swift @@ -201,6 +201,27 @@ private import _TestingInternals } } + @Test("Exit test forwards issues") func forwardsIssues() async { + await confirmation("Issue recorded") { issueRecorded in + var configuration = Configuration() + configuration.eventHandler = { event, _ in + if case let .issueRecorded(issue) = event.kind, + case .unconditional = issue.kind, + issue.comments.contains("Something went wrong!") { + issueRecorded() + } + } + configuration.exitTestHandler = ExitTest.handlerForEntryPoint() + + await Test { + await #expect(exitsWith: .success) { + #expect(Bool(false), "Something went wrong!") + exit(0) + } + }.run(configuration: configuration) + } + } + #if !os(Linux) @Test("Exit test reports > 8 bits of the exit code") func fullWidthExitCode() async { From ffff274537323520181fb00fa59b71b27a081282 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 17 Sep 2024 12:43:14 -0400 Subject: [PATCH 2/9] Address feedback, note ADO issue number for command line escaping --- Sources/Testing/ExitTests/ExitTest.swift | 4 ++-- Sources/Testing/ExitTests/SpawnProcess.swift | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 6c98f7824..704f6125a 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -71,7 +71,7 @@ public struct ExitTest: Sendable, ~Copyable { /// - Returns: A file handle open for writing to which events should be /// written, or `nil` if the file handle could not be resolved. private static func _findBackChannel() -> FileHandle? { - guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL_FD") else { + guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL") else { return nil } @@ -408,7 +408,7 @@ extension ExitTest { #warning("Platform-specific implementation missing: back-channel pipe unavailable") #endif if let backChannelEnvironmentVariable { - childEnvironment["SWT_EXPERIMENTAL_BACKCHANNEL_FD"] = backChannelEnvironmentVariable + childEnvironment["SWT_EXPERIMENTAL_BACKCHANNEL"] = backChannelEnvironmentVariable } // Spawn the child process. diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index 368f13bf1..50d78010a 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -157,7 +157,7 @@ func spawnExecutable( ) let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments) - let environ = environment.map { "\($0.key)=\($0.value)"}.joined(separator: "\0") + "\0\0" + let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0" return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in try environ.withCString(encodedAs: UTF16.self) { environ in @@ -248,7 +248,8 @@ private func _withStartupInfoEx(attributeCount: Int = 0, _ body: (UnsafeMutab /// Windows processes are responsible for handling their own command-line /// escaping. This function is adapted from the code in /// swift-corelibs-foundation (see `quoteWindowsCommandLine()`) which was -/// itself adapted from code [published by Microsoft](https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way). +/// itself adapted from code [published by Microsoft](https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way) +/// (ADO 8992662). private func _escapeCommandLine(_ arguments: [String]) -> String { return arguments.lazy .map { arg in From ededae5ee65f7594baa9ed34ae44b49369124ced Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 17 Sep 2024 14:38:52 -0400 Subject: [PATCH 3/9] Make the back channel property in ExitTest a onceler rather than a function so you can't break things by calling it twice in the same process --- Sources/Testing/ExitTests/ExitTest.swift | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 704f6125a..7925d0e5d 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -66,18 +66,19 @@ public struct ExitTest: Sendable, ~Copyable { #endif } - /// Find a back channel file handle set up by the parent process. + /// The back channel file handle set up by the parent process. /// - /// - Returns: A file handle open for writing to which events should be - /// written, or `nil` if the file handle could not be resolved. - private static func _findBackChannel() -> FileHandle? { + /// The value of this property is a file handle open for writing to which + /// events should be written, or `nil` if the file handle could not be + /// resolved. + private static let _backChannel: FileHandle? = { guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL") else { return nil } var fd: CInt? #if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) - fd = CInt(backChannelEnvironmentVariable).map(dup) + fd = CInt(backChannelEnvironmentVariable) #elseif os(Windows) if let handle = UInt(backChannelEnvironmentVariable).flatMap(HANDLE.init(bitPattern:)) { fd = _open_osfhandle(Int(bitPattern: handle), _O_WRONLY | _O_BINARY) @@ -90,7 +91,7 @@ public struct ExitTest: Sendable, ~Copyable { } return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: "wb") - } + }() /// Call the exit test in the current process. /// @@ -104,23 +105,22 @@ public struct ExitTest: Sendable, ~Copyable { // Set up the configuration for this process. var configuration = Configuration() - if let backChannel = Self._findBackChannel() { - // Encode events as JSON and write them to the back channel file handle. - var eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in - try? backChannel.write(json) - } - // Only forward issue-recorded events. (If we start handling other kinds - // of event in the future, we can forward them too.) - eventHandler = { [eventHandler] event, eventContext in - if case .issueRecorded = event.kind { - eventHandler(event, eventContext) - } - } + // Encode events as JSON and write them to the back channel file handle. + var eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in + try? Self._backChannel?.write(json) + } - configuration.eventHandler = eventHandler + // Only forward issue-recorded events. (If we start handling other kinds + // of event in the future, we can forward them too.) + eventHandler = { [eventHandler] event, eventContext in + if case .issueRecorded = event.kind { + eventHandler(event, eventContext) + } } + configuration.eventHandler = eventHandler + do { try await Configuration.withCurrent(configuration, perform: body) } catch { From 29a262bfa49222290f2b50a2588596b5ca608b41 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 18 Sep 2024 15:14:23 -0400 Subject: [PATCH 4/9] Move backchannel setup to the entry point side of things since it only applies to our own entry point and third parties would need to supply their own configuration/etc. anyway --- Sources/Testing/ExitTests/ExitTest.swift | 116 +++++++++++++---------- 1 file changed, 66 insertions(+), 50 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 7925d0e5d..587f4f922 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -66,33 +66,6 @@ public struct ExitTest: Sendable, ~Copyable { #endif } - /// The back channel file handle set up by the parent process. - /// - /// The value of this property is a file handle open for writing to which - /// events should be written, or `nil` if the file handle could not be - /// resolved. - private static let _backChannel: FileHandle? = { - guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL") else { - return nil - } - - var fd: CInt? -#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) - fd = CInt(backChannelEnvironmentVariable) -#elseif os(Windows) - if let handle = UInt(backChannelEnvironmentVariable).flatMap(HANDLE.init(bitPattern:)) { - fd = _open_osfhandle(Int(bitPattern: handle), _O_WRONLY | _O_BINARY) - } -#else -#warning("Platform-specific implementation missing: back-channel pipe unavailable") -#endif - guard let fd, fd >= 0 else { - return nil - } - - return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: "wb") - }() - /// Call the exit test in the current process. /// /// This function invokes the closure originally passed to @@ -103,26 +76,8 @@ public struct ExitTest: Sendable, ~Copyable { public consuming func callAsFunction() async -> Never { Self._disableCrashReporting() - // Set up the configuration for this process. - var configuration = Configuration() - - // Encode events as JSON and write them to the back channel file handle. - var eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in - try? Self._backChannel?.write(json) - } - - // Only forward issue-recorded events. (If we start handling other kinds - // of event in the future, we can forward them too.) - eventHandler = { [eventHandler] event, eventContext in - if case .issueRecorded = event.kind { - eventHandler(event, eventContext) - } - } - - configuration.eventHandler = eventHandler - do { - try await Configuration.withCurrent(configuration, perform: body) + try await body() } catch { _errorInMain(error) } @@ -275,6 +230,33 @@ extension ExitTest { /// recording any issues that occur. public typealias Handler = @Sendable (_ exitTest: borrowing ExitTest) async throws -> ExitCondition + /// The back channel file handle set up by the parent process. + /// + /// The value of this property is a file handle open for writing to which + /// events should be written, or `nil` if the file handle could not be + /// resolved. + private static let _backChannelForEntryPoint: FileHandle? = { + guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL") else { + return nil + } + + var fd: CInt? +#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) + fd = CInt(backChannelEnvironmentVariable) +#elseif os(Windows) + if let handle = UInt(backChannelEnvironmentVariable).flatMap(HANDLE.init(bitPattern:)) { + fd = _open_osfhandle(Int(bitPattern: handle), _O_WRONLY | _O_BINARY) + } +#else +#warning("Platform-specific implementation missing: back-channel pipe unavailable") +#endif + guard let fd, fd >= 0 else { + return nil + } + + return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: "wb") + }() + /// Find the exit test function specified in the environment of the current /// process, if any. /// @@ -285,16 +267,50 @@ extension ExitTest { /// `__swiftPMEntryPoint()` function. The effect of using it under other /// configurations is undefined. static func findInEnvironmentForEntryPoint() -> Self? { + // Find the source location of the exit test to run, if any, in the + // environment block. + var sourceLocation: SourceLocation? if var sourceLocationString = Environment.variable(named: "SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION") { - let sourceLocation = try? sourceLocationString.withUTF8 { sourceLocationBuffer in + sourceLocation = try? sourceLocationString.withUTF8 { sourceLocationBuffer in let sourceLocationBuffer = UnsafeRawBufferPointer(sourceLocationBuffer) return try JSON.decode(SourceLocation.self, from: sourceLocationBuffer) } - if let sourceLocation { - return find(at: sourceLocation) + } + guard let sourceLocation else { + return nil + } + + // If an exit test was found, inject back channel handling into its body. + // External tools authors should set up their own back channel mechanisms + // and ensure they're installed before calling ExitTest.callAsFunction(). + guard var result = find(at: sourceLocation) else { + return nil + } + + // We can't say guard let here because it counts as a consume. + guard _backChannelForEntryPoint != nil else { + return result + } + + // Set up the configuration for this process. + var configuration = Configuration() + + // Encode events as JSON and write them to the back channel file handle. + // Only forward issue-recorded events. (If we start handling other kinds + // of event in the future, we can forward them too.) + let eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in + try? _backChannelForEntryPoint?.write(json) + } + configuration.eventHandler = { event, eventContext in + if case .issueRecorded = event.kind { + eventHandler(event, eventContext) } } - return nil + + result.body = { [configuration, body = result.body] in + try await Configuration.withCurrent(configuration, perform: body) + } + return result } /// The exit test handler used when integrating with Swift Package Manager via From 0855757ef52b0f2e7515cdeee7f7903d20d988cd Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 18 Sep 2024 15:18:28 -0400 Subject: [PATCH 5/9] Catch known issues correctly --- Sources/Testing/ExitTests/ExitTest.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 587f4f922..22619d376 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -505,8 +505,14 @@ extension ExitTest { let comments: [Comment] = event.messages.compactMap { message in message.symbol == .details ? Comment(rawValue: message.text) : nil } - let issue = Issue(kind: .unconditional, comments: comments, sourceContext: .init(backtrace: nil, sourceLocation: issue.sourceLocation)) - issue.record() + let sourceContext = SourceContext( + backtrace: nil, // `issue._backtrace` will have the wrong address space. + sourceLocation: issue.sourceLocation + ) + // TODO: improve fidelity of issue kind reporting (especially those without associated values) + var issueCopy = Issue(kind: .unconditional, comments: comments, sourceContext: sourceContext) + issueCopy.isKnown = issue.isKnown + issueCopy.record() } } } From ec89f02938eebc122d9ed442c0019fb7d2d33eaa Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 19 Sep 2024 17:01:37 -0400 Subject: [PATCH 6/9] Fix typo --- Sources/Testing/ExitTests/ExitTest.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 22619d376..d585727b3 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -296,8 +296,8 @@ extension ExitTest { var configuration = Configuration() // Encode events as JSON and write them to the back channel file handle. - // Only forward issue-recorded events. (If we start handling other kinds - // of event in the future, we can forward them too.) + // Only forward issue-recorded events. (If we start handling other kinds of + // events in the future, we can forward them too.) let eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in try? _backChannelForEntryPoint?.write(json) } From c8595e0c0f5f9b8ed788897a18e2ea07b87171c0 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 19 Sep 2024 21:03:48 -0400 Subject: [PATCH 7/9] Make wait(for: pid_t) consume the pid to make it clear it's being reaped --- Sources/Testing/ExitTests/WaitFor.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Testing/ExitTests/WaitFor.swift b/Sources/Testing/ExitTests/WaitFor.swift index 865322345..6c5ec2a94 100644 --- a/Sources/Testing/ExitTests/WaitFor.swift +++ b/Sources/Testing/ExitTests/WaitFor.swift @@ -21,11 +21,11 @@ internal import _TestingInternals /// /// - Throws: If the exit status of the process with ID `pid` cannot be /// determined (i.e. it does not represent an exit condition.) -private func _blockAndWait(for pid: pid_t) throws -> ExitCondition { +private func _blockAndWait(for pid: consuming pid_t) throws -> ExitCondition { // Get the exit status of the process or throw an error (other than EINTR.) while true { var siginfo = siginfo_t() - if 0 == waitid(P_PID, id_t(pid), &siginfo, WEXITED) { + if 0 == waitid(P_PID, id_t(copy pid), &siginfo, WEXITED) { switch siginfo.si_code { case .init(CLD_EXITED): return .exitCode(siginfo.si_status) @@ -142,9 +142,9 @@ private func _createWaitThread() { /// /// - Throws: Any error encountered calling `waitpid()` except for `EINTR`, /// which is ignored. -func wait(for pid: pid_t) async throws -> ExitCondition { +func wait(for pid: consuming pid_t) async throws -> ExitCondition { #if SWT_TARGET_OS_APPLE && !SWT_NO_LIBDISPATCH - let source = DispatchSource.makeProcessSource(identifier: pid, eventMask: .exit) + let source = DispatchSource.makeProcessSource(identifier: copy pid, eventMask: .exit) defer { source.cancel() } From 2f9867ab53a6e0bcd0469e62de77f7355a665770 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 20 Sep 2024 11:05:14 -0400 Subject: [PATCH 8/9] We want to consume, not copy, process IDs --- Sources/Testing/ExitTests/WaitFor.swift | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Sources/Testing/ExitTests/WaitFor.swift b/Sources/Testing/ExitTests/WaitFor.swift index 6c5ec2a94..86c986ed4 100644 --- a/Sources/Testing/ExitTests/WaitFor.swift +++ b/Sources/Testing/ExitTests/WaitFor.swift @@ -22,10 +22,12 @@ internal import _TestingInternals /// - Throws: If the exit status of the process with ID `pid` cannot be /// determined (i.e. it does not represent an exit condition.) private func _blockAndWait(for pid: consuming pid_t) throws -> ExitCondition { + let pid = consume pid + // Get the exit status of the process or throw an error (other than EINTR.) while true { var siginfo = siginfo_t() - if 0 == waitid(P_PID, id_t(copy pid), &siginfo, WEXITED) { + if 0 == waitid(P_PID, id_t(pid), &siginfo, WEXITED) { switch siginfo.si_code { case .init(CLD_EXITED): return .exitCode(siginfo.si_status) @@ -143,8 +145,10 @@ private func _createWaitThread() { /// - Throws: Any error encountered calling `waitpid()` except for `EINTR`, /// which is ignored. func wait(for pid: consuming pid_t) async throws -> ExitCondition { + let pid = consume pid + #if SWT_TARGET_OS_APPLE && !SWT_NO_LIBDISPATCH - let source = DispatchSource.makeProcessSource(identifier: copy pid, eventMask: .exit) + let source = DispatchSource.makeProcessSource(identifier: pid, eventMask: .exit) defer { source.cancel() } @@ -189,7 +193,7 @@ func wait(for pid: consuming pid_t) async throws -> ExitCondition { /// - Throws: Any error encountered calling `WaitForSingleObject()` or /// `GetExitCodeProcess()`. func wait(for processHandle: consuming HANDLE) async throws -> ExitCondition { - let processHandle = copy processHandle + let processHandle = consume processHandle defer { _ = CloseHandle(processHandle) } From 4450a2aaf3ca70d45c373b11cf793696765d35e7 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 20 Sep 2024 13:55:13 -0400 Subject: [PATCH 9/9] Incorporate feedback --- Sources/Testing/ExitTests/ExitTest.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index d585727b3..2a058a7f9 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -446,7 +446,7 @@ extension ExitTest { // and process it as a (minimal) event stream. let readEnd = backChannel.closeWriteEnd() taskGroup.addTask { - Self._processRecordsFromBackChannel(readEnd) + Self._processRecords(fromBackChannel: readEnd) return nil } @@ -463,7 +463,7 @@ extension ExitTest { /// - Parameters: /// - backChannel: The file handle to read from. Reading continues until an /// error is encountered or the end of the file is reached. - private static func _processRecordsFromBackChannel(_ backChannel: borrowing FileHandle) { + private static func _processRecords(fromBackChannel backChannel: borrowing FileHandle) { let bytes: [UInt8] do { bytes = try backChannel.readToEnd()