From 165eca81c2740d8a6c445968fd6ec05da48499d4 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Tue, 15 Jul 2025 15:31:01 -0700 Subject: [PATCH 1/7] Introduce process file descriptor (pidfd) based process monitoring for Linux --- Sources/Subprocess/API.swift | 2 +- Sources/Subprocess/Configuration.swift | 7 +- Sources/Subprocess/Execution.swift | 14 - .../Platforms/Subprocess+Darwin.swift | 27 +- .../Platforms/Subprocess+Linux.swift | 392 +++++++++++++----- .../Platforms/Subprocess+Unix.swift | 50 ++- .../Platforms/Subprocess+Windows.swift | 46 +- Sources/Subprocess/Result.swift | 3 - .../_SubprocessCShims/include/process_shims.h | 3 + Sources/_SubprocessCShims/process_shims.c | 229 ++-------- 10 files changed, 413 insertions(+), 360 deletions(-) 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..3a4d0ed 100644 --- a/Sources/Subprocess/Execution.swift +++ b/Sources/Subprocess/Execution.swift @@ -35,16 +35,13 @@ public struct Execution: Sendable { 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 @@ -54,17 +51,6 @@ public struct Execution: Sendable { 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..59da289 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!, @@ -142,7 +144,10 @@ extension Configuration { ) let execution = Execution( - processIdentifier: .init(value: pid) + processIdentifier: .init( + value: pid, + processFileDescriptor: processFileDescriptor + ) ) return SpawnResult( execution: execution, @@ -181,6 +186,30 @@ extension Configuration { } } +// MARK: - ProcesIdentifier + +/// 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() { + _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 +286,70 @@ 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) + _processMonitorState.withLock { state in + switch state { + case .notStarted: + continuation.resume(throwing: SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: nil) + ) + case .failed(let error): + continuation.resume(throwing: error) + case .started(let storage): + // 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 error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + ) + continuation.resume(throwing: error) + return + } + // Now save the registration + var newState = storage + newState.continuations[processIdentifier.processFileDescriptor] = continuation + state = .started(newState) + } } } } -// 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) - - init() { - pthread_mutex_init(mutex, nil) +private enum ProcessMonitorState { + struct Storage { + let epollFileDescriptor: CInt + let shutdownFileDescriptor: CInt + let monitorThread: pthread_t + var continuations: [PlatformFileDescriptor : CheckedContinuation] } - func withLock(_ body: (inout [pid_t: CheckedContinuation]) throws -> R) rethrows -> R { - try withUnsafeUnderlyingLock { _, continuations in - try body(&continuations) - } - } - - 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) - } + 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,77 +371,225 @@ private extension siginfo_t { #endif } } -#endif + +private let _processMonitorState: Mutex = .init(.notStarted) + +private func shutdown() { + let state = _processMonitorState.withLock { state -> (shutdownFileDescriptor: CInt, monitorThread: pthread_t)? in + switch state { + case .failed(_), .notStarted: + return nil + case .started(let storage): + return (storage.shutdownFileDescriptor, storage.monitorThread) + } + } + + guard let state = state else { + return + } + + var one: UInt64 = 1 + // Wake up the thread for shutdown + _ = _SubprocessCShims.write(state.shutdownFileDescriptor, &one, MemoryLayout.stride) + // Cleanup the monitor thread + pthread_join(state.monitorThread, nil) +} private let setup: () = { + // Create the epollfd for monitoring + let epollFileDescriptor = epoll_create1(CInt(EPOLL_CLOEXEC)) + guard epollFileDescriptor >= 0 else { + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + ) + _processMonitorState.withLock { state in + guard case .started(let storage) = state else { + return + } + for continuation in storage.continuations.values { + continuation.resume(throwing: error) + } + } + return + } + // Create shutdownFileDescriptor + let shutdownFileDescriptor = eventfd(0, CInt(EFD_NONBLOCK | EFD_CLOEXEC)) + guard shutdownFileDescriptor >= 0 else { + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + ) + _processMonitorState.withLock { storage in + storage = .failed(error) + } + 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 { + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + ) + _processMonitorState.withLock { storage in + storage = .failed(error) + } + return + } + + // Create thread data + let context = MonitorThreadContext( + epollFileDescriptor: epollFileDescriptor, + shutdownFileDescriptor: shutdownFileDescriptor + ) + let threadContext = Unmanaged.passRetained(context) // 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. + rc = pthread_create(&thread, nil, { args in + func reportError(_ error: SubprocessError) { + _processMonitorState.withLock { state in + guard case .started(let storage) = state else { + return + } + for continuation in storage.continuations.values { + continuation.resume(throwing: error) + } + } + } + + 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 + ) + + // Enter the monitor loop + monitorLoop: while true { + let eventCount = epoll_wait( + context.epollFileDescriptor, + &events, + CInt(events.count), + -1 + ) + if eventCount < 0 { + if errno == EINTR || errno == EAGAIN { + continue // interrupted by signal; try again + } + // Report other errors + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + ) + reportError(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 + } + + var terminationStatus: Result + 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 + if 0 == waitid(P_PIDFD, id_t(targetFileDescriptor), &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 { + terminationStatus = .failure(SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno)) + ) + } - 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) - } + // Remove this pidfd from epoll to prevent further notifications + let rc = epoll_ctl( + context.epollFileDescriptor, + EPOLL_CTL_DEL, + targetFileDescriptor, + nil + ) + if rc != 0 { + terminationStatus = .failure(SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: errno) + )) + } + + // Notify the continuation + _processMonitorState.withLock { state in + guard case .started(let storage) = state, + let continuation = storage.continuations[targetFileDescriptor] else { + return + } + switch terminationStatus { + case .success(let value): + continuation.resume(returning: value) + case .failure(let error): + continuation.resume(throwing: error) } + // Remove registration + var newStorage = storage + newStorage.continuations.removeValue(forKey: targetFileDescriptor) + state = .started(newStorage) } } - }, - nil - ) + } + + return nil + },threadContext.toOpaque()) + guard rc == 0 else { + let error = SubprocessError( + code: .init(.failedToMonitorProcess), + underlyingError: .init(rawValue: rc) + ) + _processMonitorState.withLock { storage in + storage = .failed(error) + } + return + } + _processMonitorState.withLock { state in + let storage = ProcessMonitorState.Storage( + epollFileDescriptor: epollFileDescriptor, + shutdownFileDescriptor: shutdownFileDescriptor, + monitorThread: thread, + continuations: [:] + ) + state = .started(storage) + } + + atexit { + shutdown() + } }() + private func _setupMonitorSignalHandler() { // Only executed once setup diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index 122e5c5..c09d5e7 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) + // 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..e0bd237 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -135,11 +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 ) @@ -156,7 +157,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,11 +280,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 ) @@ -300,7 +302,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 +462,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 +471,6 @@ internal func monitorProcessTermination( if let waitHandle { _ = UnregisterWait(waitHandle) } - - execution.release() } try? await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in @@ -489,7 +489,7 @@ internal func monitorProcessTermination( guard RegisterWaitForSingleObject( &waitHandle, - execution.processInformation.hProcess, + processIdentifier.processHandle, callback, context, INFINITE, @@ -507,7 +507,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 +525,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 +549,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 +573,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 +676,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..fdd923a 100644 --- a/Sources/_SubprocessCShims/include/process_shims.h +++ b/Sources/_SubprocessCShims/include/process_shims.h @@ -47,6 +47,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 +79,8 @@ int _shims_snprintf( char * _Nonnull str1, char * _Nonnull str2 ); + +int _pidfd_send_signal(int pidfd, int signal); #endif #endif // !TARGET_OS_WINDOWS diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index c6a70d1..6486777 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -14,6 +14,8 @@ #if TARGET_OS_LINUX // For posix_spawn_file_actions_addchdir_np #define _GNU_SOURCE 1 +// For pidfd_open +#include #endif #include "include/process_shims.h" @@ -79,6 +81,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 +310,13 @@ 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 -} - -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); -#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 _pidfd_open(pid_t pid) { + return syscall(SYS_pidfd_open, pid, 0); } -#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 +334,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) { @@ -557,8 +394,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 +455,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 +490,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 +504,10 @@ int _subprocess_fork_exec( // If we reached this point, something went wrong write_error_and_exit; } else { + int _pidfd = _pidfd_open(childPid); + if (_pidfd < 0) { + return errno; + } // Parent process close(pipefd[1]); // Close unused write end @@ -680,6 +524,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) { From 199eb7294544bce33149ddabc44dffca950d70d9 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Tue, 22 Jul 2025 16:05:31 -0700 Subject: [PATCH 2/7] Use signal handler for process state monitoring on Linux kernel lower than 5.4 --- .../Platforms/Subprocess+Linux.swift | 550 ++++++++++++------ .../Platforms/Subprocess+Unix.swift | 2 +- .../_SubprocessCShims/include/process_shims.h | 9 + Sources/_SubprocessCShims/process_shims.c | 68 ++- .../SubprocessTests+Unix.swift | 7 +- 5 files changed, 447 insertions(+), 189 deletions(-) diff --git a/Sources/Subprocess/Platforms/Subprocess+Linux.swift b/Sources/Subprocess/Platforms/Subprocess+Linux.swift index 59da289..799db20 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Linux.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Linux.swift @@ -142,7 +142,6 @@ extension Configuration { errorRead: nil, errorWrite: errorWriteFileDescriptor ) - let execution = Execution( processIdentifier: .init( value: pid, @@ -186,7 +185,7 @@ extension Configuration { } } -// MARK: - ProcesIdentifier +// MARK: - ProcessIdentifier /// A platform independent identifier for a Subprocess. public struct ProcessIdentifier: Sendable, Hashable { @@ -200,7 +199,9 @@ public struct ProcessIdentifier: Sendable, Hashable { } internal func close() { - _SubprocessCShims.close(self.processFileDescriptor) + if self.processFileDescriptor > 0 { + _SubprocessCShims.close(self.processFileDescriptor) + } } } @@ -289,41 +290,86 @@ internal func monitorProcessTermination( for processIdentifier: ProcessIdentifier ) async throws -> TerminationStatus { return try await withCheckedThrowingContinuation { continuation in - _processMonitorState.withLock { state in + let status = _processMonitorState.withLock { state -> Result? in switch state { case .notStarted: - continuation.resume(throwing: SubprocessError( + let error = SubprocessError( code: .init(.failedToMonitorProcess), - underlyingError: nil) + underlyingError: nil ) + return .failure(error) case .failed(let error): - continuation.resume(throwing: error) + return .failure(error) case .started(let storage): - // 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 error = SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) + // 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 _isLinuxKernelAtLeast(major: 5, minor: 4) { + // Register processFileDescriptor with epoll + var event = epoll_event( + events: EPOLLIN.rawValue, + data: epoll_data(fd: processIdentifier.processFileDescriptor) ) - continuation.resume(throwing: error) - return + 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 + } + + 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) + } } - // Now save the registration - var newState = storage - newState.continuations[processIdentifier.processFileDescriptor] = continuation - state = .started(newState) } } + + if let status { + continuation.resume(with: status) + } } } @@ -372,57 +418,138 @@ private extension siginfo_t { } } +// Okay to be unlocked global mutable because this thread is only created once +private nonisolated(unsafe) var _signalPipe: (readEnd: CInt, writeEnd: CInt) = (readEnd: -1, writeEnd: -1) private let _processMonitorState: Mutex = .init(.notStarted) private func shutdown() { - let state = _processMonitorState.withLock { state -> (shutdownFileDescriptor: CInt, monitorThread: pthread_t)? in + let storage = _processMonitorState.withLock { state -> ProcessMonitorState.Storage? in switch state { case .failed(_), .notStarted: return nil case .started(let storage): - return (storage.shutdownFileDescriptor, storage.monitorThread) + return storage } } - guard let state = state else { + guard let storage else { return } var one: UInt64 = 1 // Wake up the thread for shutdown - _ = _SubprocessCShims.write(state.shutdownFileDescriptor, &one, MemoryLayout.stride) + _ = _SubprocessCShims.write(storage.shutdownFileDescriptor, &one, MemoryLayout.size) // Cleanup the monitor thread - pthread_join(state.monitorThread, nil) + 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 + } + // 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 _isLinuxKernelAtLeast(major: 5, minor: 4) { + _blockAndWaitForProcessFileDescriptor(targetFileDescriptor, context: context) + } else { + _reapAllKnownChildProcesses(targetFileDescriptor, context: context) + } + } + } + + return nil } private let setup: () = { - // Create the epollfd for monitoring - let epollFileDescriptor = epoll_create1(CInt(EPOLL_CLOEXEC)) - guard epollFileDescriptor >= 0 else { + func _reportFailureWithErrno(_ number: CInt) { let error = SubprocessError( code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) + underlyingError: .init(rawValue: number) ) _processMonitorState.withLock { state in - guard case .started(let storage) = state else { - return - } - for continuation in storage.continuations.values { - continuation.resume(throwing: error) - } + 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 { - let error = SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) - ) - _processMonitorState.withLock { storage in - storage = .failed(error) - } + _reportFailureWithErrno(errno) return } @@ -438,161 +565,226 @@ private let setup: () = { &event ) guard rc == 0 else { - let error = SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) - ) - _processMonitorState.withLock { storage in - storage = .failed(error) - } + _reportFailureWithErrno(errno) return } - // Create thread data - let context = MonitorThreadContext( + // On Linux kernel lower than 5.4 we have to rely on signal handler + // Create the self-pipe that signal handler writes to + if !_isLinuxKernelAtLeast(major: 5, minor: 4) { + 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 + } + } + let monitorThreadContext = MonitorThreadContext( epollFileDescriptor: epollFileDescriptor, shutdownFileDescriptor: shutdownFileDescriptor ) - let threadContext = Unmanaged.passRetained(context) - // Create the thread. It will run immediately; because it runs in an infinite - // loop, we aren't worried about detaching or joining it. + let unmanagedContext = Unmanaged.passRetained(monitorThreadContext) + // Create the monitor thread var thread = pthread_t() - rc = pthread_create(&thread, nil, { args in - func reportError(_ error: SubprocessError) { - _processMonitorState.withLock { state in - guard case .started(let storage) = state else { - return - } - for continuation in storage.continuations.values { - continuation.resume(throwing: error) - } - } - } + 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) + } - let unmanaged = Unmanaged.fromOpaque(args!) - let context = unmanaged.takeRetainedValue() + atexit { + shutdown() + } +}() + + +private func _setupMonitorSignalHandler() { + // Only executed once + setup +} - var events: [epoll_event] = Array( - repeating: epoll_event(events: 0, data: epoll_data(fd: 0)), - count: 256 +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)) ) + } - // Enter the monitor loop - monitorLoop: while true { - let eventCount = epoll_wait( - context.epollFileDescriptor, - &events, - CInt(events.count), - -1 - ) - if eventCount < 0 { - if errno == EINTR || errno == EAGAIN { - continue // interrupted by signal; try again - } - // Report other errors - let error = SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) - ) - reportError(error) - break monitorLoop - } + // 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) +} - 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 - } +// 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 + } - var terminationStatus: Result - - var siginfo = siginfo_t() - if 0 == waitid(P_PIDFD, id_t(targetFileDescriptor), &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 { - terminationStatus = .failure(SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno)) - ) - } + // Drain the signalFd + var buffer: UInt8 = 0 + while _SubprocessCShims.read(signalFd, &buffer, 1) > 0 { /* noop, drain the pipe */ } - // Remove this pidfd from epoll to prevent further notifications - let rc = epoll_ctl( - context.epollFileDescriptor, - EPOLL_CTL_DEL, - targetFileDescriptor, - nil - ) - if rc != 0 { - terminationStatus = .failure(SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: errno) - )) + 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 } - // Notify the continuation - _processMonitorState.withLock { state in - guard case .started(let storage) = state, - let continuation = storage.continuations[targetFileDescriptor] else { - return - } - switch terminationStatus { - case .success(let value): - continuation.resume(returning: value) - case .failure(let error): - continuation.resume(throwing: error) - } - // Remove registration - var newStorage = storage - newStorage.continuations.removeValue(forKey: targetFileDescriptor) - state = .started(newStorage) + 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 nil - },threadContext.toOpaque()) - guard rc == 0 else { - let error = SubprocessError( - code: .init(.failedToMonitorProcess), - underlyingError: .init(rawValue: rc) - ) - _processMonitorState.withLock { storage in - storage = .failed(error) - } - return + return results } - _processMonitorState.withLock { state in - let storage = ProcessMonitorState.Storage( - epollFileDescriptor: epollFileDescriptor, - shutdownFileDescriptor: shutdownFileDescriptor, - monitorThread: thread, - continuations: [:] - ) - state = .started(storage) + // Resume continuations + for c in resumingContinuations { + c.continuation.resume(with: c.result) } +} - atexit { - shutdown() +private func _isLinuxKernelAtLeast(major: Int, minor: Int) -> Bool { + var buffer = utsname() + guard uname(&buffer) == 0 else { + return false + } + let releaseString = withUnsafeBytes(of: &buffer.release) { ptr in + let buffer = ptr.bindMemory(to: CChar.self) + return String(cString: buffer.baseAddress!) } -}() + // utsname release follows the format + // major.minor.patch-extra + guard let mainVersion = releaseString.split(separator: "-").first else { + return false + } -private func _setupMonitorSignalHandler() { - // Only executed once - setup + let versionComponents = mainVersion.split(separator: ".") + + guard let currentMajor = Int(versionComponents[0]), + let currentMinor = Int(versionComponents[1]) else { + return false + } + + if currentMajor > major { + return true + } else if currentMajor == major { + return currentMinor >= minor + } else { + return false + } } #endif // canImport(Glibc) || canImport(Android) || canImport(Musl) diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index c09d5e7..779adbd 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -110,7 +110,7 @@ extension Execution { } let pid = shouldSendToProcessGroup ? -(processIdentifier.value) : processIdentifier.value - #if os(Linux) + #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 diff --git a/Sources/_SubprocessCShims/include/process_shims.h b/Sources/_SubprocessCShims/include/process_shims.h index fdd923a..be209cb 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() @@ -81,6 +83,13 @@ int _shims_snprintf( ); int _pidfd_send_signal(int pidfd, int signal); + +// P_PIDFD is only defined on Linux Kernel 5.4 and above +// Define our dummy 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 6486777..e7ac818 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -16,6 +16,8 @@ #define _GNU_SOURCE 1 // For pidfd_open #include +#include +#include #endif #include "include/process_shims.h" @@ -28,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -314,6 +315,46 @@ static int _pidfd_open(pid_t pid) { return syscall(SYS_pidfd_open, pid, 0); } +// 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 _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)); +} + +static int _linux_kernel_version_at_least( + int required_major, + int required_minor +) { + struct utsname buf; + if (uname(&buf) != 0) { + return 0; // Cannot determine kernel version + } + + int major = 0, minor = 0; + if (sscanf(buf.release, "%d.%d", &major, &minor) < 2) { + return 0; + } + + if (major > required_major) return 1; + if (major == required_major && minor >= required_minor) return 1; + return 0; +} + int _subprocess_fork_exec( pid_t * _Nonnull pid, int * _Nonnull pidfd, @@ -380,11 +421,21 @@ int _subprocess_fork_exec( return errno; } - // Finally, fork + // Finally, fork / clone + int _pidfd = -1; + pid_t childPid = -1; + if (_linux_kernel_version_at_least(5, 3)) { + // One Linux 5.3 and above, use the new clone3 syscall + // So we can atomically retrieve pidfd + childPid = _clone3(&_pidfd); + } else { + // On older linux versions, fall back to fork() #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated" - pid_t childPid = fork(); + childPid = fork(); #pragma GCC diagnostic pop + } + if (childPid < 0) { // Fork failed close(pipefd[0]); @@ -504,10 +555,15 @@ int _subprocess_fork_exec( // If we reached this point, something went wrong write_error_and_exit; } else { - int _pidfd = _pidfd_open(childPid); - if (_pidfd < 0) { - return errno; + // On Linux 5.3 and lower, we have to fetch pidfd separately + // Newer Linux supports clone3 which returns pidfd directly + if (_linux_kernel_version_at_least(5, 3) == 0) { + _pidfd = _pidfd_open(childPid); + if (_pidfd < 0) { + return errno; + } } + // Parent process close(pipefd[1]); // Close unused write end 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)") From 086c8d8358cc1193de42675e5ff8a8f9bcee1619 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Thu, 24 Jul 2025 10:47:31 -0700 Subject: [PATCH 3/7] Disable testPlatformOptionsRunAsUser on CI if we don't have privilege to create a temporary user --- .../SubprocessTests+Windows.swift | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/Tests/SubprocessTests/SubprocessTests+Windows.swift b/Tests/SubprocessTests/SubprocessTests+Windows.swift index 3b9bed7..080f1f1 100644 --- a/Tests/SubprocessTests/SubprocessTests+Windows.swift +++ b/Tests/SubprocessTests/SubprocessTests+Windows.swift @@ -452,7 +452,7 @@ extension SubprocessWindowsTests { extension SubprocessWindowsTests { @Test(.enabled(if: SubprocessWindowsTests.hasAdminPrivileges())) 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 +495,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 +735,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 +765,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 { From bbb5432402473b0a9ddbd59caba3ba95d2c496d4 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Fri, 25 Jul 2025 12:54:42 -0700 Subject: [PATCH 4/7] Remove unused consoleBehavior from Execution --- Sources/Subprocess/Execution.swift | 12 ------------ .../Subprocess/Platforms/Subprocess+Windows.swift | 6 ++---- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/Sources/Subprocess/Execution.swift b/Sources/Subprocess/Execution.swift index 3a4d0ed..4c28037 100644 --- a/Sources/Subprocess/Execution.swift +++ b/Sources/Subprocess/Execution.swift @@ -34,23 +34,11 @@ public struct Execution: Sendable { /// The process identifier of the current execution public let processIdentifier: ProcessIdentifier - #if os(Windows) - internal let consoleBehavior: PlatformOptions.ConsoleBehavior - - init( - processIdentifier: ProcessIdentifier, - consoleBehavior: PlatformOptions.ConsoleBehavior - ) { - self.processIdentifier = processIdentifier - self.consoleBehavior = consoleBehavior - } - #else init( processIdentifier: ProcessIdentifier ) { self.processIdentifier = processIdentifier } - #endif // os(Windows) } // MARK: - Output Capture diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index e0bd237..7fe6dcb 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -140,8 +140,7 @@ extension Configuration { threadHandle: processInfo.hThread ) let execution = Execution( - processIdentifier: pid, - consoleBehavior: self.platformOptions.consoleBehavior + processIdentifier: pid ) do { @@ -285,8 +284,7 @@ extension Configuration { threadHandle: processInfo.hThread ) let execution = Execution( - processIdentifier: pid, - consoleBehavior: self.platformOptions.consoleBehavior + processIdentifier: pid ) do { From fbc0f2c971d7bf0d749a94a0c293c8dd345acb49 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Mon, 28 Jul 2025 11:52:56 -0700 Subject: [PATCH 5/7] Add PlatformConformance in tests to ensure platform specifc types follow a uniform shape --- .../SubprocessTests/PlatformConformance.swift | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Tests/SubprocessTests/PlatformConformance.swift 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 {} From 89e5d2fa6e1b4f692eb63c18711bf5a92ffb5222 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Mon, 28 Jul 2025 14:22:19 -0700 Subject: [PATCH 6/7] Disable testPlatformOptionsRunAsUser until we can resolve CI filure issue --- Tests/SubprocessTests/SubprocessTests+Windows.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/SubprocessTests/SubprocessTests+Windows.swift b/Tests/SubprocessTests/SubprocessTests+Windows.swift index 080f1f1..b34fcce 100644 --- a/Tests/SubprocessTests/SubprocessTests+Windows.swift +++ b/Tests/SubprocessTests/SubprocessTests+Windows.swift @@ -450,7 +450,9 @@ extension SubprocessWindowsTests { // MARK: - PlatformOption Tests extension SubprocessWindowsTests { - @Test(.enabled(if: SubprocessWindowsTests.hasAdminPrivileges())) + // Disabled until we investigate CI specific failures + // https://github.com/swiftlang/swift-subprocess/issues/128 + @Test(.enabled(if: false)) func testPlatformOptionsRunAsUser() async throws { try await self.withTemporaryUser { username, password, succeed in // Use public directory as working directory so the newly created user From a2de33fa54cee58eaf01cc5d9029b58432ec7f45 Mon Sep 17 00:00:00 2001 From: Charles Hu Date: Tue, 29 Jul 2025 13:34:59 -0700 Subject: [PATCH 7/7] Prob whether waitid supports P_PIDFD instead of checking for Linux kernel version --- .../Platforms/Subprocess+Linux.swift | 60 +++++++++---------- .../_SubprocessCShims/include/process_shims.h | 3 +- Sources/_SubprocessCShims/process_shims.c | 43 +++++-------- .../SubprocessTests+Windows.swift | 7 ++- 4 files changed, 47 insertions(+), 66 deletions(-) diff --git a/Sources/Subprocess/Platforms/Subprocess+Linux.swift b/Sources/Subprocess/Platforms/Subprocess+Linux.swift index 799db20..6821d25 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Linux.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Linux.swift @@ -304,7 +304,7 @@ internal func monitorProcessTermination( // 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 _isLinuxKernelAtLeast(major: 5, minor: 4) { + if processIdentifier.processFileDescriptor > 0 { // Register processFileDescriptor with epoll var event = epoll_event( events: EPOLLIN.rawValue, @@ -418,8 +418,10 @@ private extension siginfo_t { } } -// Okay to be unlocked global mutable because this thread is only created once +// 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() { @@ -518,7 +520,7 @@ private func monitorThreadFunc(args: UnsafeMutableRawPointer?) -> UnsafeMutableR } // P_PIDFD requires Linux Kernel 5.4 and above - if _isLinuxKernelAtLeast(major: 5, minor: 4) { + if _waitProcessFileDescriptorSupported { _blockAndWaitForProcessFileDescriptor(targetFileDescriptor, context: context) } else { _reapAllKnownChildProcesses(targetFileDescriptor, context: context) @@ -569,9 +571,9 @@ private let setup: () = { return } - // On Linux kernel lower than 5.4 we have to rely on signal handler + // If the current kernel does not support pidfd, fallback to signal handler // Create the self-pipe that signal handler writes to - if !_isLinuxKernelAtLeast(major: 5, minor: 4) { + if !_isWaitProcessFileDescriptorSupported() { var pipeCreationError: SubprocessError? = nil do { let (readEnd, writeEnd) = try FileDescriptor.pipe() @@ -608,6 +610,9 @@ private let setup: () = { _reportFailureWithErrno(errno) return } + } else { + // Mark waitid(P_PIDFD) as supported + _waitProcessFileDescriptorSupported = true } let monitorThreadContext = MonitorThreadContext( epollFileDescriptor: epollFileDescriptor, @@ -755,36 +760,25 @@ private func _reapAllKnownChildProcesses(_ signalFd: CInt, context: MonitorThrea } } -private func _isLinuxKernelAtLeast(major: Int, minor: Int) -> Bool { - var buffer = utsname() - guard uname(&buffer) == 0 else { - return false - } - let releaseString = withUnsafeBytes(of: &buffer.release) { ptr in - let buffer = ptr.bindMemory(to: CChar.self) - return String(cString: buffer.baseAddress!) - } - - // utsname release follows the format - // major.minor.patch-extra - guard let mainVersion = releaseString.split(separator: "-").first else { - return false - } - - let versionComponents = mainVersion.split(separator: ".") - - guard let currentMajor = Int(versionComponents[0]), - let currentMinor = Int(versionComponents[1]) else { - return false - } - - if currentMajor > major { - return true - } else if currentMajor == major { - return currentMinor >= minor - } else { +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/_SubprocessCShims/include/process_shims.h b/Sources/_SubprocessCShims/include/process_shims.h index be209cb..64b7313 100644 --- a/Sources/_SubprocessCShims/include/process_shims.h +++ b/Sources/_SubprocessCShims/include/process_shims.h @@ -82,10 +82,11 @@ int _shims_snprintf( 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 dummy value if it's not available +// Define our value if it's not available #ifndef P_PIDFD #define P_PIDFD 3 #endif diff --git a/Sources/_SubprocessCShims/process_shims.c b/Sources/_SubprocessCShims/process_shims.c index e7ac818..867625c 100644 --- a/Sources/_SubprocessCShims/process_shims.c +++ b/Sources/_SubprocessCShims/process_shims.c @@ -311,7 +311,7 @@ int _subprocess_spawn( #define __GLIBC_PREREQ(maj, min) 0 #endif -static int _pidfd_open(pid_t pid) { +int _pidfd_open(pid_t pid) { return syscall(SYS_pidfd_open, pid, 0); } @@ -336,25 +336,6 @@ static int _clone3(int *pidfd) { return syscall(SYS_clone3, &args, sizeof(args)); } -static int _linux_kernel_version_at_least( - int required_major, - int required_minor -) { - struct utsname buf; - if (uname(&buf) != 0) { - return 0; // Cannot determine kernel version - } - - int major = 0, minor = 0; - if (sscanf(buf.release, "%d.%d", &major, &minor) < 2) { - return 0; - } - - if (major > required_major) return 1; - if (major == required_major && minor >= required_minor) return 1; - return 0; -} - int _subprocess_fork_exec( pid_t * _Nonnull pid, int * _Nonnull pidfd, @@ -423,17 +404,21 @@ int _subprocess_fork_exec( // Finally, fork / clone int _pidfd = -1; - pid_t childPid = -1; - if (_linux_kernel_version_at_least(5, 3)) { - // One Linux 5.3 and above, use the new clone3 syscall - // So we can atomically retrieve pidfd - childPid = _clone3(&_pidfd); - } else { - // On older linux versions, fall back to fork() + // 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" - childPid = fork(); + childPid = fork(); #pragma GCC diagnostic pop + } else { + // Report all other errors + close(pipefd[0]); + close(pipefd[1]); + return errno; + } } if (childPid < 0) { @@ -557,7 +542,7 @@ int _subprocess_fork_exec( } else { // On Linux 5.3 and lower, we have to fetch pidfd separately // Newer Linux supports clone3 which returns pidfd directly - if (_linux_kernel_version_at_least(5, 3) == 0) { + if (_pidfd < 0) { _pidfd = _pidfd_open(childPid); if (_pidfd < 0) { return errno; diff --git a/Tests/SubprocessTests/SubprocessTests+Windows.swift b/Tests/SubprocessTests/SubprocessTests+Windows.swift index b34fcce..abbe4b8 100644 --- a/Tests/SubprocessTests/SubprocessTests+Windows.swift +++ b/Tests/SubprocessTests/SubprocessTests+Windows.swift @@ -450,9 +450,10 @@ extension SubprocessWindowsTests { // MARK: - PlatformOption Tests extension SubprocessWindowsTests { - // Disabled until we investigate CI specific failures - // https://github.com/swiftlang/swift-subprocess/issues/128 - @Test(.enabled(if: false)) + @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, succeed in // Use public directory as working directory so the newly created user