diff --git a/Sources/Subprocess/API.swift b/Sources/Subprocess/API.swift index 6710d0a..5fb2045 100644 --- a/Sources/Subprocess/API.swift +++ b/Sources/Subprocess/API.swift @@ -753,7 +753,7 @@ public func runDetached( errorPipe: try processError.createPipe() ).execution } - execution.release() + execution.processIdentifier.close() return execution.processIdentifier } diff --git a/Sources/Subprocess/Configuration.swift b/Sources/Subprocess/Configuration.swift index 6e6cd38..60517f5 100644 --- a/Sources/Subprocess/Configuration.swift +++ b/Sources/Subprocess/Configuration.swift @@ -103,7 +103,12 @@ public struct Configuration: Sendable { // even if `body` throws, and we are not leaving zombie processes in the // process table which will cause the process termination monitoring thread // to effectively hang due to the pid never being awaited - let terminationStatus = try await Subprocess.monitorProcessTermination(forExecution: _spawnResult.execution) + let terminationStatus = try await Subprocess.monitorProcessTermination( + for: execution.processIdentifier + ) + + // Close process file descriptor now we finished monitoring + execution.processIdentifier.close() return try ExecutionResult(terminationStatus: terminationStatus, value: result.get()) } diff --git a/Sources/Subprocess/Execution.swift b/Sources/Subprocess/Execution.swift index 66f8628..4c28037 100644 --- a/Sources/Subprocess/Execution.swift +++ b/Sources/Subprocess/Execution.swift @@ -34,37 +34,11 @@ public struct Execution: Sendable { /// The process identifier of the current execution public let processIdentifier: ProcessIdentifier - #if os(Windows) - internal nonisolated(unsafe) let processInformation: PROCESS_INFORMATION - internal let consoleBehavior: PlatformOptions.ConsoleBehavior - - init( - processIdentifier: ProcessIdentifier, - processInformation: PROCESS_INFORMATION, - consoleBehavior: PlatformOptions.ConsoleBehavior - ) { - self.processIdentifier = processIdentifier - self.processInformation = processInformation - self.consoleBehavior = consoleBehavior - } - #else init( processIdentifier: ProcessIdentifier ) { self.processIdentifier = processIdentifier } - #endif // os(Windows) - - internal func release() { - #if os(Windows) - guard CloseHandle(processInformation.hThread) else { - fatalError("Failed to close thread HANDLE: \(SubprocessError.UnderlyingError(rawValue: GetLastError()))") - } - guard CloseHandle(processInformation.hProcess) else { - fatalError("Failed to close process HANDLE: \(SubprocessError.UnderlyingError(rawValue: GetLastError()))") - } - #endif - } } // MARK: - Output Capture diff --git a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift index 5309f1e..6762578 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift @@ -477,26 +477,41 @@ extension Configuration { } } -// Special keys used in Error's user dictionary -extension String { - static let debugDescriptionErrorKey = "NSDebugDescription" +// MARK: - ProcessIdentifier + +/// A platform independent identifier for a Subprocess. +public struct ProcessIdentifier: Sendable, Hashable { + /// The platform specific process identifier value + public let value: pid_t + + public init(value: pid_t) { + self.value = value + } + + internal func close() { /* No-op on Darwin */ } +} + +extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertible { + public var description: String { "\(self.value)" } + + public var debugDescription: String { "\(self.value)" } } // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forExecution execution: Execution + for processIdentifier: ProcessIdentifier ) async throws -> TerminationStatus { return try await withCheckedThrowingContinuation { continuation in let source = DispatchSource.makeProcessSource( - identifier: execution.processIdentifier.value, + identifier: processIdentifier.value, eventMask: [.exit], queue: .global() ) source.setEventHandler { source.cancel() var siginfo = siginfo_t() - let rc = waitid(P_PID, id_t(execution.processIdentifier.value), &siginfo, WEXITED) + let rc = waitid(P_PID, id_t(processIdentifier.value), &siginfo, WEXITED) guard rc == 0 else { continuation.resume( throwing: SubprocessError( diff --git a/Sources/Subprocess/Platforms/Subprocess+Linux.swift b/Sources/Subprocess/Platforms/Subprocess+Linux.swift index ae1b5d8..6821d25 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Linux.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Linux.swift @@ -88,12 +88,14 @@ extension Configuration { // Spawn var pid: pid_t = 0 + var processFileDescriptor: PlatformFileDescriptor = -1 let spawnError: CInt = possibleExecutablePath.withCString { exePath in return (self.workingDirectory?.string).withOptionalCString { workingDir in return supplementaryGroups.withOptionalUnsafeBufferPointer { sgroups in return fileDescriptors.withUnsafeBufferPointer { fds in return _subprocess_fork_exec( &pid, + &processFileDescriptor, exePath, workingDir, fds.baseAddress!, @@ -140,9 +142,11 @@ extension Configuration { errorRead: nil, errorWrite: errorWriteFileDescriptor ) - let execution = Execution( - processIdentifier: .init(value: pid) + processIdentifier: .init( + value: pid, + processFileDescriptor: processFileDescriptor + ) ) return SpawnResult( execution: execution, @@ -181,6 +185,32 @@ extension Configuration { } } +// MARK: - ProcessIdentifier + +/// A platform independent identifier for a Subprocess. +public struct ProcessIdentifier: Sendable, Hashable { + /// The platform specific process identifier value + public let value: pid_t + internal let processFileDescriptor: PlatformFileDescriptor + + internal init(value: pid_t, processFileDescriptor: PlatformFileDescriptor) { + self.value = value + self.processFileDescriptor = processFileDescriptor + } + + internal func close() { + if self.processFileDescriptor > 0 { + _SubprocessCShims.close(self.processFileDescriptor) + } + } +} + +extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertible { + public var description: String { "\(self.value)" } + + public var debugDescription: String { "\(self.value)" } +} + // MARK: - Platform Specific Options /// The collection of platform-specific settings @@ -257,63 +287,115 @@ extension String { // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forExecution execution: Execution + for processIdentifier: ProcessIdentifier ) async throws -> TerminationStatus { return try await withCheckedThrowingContinuation { continuation in - _childProcessContinuations.withLock { continuations in - // We don't need to worry about a race condition here because waitid() - // does not clear the wait/zombie state of the child process. If it sees - // the child process has terminated and manages to acquire the lock before - // we add this continuation to the dictionary, then it will simply loop - // and report the status again. - let oldContinuation = continuations.updateValue(continuation, forKey: execution.processIdentifier.value) - precondition(oldContinuation == nil) - - // Wake up the waiter thread if it is waiting for more child processes. - _ = pthread_cond_signal(_waitThreadNoChildrenCondition) - } - } -} - -// Small helper to provide thread-safe access to the child process to continuations map as well as a condition variable to suspend the calling thread when there are no subprocesses to wait for. Note that Mutex cannot be used here because we need the semantics of pthread_cond_wait, which requires passing the pthread_mutex_t instance as a parameter, something the Mutex API does not provide access to. -private final class ChildProcessContinuations: Sendable { - typealias MutexType = pthread_mutex_t - - private nonisolated(unsafe) var continuations = [pid_t: CheckedContinuation]() - private nonisolated(unsafe) let mutex = UnsafeMutablePointer.allocate(capacity: 1) + let status = _processMonitorState.withLock { state -> Result? in + switch state { + case .notStarted: + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: nil + ) + return .failure(error) + case .failed(let error): + return .failure(error) + case .started(let storage): + // pidfd is only supported on Linux kernel 5.4 and above + // On older releases, use signalfd so we do not need + // to register anything with epoll + if processIdentifier.processFileDescriptor > 0 { + // Register processFileDescriptor with epoll + var event = epoll_event( + events: EPOLLIN.rawValue, + data: epoll_data(fd: processIdentifier.processFileDescriptor) + ) + let rc = epoll_ctl( + storage.epollFileDescriptor, + EPOLL_CTL_ADD, + processIdentifier.processFileDescriptor, + &event + ) + if rc != 0 { + let epollErrno = errno + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: epollErrno) + ) + return .failure(error) + } + // Now save the registration + var newState = storage + newState.continuations[processIdentifier.processFileDescriptor] = continuation + state = .started(newState) + // No state to resume + return nil + } else { + // Fallback to using signal handler directly on older Linux kernels + // Since Linux coalesce signals, it's possible by the time we request + // monitoring the process has already exited. Check to make sure that + // is not the case and only save continuation then. + var siginfo = siginfo_t() + // Use NOHANG here because the child process might still be running + if 0 == waitid(P_PID, id_t(processIdentifier.value), &siginfo, WEXITED | WNOHANG) { + // If si_pid and si_signo are both 0, the child is still running since we used WNOHANG + if siginfo.si_pid == 0 && siginfo.si_signo == 0 { + // Save this continuation to be called by signal hander + var newState = storage + newState.continuations[processIdentifier.processFileDescriptor] = continuation + state = .started(newState) + return nil + } - init() { - pthread_mutex_init(mutex, nil) - } + switch siginfo.si_code { + case .init(CLD_EXITED): + return .success(.exited(siginfo.si_status)) + case .init(CLD_KILLED), .init(CLD_DUMPED): + return .success(.unhandledException(siginfo.si_status)) + default: + fatalError("Unexpected exit status: \(siginfo.si_code)") + } + } else { + let waitidError = errno + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: waitidError) + ) + return .failure(error) + } + } + } + } - func withLock(_ body: (inout [pid_t: CheckedContinuation]) throws -> R) rethrows -> R { - try withUnsafeUnderlyingLock { _, continuations in - try body(&continuations) + if let status { + continuation.resume(with: status) } } +} - func withUnsafeUnderlyingLock(_ body: (UnsafeMutablePointer, inout [pid_t: CheckedContinuation]) throws -> R) rethrows -> R { - pthread_mutex_lock(mutex) - defer { - pthread_mutex_unlock(mutex) - } - return try body(mutex, &continuations) +private enum ProcessMonitorState { + struct Storage { + let epollFileDescriptor: CInt + let shutdownFileDescriptor: CInt + let monitorThread: pthread_t + var continuations: [PlatformFileDescriptor : CheckedContinuation] } + + case notStarted + case started(Storage) + case failed(SubprocessError) } -private let _childProcessContinuations = ChildProcessContinuations() +private final class MonitorThreadContext { + let epollFileDescriptor: CInt + let shutdownFileDescriptor: CInt -private nonisolated(unsafe) let _waitThreadNoChildrenCondition = { - #if os(FreeBSD) || os(OpenBSD) - let result = UnsafeMutablePointer.allocate(capacity: 1) - #else - let result = UnsafeMutablePointer.allocate(capacity: 1) - #endif - _ = pthread_cond_init(result, nil) - return result -}() + init(epollFileDescriptor: CInt, shutdownFileDescriptor: CInt) { + self.epollFileDescriptor = epollFileDescriptor + self.shutdownFileDescriptor = shutdownFileDescriptor + } +} -#if !os(FreeBSD) && !os(OpenBSD) private extension siginfo_t { var si_status: Int32 { #if canImport(Glibc) @@ -335,80 +417,368 @@ private extension siginfo_t { #endif } } -#endif -private let setup: () = { - // Create the thread. It will run immediately; because it runs in an infinite - // loop, we aren't worried about detaching or joining it. - #if os(FreeBSD) || os(OpenBSD) - var thread: pthread_t? - #else - var thread = pthread_t() - #endif - _ = pthread_create( - &thread, - nil, - { _ -> UnsafeMutableRawPointer? in - // Run an infinite loop that waits for child processes to terminate and - // captures their exit statuses. - while true { - // Listen for child process exit events. WNOWAIT means we don't perturb the - // state of a terminated (zombie) child process, allowing us to fetch the - // continuation (if available) before reaping. - var siginfo = siginfo_t() - errno = 0 - if waitid(P_ALL, id_t(0), &siginfo, WEXITED | WNOWAIT) == 0 { - let pid = siginfo.si_pid - - // If we had a continuation for this PID, allow the process to be reaped - // and pass the resulting exit condition back to the calling task. If - // there is no continuation, then either it hasn't been stored yet or - // this child process is not tracked by the waiter thread. - guard pid != 0, let c = _childProcessContinuations.withLock({ $0.removeValue(forKey: pid) }) else { - continue - } +// Okay to be unlocked global mutable because this value is only set once like dispatch_once +private nonisolated(unsafe) var _signalPipe: (readEnd: CInt, writeEnd: CInt) = (readEnd: -1, writeEnd: -1) +// Okay to be unlocked global mutable because this value is only set once like dispatch_once +private nonisolated(unsafe) var _waitProcessFileDescriptorSupported = false +private let _processMonitorState: Mutex = .init(.notStarted) + +private func shutdown() { + let storage = _processMonitorState.withLock { state -> ProcessMonitorState.Storage? in + switch state { + case .failed(_), .notStarted: + return nil + case .started(let storage): + return storage + } + } - c.resume(with: Result { - // Here waitid should not block because `pid` has already terminated at this point. - while true { - var siginfo = siginfo_t() - errno = 0 - if waitid(P_PID, numericCast(pid), &siginfo, WEXITED) == 0 { - var status: TerminationStatus? = nil - switch siginfo.si_code { - case .init(CLD_EXITED): - return .exited(siginfo.si_status) - case .init(CLD_KILLED), .init(CLD_DUMPED): - return .unhandledException(siginfo.si_status) - default: - fatalError("Unexpected exit status: \(siginfo.si_code)") - } - } else if errno != EINTR { - throw SubprocessError.UnderlyingError(rawValue: errno) - } - } - }) - } else if errno == ECHILD { - // We got ECHILD. If there are no continuations added right now, we should - // suspend this thread on the no-children condition until it's awoken by a - // newly-scheduled waiter process. (If this condition is spuriously - // woken, we'll just loop again, which is fine.) Note that we read errno - // outside the lock in case acquiring the lock perturbs it. - _childProcessContinuations.withUnsafeUnderlyingLock { lock, childProcessContinuations in - if childProcessContinuations.isEmpty { - _ = pthread_cond_wait(_waitThreadNoChildrenCondition, lock) - } - } + guard let storage else { + return + } + + var one: UInt64 = 1 + // Wake up the thread for shutdown + _ = _SubprocessCShims.write(storage.shutdownFileDescriptor, &one, MemoryLayout.size) + // Cleanup the monitor thread + pthread_join(storage.monitorThread, nil) +} + + +/// See the following page for the complete list of `async-signal-safe` functions +/// https://man7.org/linux/man-pages/man7/signal-safety.7.html +/// Only these functions can be used in the signal handler below +private func signalHandler( + _ signalNumber: CInt, + _ signalInfo: UnsafeMutablePointer?, + _ context: UnsafeMutableRawPointer? +) { + let savedErrno = errno + var one: UInt8 = 1 + _SubprocessCShims.write(_signalPipe.writeEnd, &one, 1) + errno = savedErrno +} + +private func monitorThreadFunc(args: UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer? { + let unmanaged = Unmanaged.fromOpaque(args!) + let context = unmanaged.takeRetainedValue() + + var events: [epoll_event] = Array( + repeating: epoll_event(events: 0, data: epoll_data(fd: 0)), + count: 256 + ) + var waitMask = sigset_t(); + sigemptyset(&waitMask); + sigaddset(&waitMask, SIGCHLD); + // Enter the monitor loop + monitorLoop: while true { + let eventCount = epoll_pwait( + context.epollFileDescriptor, + &events, + CInt(events.count), + -1, + &waitMask + ) + if eventCount < 0 { + let pwaitErrno = errno + if pwaitErrno == EINTR || pwaitErrno == EAGAIN { + continue // interrupted by signal; try again + } + // Report other errors + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: pwaitErrno) + ) + let continuations = _processMonitorState.withLock { state -> [CheckedContinuation] in + let result: [CheckedContinuation] + if case .started(let storage) = state { + result = Array(storage.continuations.values) + } else { + result = [] } + state = .failed(error) + return result } - }, - nil + // Report error to all existing continuations + for continuation in continuations { + continuation.resume(throwing: error) + } + break monitorLoop + } + + for index in 0 ..< Int(eventCount) { + let event = events[index] + let targetFileDescriptor = event.data.fd + + // Breakout the monitor loop if we received shutdown + // from the shutdownFD + if targetFileDescriptor == context.shutdownFileDescriptor { + var buf: UInt64 = 0 + _ = _SubprocessCShims.read(context.shutdownFileDescriptor, &buf, MemoryLayout.size) + break monitorLoop + } + + // P_PIDFD requires Linux Kernel 5.4 and above + if _waitProcessFileDescriptorSupported { + _blockAndWaitForProcessFileDescriptor(targetFileDescriptor, context: context) + } else { + _reapAllKnownChildProcesses(targetFileDescriptor, context: context) + } + } + } + + return nil +} + +private let setup: () = { + func _reportFailureWithErrno(_ number: CInt) { + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: number) + ) + _processMonitorState.withLock { state in + state = .failed(error) + } + } + + // Create the epollfd for monitoring + let epollFileDescriptor = epoll_create1(CInt(EPOLL_CLOEXEC)) + guard epollFileDescriptor >= 0 else { + _reportFailureWithErrno(errno) + return + } + // Create shutdownFileDescriptor + let shutdownFileDescriptor = eventfd(0, CInt(EFD_NONBLOCK | EFD_CLOEXEC)) + guard shutdownFileDescriptor >= 0 else { + _reportFailureWithErrno(errno) + return + } + + // Register shutdownFileDescriptor with epoll + var event = epoll_event( + events: EPOLLIN.rawValue, + data: epoll_data(fd: shutdownFileDescriptor) + ) + var rc = epoll_ctl( + epollFileDescriptor, + EPOLL_CTL_ADD, + shutdownFileDescriptor, + &event ) + guard rc == 0 else { + _reportFailureWithErrno(errno) + return + } + + // If the current kernel does not support pidfd, fallback to signal handler + // Create the self-pipe that signal handler writes to + if !_isWaitProcessFileDescriptorSupported() { + var pipeCreationError: SubprocessError? = nil + do { + let (readEnd, writeEnd) = try FileDescriptor.pipe() + _signalPipe = (readEnd.rawValue, writeEnd.rawValue) + } catch { + var underlying: SubprocessError.UnderlyingError? = nil + if let err = error as? Errno { + underlying = .init(rawValue: err.rawValue) + } + pipeCreationError = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: underlying + ) + } + if let pipeCreationError { + _processMonitorState.withLock { state in + state = .failed(pipeCreationError) + } + return + } + // Register the read end with epoll so we can get updates + // about it. The write end is written by the signal hander + var event = epoll_event( + events: EPOLLIN.rawValue, + data: epoll_data(fd: _signalPipe.readEnd) + ) + rc = epoll_ctl( + epollFileDescriptor, + EPOLL_CTL_ADD, + _signalPipe.readEnd, + &event + ) + guard rc == 0 else { + _reportFailureWithErrno(errno) + return + } + } else { + // Mark waitid(P_PIDFD) as supported + _waitProcessFileDescriptorSupported = true + } + let monitorThreadContext = MonitorThreadContext( + epollFileDescriptor: epollFileDescriptor, + shutdownFileDescriptor: shutdownFileDescriptor + ) + let unmanagedContext = Unmanaged.passRetained(monitorThreadContext) + // Create the monitor thread + var thread = pthread_t() + pthread_create(&thread, nil, monitorThreadFunc, unmanagedContext.toOpaque()) + + let storage = ProcessMonitorState.Storage( + epollFileDescriptor: epollFileDescriptor, + shutdownFileDescriptor: shutdownFileDescriptor, + monitorThread: thread, + continuations: [:] + ) + + _processMonitorState.withLock { state in + state = .started(storage) + } + + atexit { + shutdown() + } }() + private func _setupMonitorSignalHandler() { // Only executed once setup } +private func _blockAndWaitForProcessFileDescriptor(_ pidfd: CInt, context: MonitorThreadContext) { + var terminationStatus: Result + + var siginfo = siginfo_t() + if 0 == waitid(idtype_t(UInt32(P_PIDFD)), id_t(pidfd), &siginfo, WEXITED) { + switch siginfo.si_code { + case .init(CLD_EXITED): + terminationStatus = .success(.exited(siginfo.si_status)) + case .init(CLD_KILLED), .init(CLD_DUMPED): + terminationStatus = .success(.unhandledException(siginfo.si_status)) + default: + fatalError("Unexpected exit status: \(siginfo.si_code)") + } + } else { + let waitidErrno = errno + terminationStatus = .failure(SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: waitidErrno)) + ) + } + + // Remove this pidfd from epoll to prevent further notifications + let rc = epoll_ctl( + context.epollFileDescriptor, + EPOLL_CTL_DEL, + pidfd, + nil + ) + if rc != 0 { + let epollErrno = errno + terminationStatus = .failure(SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: epollErrno) + )) + } + // Notify the continuation + let continuation = _processMonitorState.withLock { state -> CheckedContinuation? in + guard case .started(let storage) = state, + let continuation = storage.continuations[pidfd] else { + return nil + } + // Remove registration + var newStorage = storage + newStorage.continuations.removeValue(forKey: pidfd) + state = .started(newStorage) + return continuation + } + continuation?.resume(with: terminationStatus) +} + +// On older kernel, fallback to using signal handlers +private typealias ResultContinuation = ( + result: Result, + continuation: CheckedContinuation +) +private func _reapAllKnownChildProcesses(_ signalFd: CInt, context: MonitorThreadContext) { + guard signalFd == _signalPipe.readEnd else { + return + } + + // Drain the signalFd + var buffer: UInt8 = 0 + while _SubprocessCShims.read(signalFd, &buffer, 1) > 0 { /* noop, drain the pipe */ } + + let resumingContinuations: [ResultContinuation] = _processMonitorState.withLock { state in + guard case .started(let storage) = state else { + return [] + } + var updatedContinuations = storage.continuations + var results: [ResultContinuation] = [] + // Since Linux coalesce signals, we need to loop through all known child process + // to check if they exited. + for knownChildPID in storage.continuations.keys { + let terminationStatus: Result + var siginfo = siginfo_t() + // Use `WNOHANG` here so waitid isn't blocking because we expect some + // child processes might be still running + if 0 == waitid(P_PID, id_t(knownChildPID), &siginfo, WEXITED | WNOHANG) { + // If si_pid and si_signo, the child is still running since we used WNOHANG + if siginfo.si_pid == 0 && siginfo.si_signo == 0 { + // Move on to the next child + continue + } + + switch siginfo.si_code { + case .init(CLD_EXITED): + terminationStatus = .success(.exited(siginfo.si_status)) + case .init(CLD_KILLED), .init(CLD_DUMPED): + terminationStatus = .success(.unhandledException(siginfo.si_status)) + default: + fatalError("Unexpected exit status: \(siginfo.si_code)") + } + } else { + let waitidErrno = errno + terminationStatus = .failure(SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: waitidErrno)) + ) + } + results.append((result: terminationStatus, continuation: storage.continuations[knownChildPID]!)) + // Now we have the exit code, remove saved continuations + updatedContinuations.removeValue(forKey: knownChildPID) + } + var updatedStorage = storage + updatedStorage.continuations = updatedContinuations + state = .started(updatedStorage) + + return results + } + // Resume continuations + for c in resumingContinuations { + c.continuation.resume(with: c.result) + } +} + +internal func _isWaitProcessFileDescriptorSupported() -> Bool { + // waitid(P_PIDFD) is only supported on Linux kernel 5.4 and above + // Prob whether the current system supports it by calling it with self pidfd + // and checking for EINVAL (waitid sets errno to EINVAL if it does not + // recognize the id type). + var siginfo = siginfo_t() + let selfPidfd = _pidfd_open(getpid()) + if selfPidfd < 0 { + // If we can not retrieve pidfd, the system does not support waitid(P_PIDFD) + return false + } + /// The following call will fail either with + /// - ECHILD: in this case we know P_PIDFD is supported and waitid correctly + /// reported that we don't have a child with the same selfPidfd; + /// - EINVAL: in this case we know P_PIDFD is not supported because it does not + /// recognize the `P_PIDFD` type + waitid(idtype_t(UInt32(P_PIDFD)), id_t(selfPidfd), &siginfo, WEXITED | WNOWAIT) + return errno == ECHILD +} + + #endif // canImport(Glibc) || canImport(Android) || canImport(Musl) diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index 122e5c5..779adbd 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -90,24 +90,6 @@ public struct Signal: Hashable, Sendable { public static var windowSizeChange: Self { .init(rawValue: SIGWINCH) } } -// MARK: - ProcessIdentifier - -/// A platform independent identifier for a Subprocess. -public struct ProcessIdentifier: Sendable, Hashable, Codable { - /// The platform specific process identifier value - public let value: pid_t - - public init(value: pid_t) { - self.value = value - } -} - -extension ProcessIdentifier: CustomStringConvertible, CustomDebugStringConvertible { - public var description: String { "\(self.value)" } - - public var debugDescription: String { "\(self.value)" } -} - extension Execution { /// Send the given signal to the child process. /// - Parameters: @@ -118,13 +100,35 @@ extension Execution { signal: Signal, toProcessGroup shouldSendToProcessGroup: Bool = false ) throws { + func _kill(_ pid: pid_t, signal: Signal) throws { + guard kill(pid, signal.rawValue) == 0 else { + throw SubprocessError( + code: .init(.failedToSendSignal(signal.rawValue)), + underlyingError: .init(rawValue: errno) + ) + } + } let pid = shouldSendToProcessGroup ? -(processIdentifier.value) : processIdentifier.value - guard kill(pid, signal.rawValue) == 0 else { - throw SubprocessError( - code: .init(.failedToSendSignal(signal.rawValue)), - underlyingError: .init(rawValue: errno) - ) + + #if os(Linux) || os(Android) + // On linux, use pidfd_send_signal if possible + if shouldSendToProcessGroup { + // pidfd_send_signal does not support sending signal to process group + try _kill(pid, signal: signal) + } else { + guard _pidfd_send_signal( + processIdentifier.processFileDescriptor, + signal.rawValue + ) == 0 else { + throw SubprocessError( + code: .init(.failedToSendSignal(signal.rawValue)), + underlyingError: .init(rawValue: errno) + ) + } } + #else + try _kill(pid, signal: signal) + #endif } } diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index fe3c54f..7fe6dcb 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -135,12 +135,12 @@ extension Configuration { } let pid = ProcessIdentifier( - value: processInfo.dwProcessId + value: processInfo.dwProcessId, + processHandle: processInfo.hProcess, + threadHandle: processInfo.hThread ) let execution = Execution( - processIdentifier: pid, - processInformation: processInfo, - consoleBehavior: self.platformOptions.consoleBehavior + processIdentifier: pid ) do { @@ -156,7 +156,7 @@ extension Configuration { } catch { // If spawn() throws, monitorProcessTermination or runDetached // won't have an opportunity to call release, so do it here to avoid leaking the handles. - execution.release() + pid.close() throw error } @@ -279,12 +279,12 @@ extension Configuration { } let pid = ProcessIdentifier( - value: processInfo.dwProcessId + value: processInfo.dwProcessId, + processHandle: processInfo.hProcess, + threadHandle: processInfo.hThread ) let execution = Execution( - processIdentifier: pid, - processInformation: processInfo, - consoleBehavior: self.platformOptions.consoleBehavior + processIdentifier: pid ) do { @@ -300,7 +300,7 @@ extension Configuration { } catch { // If spawn() throws, monitorProcessTermination or runDetached // won't have an opportunity to call release, so do it here to avoid leaking the handles. - execution.release() + pid.close() throw error } @@ -460,7 +460,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forExecution execution: Execution + for processIdentifier: ProcessIdentifier ) async throws -> TerminationStatus { // Once the continuation resumes, it will need to unregister the wait, so // yield the wait handle back to the calling scope. @@ -469,8 +469,6 @@ internal func monitorProcessTermination( if let waitHandle { _ = UnregisterWait(waitHandle) } - - execution.release() } try? await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in @@ -489,7 +487,7 @@ internal func monitorProcessTermination( guard RegisterWaitForSingleObject( &waitHandle, - execution.processInformation.hProcess, + processIdentifier.processHandle, callback, context, INFINITE, @@ -507,7 +505,7 @@ internal func monitorProcessTermination( } var status: DWORD = 0 - guard GetExitCodeProcess(execution.processInformation.hProcess, &status) else { + guard GetExitCodeProcess(processIdentifier.processHandle, &status) else { // The child process terminated but we couldn't get its status back. // Assume generic failure. return .exited(1) @@ -525,7 +523,7 @@ extension Execution { /// Terminate the current subprocess with the given exit code /// - Parameter exitCode: The exit code to use for the subprocess. public func terminate(withExitCode exitCode: DWORD) throws { - guard TerminateProcess(processInformation.hProcess, exitCode) else { + guard TerminateProcess(self.processIdentifier.processHandle, exitCode) else { throw SubprocessError( code: .init(.failedToTerminate), underlyingError: .init(rawValue: GetLastError()) @@ -549,7 +547,7 @@ extension Execution { underlyingError: .init(rawValue: GetLastError()) ) } - guard NTSuspendProcess(processInformation.hProcess) >= 0 else { + guard NTSuspendProcess(self.processIdentifier.processHandle) >= 0 else { throw SubprocessError( code: .init(.failedToSuspend), underlyingError: .init(rawValue: GetLastError()) @@ -573,7 +571,7 @@ extension Execution { underlyingError: .init(rawValue: GetLastError()) ) } - guard NTResumeProcess(processInformation.hProcess) >= 0 else { + guard NTResumeProcess(self.processIdentifier.processHandle) >= 0 else { throw SubprocessError( code: .init(.failedToResume), underlyingError: .init(rawValue: GetLastError()) @@ -676,12 +674,26 @@ extension Environment { // MARK: - ProcessIdentifier /// A platform independent identifier for a subprocess. -public struct ProcessIdentifier: Sendable, Hashable, Codable { +public struct ProcessIdentifier: Sendable, Hashable { /// Windows specific process identifier value public let value: DWORD + internal nonisolated(unsafe) let processHandle: HANDLE + internal nonisolated(unsafe) let threadHandle: HANDLE + - internal init(value: DWORD) { + internal init(value: DWORD, processHandle: HANDLE, threadHandle: HANDLE) { self.value = value + self.processHandle = processHandle + self.threadHandle = threadHandle + } + + internal func close() { + guard CloseHandle(self.threadHandle) else { + fatalError("Failed to close thread HANDLE: \(SubprocessError.UnderlyingError(rawValue: GetLastError()))") + } + guard CloseHandle(self.processHandle) else { + fatalError("Failed to close process HANDLE: \(SubprocessError.UnderlyingError(rawValue: GetLastError()))") + } } } diff --git a/Sources/Subprocess/Result.swift b/Sources/Subprocess/Result.swift index 88bcc45..28a362a 100644 --- a/Sources/Subprocess/Result.swift +++ b/Sources/Subprocess/Result.swift @@ -64,7 +64,6 @@ extension CollectedResult: Equatable where Output.OutputType: Equatable, Error.O extension CollectedResult: Hashable where Output.OutputType: Hashable, Error.OutputType: Hashable {} -extension CollectedResult: Codable where Output.OutputType: Codable, Error.OutputType: Codable {} extension CollectedResult: CustomStringConvertible where Output.OutputType: CustomStringConvertible, Error.OutputType: CustomStringConvertible { @@ -99,8 +98,6 @@ extension ExecutionResult: Equatable where Result: Equatable {} extension ExecutionResult: Hashable where Result: Hashable {} -extension ExecutionResult: Codable where Result: Codable {} - extension ExecutionResult: CustomStringConvertible where Result: CustomStringConvertible { public var description: String { return """ diff --git a/Sources/_SubprocessCShims/include/process_shims.h b/Sources/_SubprocessCShims/include/process_shims.h index 6eb185e..64b7313 100644 --- a/Sources/_SubprocessCShims/include/process_shims.h +++ b/Sources/_SubprocessCShims/include/process_shims.h @@ -23,7 +23,9 @@ #if TARGET_OS_LINUX #include +#include #include +#include #endif // TARGET_OS_LINUX #if __has_include() @@ -47,6 +49,7 @@ int _subprocess_spawn( int _subprocess_fork_exec( pid_t * _Nonnull pid, + int * _Nonnull pidfd, const char * _Nonnull exec_path, const char * _Nullable working_directory, const int file_descriptors[_Nonnull], @@ -78,6 +81,16 @@ int _shims_snprintf( char * _Nonnull str1, char * _Nonnull str2 ); + +int _pidfd_open(pid_t pid); +int _pidfd_send_signal(int pidfd, int signal); + +// P_PIDFD is only defined on Linux Kernel 5.4 and above +// Define our value if it's not available +#ifndef P_PIDFD +#define P_PIDFD 3 +#endif + #endif #endif // !TARGET_OS_WINDOWS diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index c6a70d1..867625c 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -14,6 +14,10 @@ #if TARGET_OS_LINUX // For posix_spawn_file_actions_addchdir_np #define _GNU_SOURCE 1 +// For pidfd_open +#include +#include +#include #endif #include "include/process_shims.h" @@ -26,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -79,6 +82,11 @@ int _shims_snprintf( ) { return snprintf(str, len, format, str1, str2); } + +int _pidfd_send_signal(int pidfd, int signal) { + return syscall(SYS_pidfd_send_signal, pidfd, signal, NULL, 0); +} + #endif #if __has_include() @@ -303,157 +311,34 @@ int _subprocess_spawn( #define __GLIBC_PREREQ(maj, min) 0 #endif -#if _POSIX_SPAWN -static int _subprocess_is_addchdir_np_available() { -#if defined(__GLIBC__) && !__GLIBC_PREREQ(2, 29) - // Glibc versions prior to 2.29 don't support posix_spawn_file_actions_addchdir_np, impacting: - // - Amazon Linux 2 (EoL mid-2025) - return 0; -#elif defined(__OpenBSD__) || defined(__QNX__) - // Currently missing as of: - // - OpenBSD 7.5 (April 2024) - // - QNX 8 (December 2023) - return 0; -#elif defined(__GLIBC__) || TARGET_OS_DARWIN || defined(__FreeBSD__) || (defined(__ANDROID__) && __ANDROID_API__ >= 34) || defined(__musl__) - // Pre-standard posix_spawn_file_actions_addchdir_np version available in: - // - Solaris 11.3 (October 2015) - // - Glibc 2.29 (February 2019) - // - macOS 10.15 (October 2019) - // - musl 1.1.24 (October 2019) - // - FreeBSD 13.1 (May 2022) - // - Android 14 (October 2023) - return 1; -#else - // Standardized posix_spawn_file_actions_addchdir version (POSIX.1-2024, June 2024) available in: - // - Solaris 11.4 (August 2018) - // - NetBSD 10.0 (March 2024) - return 1; -#endif +int _pidfd_open(pid_t pid) { + return syscall(SYS_pidfd_open, pid, 0); } -static int _subprocess_addchdir_np( - posix_spawn_file_actions_t *file_actions, - const char * __restrict path -) { -#if defined(__GLIBC__) && !__GLIBC_PREREQ(2, 29) - // Glibc versions prior to 2.29 don't support posix_spawn_file_actions_addchdir_np, impacting: - // - Amazon Linux 2 (EoL mid-2025) - return ENOTSUP; -#elif defined(__ANDROID__) && __ANDROID_API__ < 34 - // Android versions prior to 14 (API level 34) don't support posix_spawn_file_actions_addchdir_np - return ENOTSUP; -#elif defined(__OpenBSD__) || defined(__QNX__) - // Currently missing as of: - // - OpenBSD 7.5 (April 2024) - // - QNX 8 (December 2023) - return ENOTSUP; -#elif defined(__GLIBC__) || TARGET_OS_DARWIN || defined(__FreeBSD__) || defined(__ANDROID__) || defined(__musl__) - // Pre-standard posix_spawn_file_actions_addchdir_np version available in: - // - Solaris 11.3 (October 2015) - // - Glibc 2.29 (February 2019) - // - macOS 10.15 (October 2019) - // - musl 1.1.24 (October 2019) - // - FreeBSD 13.1 (May 2022) - // - Android 14 (API level 34) (October 2023) - return posix_spawn_file_actions_addchdir_np(file_actions, path); -#else - // Standardized posix_spawn_file_actions_addchdir version (POSIX.1-2024, June 2024) available in: - // - Solaris 11.4 (August 2018) - // - NetBSD 10.0 (March 2024) - return posix_spawn_file_actions_addchdir(file_actions, path); +// SYS_clone3 is only defined on Linux Kernel 5.3 and above +// Define our dummy value if it's not available +#ifndef SYS_clone3 +#define SYS_clone3 0xFFFF #endif -} - -static int _subprocess_posix_spawn_fallback( - pid_t * _Nonnull pid, - const char * _Nonnull exec_path, - const char * _Nullable working_directory, - const int file_descriptors[_Nonnull], - char * _Nullable const args[_Nonnull], - char * _Nullable const env[_Nullable], - gid_t * _Nullable process_group_id -) { - // Setup stdin, stdout, and stderr - posix_spawn_file_actions_t file_actions; - - int rc = posix_spawn_file_actions_init(&file_actions); - if (rc != 0) { return rc; } - if (file_descriptors[0] >= 0) { - rc = posix_spawn_file_actions_adddup2( - &file_actions, file_descriptors[0], STDIN_FILENO - ); - if (rc != 0) { return rc; } - } - if (file_descriptors[2] >= 0) { - rc = posix_spawn_file_actions_adddup2( - &file_actions, file_descriptors[2], STDOUT_FILENO - ); - if (rc != 0) { return rc; } - } - if (file_descriptors[4] >= 0) { - rc = posix_spawn_file_actions_adddup2( - &file_actions, file_descriptors[4], STDERR_FILENO - ); - if (rc != 0) { return rc; } - } - // Setup working directory - if (working_directory != NULL) { - rc = _subprocess_addchdir_np(&file_actions, working_directory); - if (rc != 0) { - return rc; - } - } - - // Close parent side - if (file_descriptors[1] >= 0) { - rc = posix_spawn_file_actions_addclose(&file_actions, file_descriptors[1]); - if (rc != 0) { return rc; } - } - if (file_descriptors[3] >= 0) { - rc = posix_spawn_file_actions_addclose(&file_actions, file_descriptors[3]); - if (rc != 0) { return rc; } - } - if (file_descriptors[5] >= 0) { - rc = posix_spawn_file_actions_addclose(&file_actions, file_descriptors[5]); - if (rc != 0) { return rc; } - } - // Setup spawnattr - posix_spawnattr_t spawn_attr; - rc = posix_spawnattr_init(&spawn_attr); - if (rc != 0) { return rc; } - // Masks - sigset_t no_signals; - sigset_t all_signals; - sigemptyset(&no_signals); - sigfillset(&all_signals); - rc = posix_spawnattr_setsigmask(&spawn_attr, &no_signals); - if (rc != 0) { return rc; } - rc = posix_spawnattr_setsigdefault(&spawn_attr, &all_signals); - if (rc != 0) { return rc; } - // Flags - short flags = POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF; - if (process_group_id != NULL) { - flags |= POSIX_SPAWN_SETPGROUP; - rc = posix_spawnattr_setpgroup(&spawn_attr, *process_group_id); - if (rc != 0) { return rc; } - } - rc = posix_spawnattr_setflags(&spawn_attr, flags); - - // Spawn! - rc = posix_spawn( - pid, exec_path, - &file_actions, &spawn_attr, - args, env - ); - posix_spawn_file_actions_destroy(&file_actions); - posix_spawnattr_destroy(&spawn_attr); - return rc; +static int _clone3(int *pidfd) { + struct clone_args args = { + .flags = CLONE_PIDFD, // Get a pidfd referring to child + .pidfd = (uintptr_t)pidfd, // Int pointer for the pidfd (int pidfd = -1;) + .exit_signal = SIGCHLD, // Ensure parent gets SIGCHLD + .stack = 0, // No stack needed for separate address space + .stack_size = 0, + .parent_tid = 0, + .child_tid = 0, + .tls = 0 + }; + + return syscall(SYS_clone3, &args, sizeof(args)); } -#endif // _POSIX_SPAWN int _subprocess_fork_exec( pid_t * _Nonnull pid, + int * _Nonnull pidfd, const char * _Nonnull exec_path, const char * _Nullable working_directory, const int file_descriptors[_Nonnull], @@ -471,32 +356,6 @@ int _subprocess_fork_exec( close(pipefd[1]); \ _exit(EXIT_FAILURE) - int require_pre_fork = _subprocess_is_addchdir_np_available() == 0 || - uid != NULL || - gid != NULL || - process_group_id != NULL || - (number_of_sgroups > 0 && sgroups != NULL) || - create_session || - configurator != NULL; - -#if _POSIX_SPAWN - // If posix_spawn is available on this platform and - // we do not require prefork, use posix_spawn if possible. - // - // (Glibc's posix_spawn does not support - // `POSIX_SPAWN_SETEXEC` therefore we have to keep - // using fork/exec if `require_pre_fork` is true. - if (require_pre_fork == 0) { - return _subprocess_posix_spawn_fallback( - pid, exec_path, - working_directory, - file_descriptors, - args, env, - process_group_id - ); - } -#endif - // Setup pipe to catch exec failures from child int pipefd[2]; if (pipe(pipefd) != 0) { @@ -543,11 +402,25 @@ int _subprocess_fork_exec( return errno; } - // Finally, fork + // Finally, fork / clone + int _pidfd = -1; + // First attempt to use clone3, only fall back to fork if clone3 is not available + pid_t childPid = _clone3(&_pidfd); + if (childPid < 0) { + if (errno == ENOSYS) { + // clone3 is not implemented. Use fork instead #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated" - pid_t childPid = fork(); + childPid = fork(); #pragma GCC diagnostic pop + } else { + // Report all other errors + close(pipefd[0]); + close(pipefd[1]); + return errno; + } + } + if (childPid < 0) { // Fork failed close(pipefd[0]); @@ -557,8 +430,6 @@ int _subprocess_fork_exec( if (childPid == 0) { // Child process - close(pipefd[0]); // Close unused read end - // Reset signal handlers for (int signo = 1; signo < _SUBPROCESS_SIG_MAX; signo++) { if (signo == SIGKILL || signo == SIGSTOP) { @@ -620,25 +491,31 @@ int _subprocess_fork_exec( // Bind stdin, stdout, and stderr if (file_descriptors[0] >= 0) { rc = dup2(file_descriptors[0], STDIN_FILENO); - if (rc < 0) { - write_error_and_exit; - } + } else { + rc = close(STDIN_FILENO); + } + if (rc < 0) { + write_error_and_exit; } + if (file_descriptors[2] >= 0) { rc = dup2(file_descriptors[2], STDOUT_FILENO); - if (rc < 0) { - write_error_and_exit; - } + } else { + rc = close(STDOUT_FILENO); } + if (rc < 0) { + write_error_and_exit; + } + if (file_descriptors[4] >= 0) { rc = dup2(file_descriptors[4], STDERR_FILENO); - if (rc < 0) { - int error = errno; - write(pipefd[1], &error, sizeof(error)); - close(pipefd[1]); - _exit(EXIT_FAILURE); - } + } else { + rc = close(STDERR_FILENO); + } + if (rc < 0) { + write_error_and_exit; } + // Close parent side if (file_descriptors[1] >= 0) { rc = close(file_descriptors[1]); @@ -649,12 +526,11 @@ int _subprocess_fork_exec( if (file_descriptors[5] >= 0) { rc = close(file_descriptors[5]); } - if (rc != 0) { - int error = errno; - write(pipefd[1], &error, sizeof(error)); - close(pipefd[1]); - _exit(EXIT_FAILURE); + + if (rc < 0) { + write_error_and_exit; } + // Run custom configuratior if (configurator != NULL) { configurator(); @@ -664,6 +540,15 @@ int _subprocess_fork_exec( // If we reached this point, something went wrong write_error_and_exit; } else { + // On Linux 5.3 and lower, we have to fetch pidfd separately + // Newer Linux supports clone3 which returns pidfd directly + if (_pidfd < 0) { + _pidfd = _pidfd_open(childPid); + if (_pidfd < 0) { + return errno; + } + } + // Parent process close(pipefd[1]); // Close unused write end @@ -680,6 +565,7 @@ int _subprocess_fork_exec( // Communicate child pid back *pid = childPid; + *pidfd = _pidfd; // Read from the pipe until pipe is closed // either due to exec succeeds or error is written while (1) { diff --git a/Tests/SubprocessTests/PlatformConformance.swift b/Tests/SubprocessTests/PlatformConformance.swift new file mode 100644 index 0000000..8cbbacf --- /dev/null +++ b/Tests/SubprocessTests/PlatformConformance.swift @@ -0,0 +1,40 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 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 +// +//===----------------------------------------------------------------------===// + +@testable import Subprocess + +#if canImport(Darwin) +import Darwin +#elseif canImport(Bionic) +import Bionic +#elseif canImport(Glibc) +import Glibc +#elseif canImport(Musl) +import Musl +#elseif canImport(WinSDK) +import WinSDK +#endif + +/// This file defines protocols for platform-specific structs +/// and adds retroactive conformances to them to ensure they all +/// conform to a uniform shape. We opted to keep these protocols +/// in the test target as opposed to making them public APIs +/// because we don't directly use them in public APIs. + +protocol ProcessIdentifierProtocol: Sendable, Hashable, CustomStringConvertible, CustomDebugStringConvertible { + #if os(Windows) + var value: DWORD { get } + #else + var value: pid_t { get } + #endif +} + +extension ProcessIdentifier : ProcessIdentifierProtocol {} diff --git a/Tests/SubprocessTests/SubprocessTests+Unix.swift b/Tests/SubprocessTests/SubprocessTests+Unix.swift index 4dc1a14..3801ebe 100644 --- a/Tests/SubprocessTests/SubprocessTests+Unix.swift +++ b/Tests/SubprocessTests/SubprocessTests+Unix.swift @@ -796,12 +796,13 @@ extension SubprocessUnixTests { @Test func testTerminateProcess() async throws { let stuckResult = try await Subprocess.run( // This will intentionally hang - .path("/bin/cat"), + .path("/bin/sleep"), + arguments: ["infinity"], + output: .discarded, error: .discarded - ) { subprocess, standardOutput in + ) { subprocess in // Make sure we can send signals to terminate the process try subprocess.send(signal: .terminate) - for try await _ in standardOutput {} } guard case .unhandledException(let exception) = stuckResult.terminationStatus else { Issue.record("Wrong termination status reported: \(stuckResult.terminationStatus)") diff --git a/Tests/SubprocessTests/SubprocessTests+Windows.swift b/Tests/SubprocessTests/SubprocessTests+Windows.swift index 3b9bed7..abbe4b8 100644 --- a/Tests/SubprocessTests/SubprocessTests+Windows.swift +++ b/Tests/SubprocessTests/SubprocessTests+Windows.swift @@ -450,9 +450,12 @@ extension SubprocessWindowsTests { // MARK: - PlatformOption Tests extension SubprocessWindowsTests { - @Test(.enabled(if: SubprocessWindowsTests.hasAdminPrivileges())) + @Test( + .disabled("Disabled until we investigate CI specific failures"), + .bug("https://github.com/swiftlang/swift-subprocess/issues/128") + ) func testPlatformOptionsRunAsUser() async throws { - try await self.withTemporaryUser { username, password in + try await self.withTemporaryUser { username, password, succeed in // Use public directory as working directory so the newly created user // has access to it let workingDirectory = FilePath("C:\\Users\\Public") @@ -495,6 +498,12 @@ extension SubprocessWindowsTests { } return String(decodingCString: pointer, as: UTF16.self) } + // On CI, we might failed to create user due to privilege issues + // There's nothing much we can do in this case + guard succeed else { + // If we fail to create the user, skip this test + return true + } // CreateProcessWithLogonW doesn't appear to work when running in a container return whoamiResult.terminationStatus == .unhandledException(STATUS_DLL_INIT_FAILED) && userName() == "ContainerAdministrator" } @@ -729,16 +738,16 @@ extension SubprocessWindowsTests { // MARK: - User Utils extension SubprocessWindowsTests { private func withTemporaryUser( - _ work: (String, String) async throws -> Void + _ work: (String, String, Bool) async throws -> Void ) async throws { let username: String = "TestUser\(randomString(length: 5, lettersOnly: true))" let password: String = "Password\(randomString(length: 10))" - func createUser(withUsername username: String, password: String) { - username.withCString( + func createUser(withUsername username: String, password: String) -> Bool { + return username.withCString( encodedAs: UTF16.self ) { usernameW in - password.withCString( + return password.withCString( encodedAs: UTF16.self ) { passwordW in var userInfo: USER_INFO_1 = USER_INFO_1() @@ -759,27 +768,30 @@ extension SubprocessWindowsTests { &error ) guard status == NERR_Success else { - Issue.record("Failed to create user with error: \(error)") - return + return false } + return true } } } - createUser(withUsername: username, password: password) + let succeed = createUser(withUsername: username, password: password) + defer { - // Now delete the user - let status = username.withCString( - encodedAs: UTF16.self - ) { usernameW in - return NetUserDel(nil, usernameW) - } - if status != NERR_Success { - Issue.record("Failed to delete user with error: \(status)") + if succeed { + // Now delete the user + let status = username.withCString( + encodedAs: UTF16.self + ) { usernameW in + return NetUserDel(nil, usernameW) + } + if status != NERR_Success { + Issue.record("Failed to delete user with error: \(status)") + } } } // Run work - try await work(username, password) + try await work(username, password, succeed) } private static func hasAdminPrivileges() -> Bool {