diff --git a/Sources/Subprocess/API.swift b/Sources/Subprocess/API.swift index d40c747..b7788d6 100644 --- a/Sources/Subprocess/API.swift +++ b/Sources/Subprocess/API.swift @@ -644,16 +644,17 @@ public func runDetached( output: FileDescriptor? = nil, error: FileDescriptor? = nil ) throws -> ProcessIdentifier { + let execution: Execution switch (input, output, error) { case (.none, .none, .none): let processInput = NoInput() let processOutput = DiscardedOutput() let processError = DiscardedOutput() - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.none, .none, .some(let errorFd)): let processInput = NoInput() let processOutput = DiscardedOutput() @@ -661,22 +662,22 @@ public func runDetached( fileDescriptor: errorFd, closeAfterSpawningProcess: false ) - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.none, .some(let outputFd), .none): let processInput = NoInput() let processOutput = FileDescriptorOutput( fileDescriptor: outputFd, closeAfterSpawningProcess: false ) let processError = DiscardedOutput() - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.none, .some(let outputFd), .some(let errorFd)): let processInput = NoInput() let processOutput = FileDescriptorOutput( @@ -687,11 +688,11 @@ public func runDetached( fileDescriptor: errorFd, closeAfterSpawningProcess: false ) - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.some(let inputFd), .none, .none): let processInput = FileDescriptorInput( fileDescriptor: inputFd, @@ -699,11 +700,11 @@ public func runDetached( ) let processOutput = DiscardedOutput() let processError = DiscardedOutput() - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.some(let inputFd), .none, .some(let errorFd)): let processInput = FileDescriptorInput( fileDescriptor: inputFd, closeAfterSpawningProcess: false @@ -713,11 +714,11 @@ public func runDetached( fileDescriptor: errorFd, closeAfterSpawningProcess: false ) - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.some(let inputFd), .some(let outputFd), .none): let processInput = FileDescriptorInput( fileDescriptor: inputFd, @@ -728,11 +729,11 @@ public func runDetached( closeAfterSpawningProcess: false ) let processError = DiscardedOutput() - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution case (.some(let inputFd), .some(let outputFd), .some(let errorFd)): let processInput = FileDescriptorInput( fileDescriptor: inputFd, @@ -746,11 +747,13 @@ public func runDetached( fileDescriptor: errorFd, closeAfterSpawningProcess: false ) - return try configuration.spawn( + execution = try configuration.spawn( withInput: try processInput.createPipe(), outputPipe: try processOutput.createPipe(), errorPipe: try processError.createPipe() - ).execution.processIdentifier + ).execution } + execution.release() + return execution.processIdentifier } diff --git a/Sources/Subprocess/Configuration.swift b/Sources/Subprocess/Configuration.swift index 2b30ce0..5396506 100644 --- a/Sources/Subprocess/Configuration.swift +++ b/Sources/Subprocess/Configuration.swift @@ -71,12 +71,11 @@ public struct Configuration: Sendable { outputPipe: output, errorPipe: error ) - let pid = spawnResults.execution.processIdentifier var spawnResultBox: SpawnResult?? = consume spawnResults var _spawnResult = spawnResultBox!.take()! - let processIdentifier = _spawnResult.execution.processIdentifier + let execution = _spawnResult.execution let result: Swift.Result do { @@ -89,9 +88,8 @@ public struct Configuration: Sendable { return try await body(_spawnResult.execution, inputIO, outputIO, errorIO) } onCleanup: { // Attempt to terminate the child process - await Execution.runTeardownSequence( - self.platformOptions.teardownSequence, - on: pid + await execution.runTeardownSequence( + self.platformOptions.teardownSequence ) }) } catch { @@ -103,7 +101,7 @@ 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(forProcessWithIdentifier: processIdentifier) + let terminationStatus = try await Subprocess.monitorProcessTermination(forExecution: _spawnResult.execution) return try ExecutionResult(terminationStatus: terminationStatus, value: result.get()) } diff --git a/Sources/Subprocess/Execution.swift b/Sources/Subprocess/Execution.swift index 0aa06a7..a21a170 100644 --- a/Sources/Subprocess/Execution.swift +++ b/Sources/Subprocess/Execution.swift @@ -35,13 +35,16 @@ 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 @@ -51,6 +54,17 @@ 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 a45e3e1..7cd32c7 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Darwin.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Darwin.swift @@ -485,18 +485,18 @@ extension String { // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forProcessWithIdentifier pid: ProcessIdentifier + forExecution execution: Execution ) async throws -> TerminationStatus { return try await withCheckedThrowingContinuation { continuation in let source = DispatchSource.makeProcessSource( - identifier: pid.value, + identifier: execution.processIdentifier.value, eventMask: [.exit], queue: .global() ) source.setEventHandler { source.cancel() var siginfo = siginfo_t() - let rc = waitid(P_PID, id_t(pid.value), &siginfo, WEXITED) + let rc = waitid(P_PID, id_t(execution.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 7d4880e..125ba46 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Linux.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Linux.swift @@ -265,7 +265,7 @@ extension String { // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forProcessWithIdentifier pid: ProcessIdentifier + forExecution execution: Execution ) async throws -> TerminationStatus { try await withCheckedThrowingContinuation { continuation in _childProcessContinuations.withLock { continuations in @@ -274,7 +274,7 @@ internal func monitorProcessTermination( // 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: pid.value) + 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. diff --git a/Sources/Subprocess/Platforms/Subprocess+Unix.swift b/Sources/Subprocess/Platforms/Subprocess+Unix.swift index 5159e0a..268d32e 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Unix.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Unix.swift @@ -117,18 +117,6 @@ extension Execution { public func send( signal: Signal, toProcessGroup shouldSendToProcessGroup: Bool = false - ) throws { - try Self.send( - signal: signal, - to: self.processIdentifier, - toProcessGroup: shouldSendToProcessGroup - ) - } - - internal static func send( - signal: Signal, - to processIdentifier: ProcessIdentifier, - toProcessGroup shouldSendToProcessGroup: Bool ) throws { let pid = shouldSendToProcessGroup ? -(processIdentifier.value) : processIdentifier.value guard kill(pid, signal.rawValue) == 0 else { diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index d65b2ea..70afa33 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -116,54 +116,33 @@ extension Configuration { } } } - // We don't need the handle objects, so close it right away - guard CloseHandle(processInfo.hThread) else { - let windowsError = GetLastError() - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) - throw SubprocessError( - code: .init(.spawnFailed), - underlyingError: .init(rawValue: windowsError) - ) - } - guard CloseHandle(processInfo.hProcess) else { - let windowsError = GetLastError() - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) - throw SubprocessError( - code: .init(.spawnFailed), - underlyingError: .init(rawValue: windowsError) - ) - } - - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: nil, - outputRead: nil, - outputWrite: outputWriteFileDescriptor, - errorRead: nil, - errorWrite: errorWriteFileDescriptor - ) let pid = ProcessIdentifier( value: processInfo.dwProcessId ) let execution = Execution( processIdentifier: pid, + processInformation: processInfo, consoleBehavior: self.platformOptions.consoleBehavior ) + + do { + // After spawn finishes, close all child side fds + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: nil, + outputRead: nil, + outputWrite: outputWriteFileDescriptor, + errorRead: nil, + errorWrite: errorWriteFileDescriptor + ) + } 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() + throw error + } + return SpawnResult( execution: execution, inputWriteEnd: inputWriteFileDescriptor?.createPlatformDiskIO(), @@ -259,55 +238,33 @@ extension Configuration { } } } - // We don't need the handle objects, so close it right away - guard CloseHandle(processInfo.hThread) else { - let windowsError = GetLastError() - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) - throw SubprocessError( - code: .init(.spawnFailed), - underlyingError: .init(rawValue: windowsError) - ) - } - guard CloseHandle(processInfo.hProcess) else { - let windowsError = GetLastError() - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) - throw SubprocessError( - code: .init(.spawnFailed), - underlyingError: .init(rawValue: windowsError) - ) - } - - // After spawn finishes, close all child side fds - try self.safelyCloseMultiple( - inputRead: inputReadFileDescriptor, - inputWrite: nil, - outputRead: nil, - outputWrite: outputWriteFileDescriptor, - errorRead: nil, - errorWrite: errorWriteFileDescriptor - ) let pid = ProcessIdentifier( value: processInfo.dwProcessId ) let execution = Execution( processIdentifier: pid, + processInformation: processInfo, consoleBehavior: self.platformOptions.consoleBehavior ) + + do { + // After spawn finishes, close all child side fds + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: nil, + outputRead: nil, + outputWrite: outputWriteFileDescriptor, + errorRead: nil, + errorWrite: errorWriteFileDescriptor + ) + } 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() + throw error + } + return SpawnResult( execution: execution, inputWriteEnd: inputWriteFileDescriptor?.createPlatformDiskIO(), @@ -464,7 +421,7 @@ extension PlatformOptions: CustomStringConvertible, CustomDebugStringConvertible // MARK: - Process Monitoring @Sendable internal func monitorProcessTermination( - forProcessWithIdentifier pid: ProcessIdentifier + forExecution execution: Execution ) async throws -> TerminationStatus { // Once the continuation resumes, it will need to unregister the wait, so // yield the wait handle back to the calling scope. @@ -473,15 +430,8 @@ internal func monitorProcessTermination( if let waitHandle { _ = UnregisterWait(waitHandle) } - } - guard - let processHandle = OpenProcess( - DWORD(PROCESS_QUERY_INFORMATION | SYNCHRONIZE), - false, - pid.value - ) - else { - return .exited(1) + + execution.release() } try? await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in @@ -500,7 +450,7 @@ internal func monitorProcessTermination( guard RegisterWaitForSingleObject( &waitHandle, - processHandle, + execution.processInformation.hProcess, callback, context, INFINITE, @@ -518,7 +468,7 @@ internal func monitorProcessTermination( } var status: DWORD = 0 - guard GetExitCodeProcess(processHandle, &status) else { + guard GetExitCodeProcess(execution.processInformation.hProcess, &status) else { // The child process terminated but we couldn't get its status back. // Assume generic failure. return .exited(1) @@ -536,30 +486,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 { - try Self.terminate(self.processIdentifier, withExitCode: exitCode) - } - - internal static func terminate( - _ processIdentifier: ProcessIdentifier, - withExitCode exitCode: DWORD - ) throws { - guard - let processHandle = OpenProcess( - // PROCESS_ALL_ACCESS - DWORD(STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0xFFFF), - false, - processIdentifier.value - ) - else { - throw SubprocessError( - code: .init(.failedToTerminate), - underlyingError: .init(rawValue: GetLastError()) - ) - } - defer { - CloseHandle(processHandle) - } - guard TerminateProcess(processHandle, exitCode) else { + guard TerminateProcess(processInformation.hProcess, exitCode) else { throw SubprocessError( code: .init(.failedToTerminate), underlyingError: .init(rawValue: GetLastError()) @@ -569,23 +496,6 @@ extension Execution { /// Suspend the current subprocess public func suspend() throws { - guard - let processHandle = OpenProcess( - // PROCESS_ALL_ACCESS - DWORD(STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0xFFFF), - false, - self.processIdentifier.value - ) - else { - throw SubprocessError( - code: .init(.failedToSuspend), - underlyingError: .init(rawValue: GetLastError()) - ) - } - defer { - CloseHandle(processHandle) - } - let NTSuspendProcess: (@convention(c) (HANDLE) -> LONG)? = unsafeBitCast( GetProcAddress( @@ -600,7 +510,7 @@ extension Execution { underlyingError: .init(rawValue: GetLastError()) ) } - guard NTSuspendProcess(processHandle) >= 0 else { + guard NTSuspendProcess(processInformation.hProcess) >= 0 else { throw SubprocessError( code: .init(.failedToSuspend), underlyingError: .init(rawValue: GetLastError()) @@ -610,23 +520,6 @@ extension Execution { /// Resume the current subprocess after suspension public func resume() throws { - guard - let processHandle = OpenProcess( - // PROCESS_ALL_ACCESS - DWORD(STANDARD_RIGHTS_REQUIRED | SYNCHRONIZE | 0xFFFF), - false, - self.processIdentifier.value - ) - else { - throw SubprocessError( - code: .init(.failedToResume), - underlyingError: .init(rawValue: GetLastError()) - ) - } - defer { - CloseHandle(processHandle) - } - let NTResumeProcess: (@convention(c) (HANDLE) -> LONG)? = unsafeBitCast( GetProcAddress( @@ -641,7 +534,7 @@ extension Execution { underlyingError: .init(rawValue: GetLastError()) ) } - guard NTResumeProcess(processHandle) >= 0 else { + guard NTResumeProcess(processInformation.hProcess) >= 0 else { throw SubprocessError( code: .init(.failedToResume), underlyingError: .init(rawValue: GetLastError()) diff --git a/Sources/Subprocess/Teardown.swift b/Sources/Subprocess/Teardown.swift index ec452d7..a4c0352 100644 --- a/Sources/Subprocess/Teardown.swift +++ b/Sources/Subprocess/Teardown.swift @@ -78,15 +78,8 @@ extension Execution { /// Teardown sequence always ends with a `.kill` signal /// - Parameter sequence: The steps to perform. public func teardown(using sequence: some Sequence & Sendable) async { - await Self.runTeardownSequence(sequence, on: self.processIdentifier) - } - - internal static func teardown( - using sequence: some Sequence & Sendable, - on processIdentifier: ProcessIdentifier - ) async { await withUncancelledTask { - await Self.runTeardownSequence(sequence, on: processIdentifier) + await runTeardownSequence(sequence) } } } @@ -98,25 +91,10 @@ internal enum TeardownStepCompletion { } extension Execution { - internal static func gracefulShutDown( - _ processIdentifier: ProcessIdentifier, + internal func gracefulShutDown( allowedDurationToNextStep duration: Duration ) async { #if os(Windows) - guard - let processHandle = OpenProcess( - DWORD(PROCESS_QUERY_INFORMATION | SYNCHRONIZE), - false, - processIdentifier.value - ) - else { - // Nothing more we can do - return - } - defer { - CloseHandle(processHandle) - } - // 1. Attempt to send WM_CLOSE to the main window if _subprocess_windows_send_vm_close( processIdentifier.value @@ -148,15 +126,13 @@ extension Execution { // Send SIGTERM try? self.send( signal: .terminate, - to: processIdentifier, toProcessGroup: false ) #endif } - internal static func runTeardownSequence( - _ sequence: some Sequence & Sendable, - on processIdentifier: ProcessIdentifier + internal func runTeardownSequence( + _ sequence: some Sequence & Sendable ) async { // First insert the `.kill` step let finalSequence = sequence + [TeardownStep(storage: .kill)] @@ -177,7 +153,6 @@ extension Execution { } } await self.gracefulShutDown( - processIdentifier, allowedDurationToNextStep: allowedDuration ) return await group.next()! @@ -195,18 +170,17 @@ extension Execution { return .processHasExited } } - try? self.send(signal: signal, to: processIdentifier, toProcessGroup: false) + try? self.send(signal: signal, toProcessGroup: false) return await group.next()! } #endif // !os(Windows) case .kill: #if os(Windows) try? self.terminate( - processIdentifier, withExitCode: 0 ) #else - try? self.send(signal: .kill, to: processIdentifier, toProcessGroup: false) + try? self.send(signal: .kill, toProcessGroup: false) #endif stepCompletion = .killedTheProcess }