diff --git a/Sources/Subprocess/Platforms/Subprocess+Windows.swift b/Sources/Subprocess/Platforms/Subprocess+Windows.swift index 85575c7..83f37e6 100644 --- a/Sources/Subprocess/Platforms/Subprocess+Windows.swift +++ b/Sources/Subprocess/Platforms/Subprocess+Windows.swift @@ -66,6 +66,17 @@ extension Configuration { let errorReadFileDescriptor: IODescriptor? = errorPipe.readFileDescriptor() let errorWriteFileDescriptor: IODescriptor? = errorPipe.writeFileDescriptor() + // Create the Job Object up front. It persists across all candidate path + // attempts, and is owned by this function until ownership transfers to + // the `ProcessIdentifier` on the success path. + let jobHandle = try Self.createJobObject() + var jobHandleOwned = true + defer { + if jobHandleOwned { + _ = CloseHandle(jobHandle) + } + } + // CreateProcessW supports using `lpApplicationName` as well as `lpCommandLine` to // specify executable path. However, only `lpCommandLine` supports PATH looking up, // whereas `lpApplicationName` does not. In general we should rely on `lpCommandLine`'s @@ -115,52 +126,77 @@ extension Configuration { var createProcessFlags = self.generateCreateProcessFlag() - let (created, processInfo, windowsError) = try await self.withStartupInfoEx( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) { startupInfo throws(SubprocessError) in - // Give calling process a chance to modify flag and startup info - if let configurator = self.platformOptions.preSpawnProcessConfigurator { - try configurator(&createProcessFlags, &startupInfo.pointer(to: \.StartupInfo)!.pointee) - } + let (created, processInfo, windowsError, userManagesResume): (Bool, PROCESS_INFORMATION, DWORD, Bool) + do { + (created, processInfo, windowsError, userManagesResume) = try await self.withStartupInfoEx( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) { startupInfo throws(SubprocessError) in + // Give calling process a chance to modify flag and startup info + if let configurator = self.platformOptions.preSpawnProcessConfigurator { + try configurator(&createProcessFlags, &startupInfo.pointer(to: \.StartupInfo)!.pointee) + } - // Spawn! - let spawnContext = SpawnContext( - startupInfo: startupInfo, - createProcessFlags: createProcessFlags - ) - return try await runOnBackgroundThread { () throws(SubprocessError) in - return try applicationName.withOptionalNTPathRepresentation { applicationNameW throws(SubprocessError) in - try commandAndArgs._withCString( - encodedAs: UTF16.self - ) { commandAndArgsW throws(SubprocessError) in - try environment._withCString( + // If the configurator set `CREATE_SUSPENDED`, the user has + // explicitly opted into managing the child's initial + // resume themselves. Otherwise, Subprocess resumes the + // child after assigning it to the Job Object. + let userManagesResume = (createProcessFlags & DWORD(CREATE_SUSPENDED)) != 0 + + // Subprocess assigns every spawned child to a Job Object + // before any user code runs in the child. `CREATE_SUSPENDED` + // makes the assignment atomic, so it is always set + // regardless of what the configurator did. + createProcessFlags |= DWORD(CREATE_SUSPENDED) + + // Spawn! + let spawnContext = SpawnContext( + startupInfo: startupInfo, + createProcessFlags: createProcessFlags + ) + return try await runOnBackgroundThread { () throws(SubprocessError) in + return try applicationName.withOptionalNTPathRepresentation { applicationNameW throws(SubprocessError) in + try commandAndArgs._withCString( encodedAs: UTF16.self - ) { environmentW throws(SubprocessError) in - try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW throws(SubprocessError) in - var processInfo = PROCESS_INFORMATION() - let result = CreateProcessW( - applicationNameW, - UnsafeMutablePointer(mutating: commandAndArgsW), - nil, // lpProcessAttributes - nil, // lpThreadAttributes - true, // bInheritHandles - spawnContext.createProcessFlags, - UnsafeMutableRawPointer(mutating: environmentW), - intendedWorkingDirW, - spawnContext.startupInfo.pointer(to: \.StartupInfo)!, - &processInfo - ) - return (result, processInfo, GetLastError()) + ) { commandAndArgsW throws(SubprocessError) in + try environment._withCString( + encodedAs: UTF16.self + ) { environmentW throws(SubprocessError) in + try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW throws(SubprocessError) in + var processInfo = PROCESS_INFORMATION() + let result = CreateProcessW( + applicationNameW, + UnsafeMutablePointer(mutating: commandAndArgsW), + nil, // lpProcessAttributes + nil, // lpThreadAttributes + true, // bInheritHandles + spawnContext.createProcessFlags, + UnsafeMutableRawPointer(mutating: environmentW), + intendedWorkingDirW, + spawnContext.startupInfo.pointer(to: \.StartupInfo)!, + &processInfo + ) + return (result, processInfo, GetLastError(), userManagesResume) + } } } } } } + } catch { + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) + throw error } guard created else { @@ -192,10 +228,32 @@ extension Configuration { ) } + do { + try Self.assignChildToJobObjectAndResume( + jobHandle: jobHandle, + processInfo: processInfo, + resumeThread: !userManagesResume + ) + } catch { + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) + throw error + } + + // Transfer ownership of the job handle to the `ProcessIdentifier`. + jobHandleOwned = false + let pid = ProcessIdentifier( value: processInfo.dwProcessId, processDescriptor: processInfo.hProcess, - threadHandle: processInfo.hThread + threadHandle: processInfo.hThread, + jobHandle: jobHandle ) do { @@ -260,6 +318,17 @@ extension Configuration { let errorReadFileDescriptor: IODescriptor? = _errorPipe.readFileDescriptor() let errorWriteFileDescriptor: IODescriptor? = _errorPipe.writeFileDescriptor() + // Create the Job Object up front. It persists across all candidate path + // attempts, and is owned by this function until ownership transfers to + // the `ProcessIdentifier` on the success path. + let jobHandle = try Self.createJobObject() + var jobHandleOwned = true + defer { + if jobHandleOwned { + _ = CloseHandle(jobHandle) + } + } + // CreateProcessW supports using `lpApplicationName` as well as `lpCommandLine` to // specify executable path. However, only `lpCommandLine` supports PATH looking up, // whereas `lpApplicationName` does not. In general we should rely on `lpCommandLine`'s @@ -310,57 +379,72 @@ extension Configuration { var createProcessFlags = self.generateCreateProcessFlag() - let (created, processInfo, windowsError) = try await self.withStartupInfoEx( - inputRead: inputReadFileDescriptor, - inputWrite: inputWriteFileDescriptor, - outputRead: outputReadFileDescriptor, - outputWrite: outputWriteFileDescriptor, - errorRead: errorReadFileDescriptor, - errorWrite: errorWriteFileDescriptor - ) { startupInfo throws(SubprocessError) in - // Give calling process a chance to modify flag and startup info - if let configurator = self.platformOptions.preSpawnProcessConfigurator { - try configurator(&createProcessFlags, &startupInfo.pointer(to: \.StartupInfo)!.pointee) - } + let (created, processInfo, windowsError, userManagesResume): (Bool, PROCESS_INFORMATION, DWORD, Bool) + do { + (created, processInfo, windowsError, userManagesResume) = try await self.withStartupInfoEx( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) { startupInfo throws(SubprocessError) in + // Give calling process a chance to modify flag and startup info + if let configurator = self.platformOptions.preSpawnProcessConfigurator { + try configurator(&createProcessFlags, &startupInfo.pointer(to: \.StartupInfo)!.pointee) + } - let spawnContext = SpawnContext( - startupInfo: startupInfo, - createProcessFlags: createProcessFlags - ) - // Spawn (featuring pyramid!) - return try await runOnBackgroundThread { () throws(SubprocessError) in - return try userCredentials.username._withCString( - encodedAs: UTF16.self - ) { usernameW throws(SubprocessError) in - try userCredentials.password._withCString( + // If the configurator set `CREATE_SUSPENDED`, the user has + // explicitly opted into managing the child's initial + // resume themselves. Otherwise, Subprocess resumes the + // child after assigning it to the Job Object. + let userManagesResume = (createProcessFlags & DWORD(CREATE_SUSPENDED)) != 0 + + // Subprocess assigns every spawned child to a Job Object + // before any user code runs in the child. `CREATE_SUSPENDED` + // makes the assignment atomic, so it is always set + // regardless of what the configurator did. + createProcessFlags |= DWORD(CREATE_SUSPENDED) + + let spawnContext = SpawnContext( + startupInfo: startupInfo, + createProcessFlags: createProcessFlags + ) + // Spawn (featuring pyramid!) + return try await runOnBackgroundThread { () throws(SubprocessError) in + return try userCredentials.username._withCString( encodedAs: UTF16.self - ) { passwordW throws(SubprocessError) in - try userCredentials.domain.withOptionalCString( + ) { usernameW throws(SubprocessError) in + try userCredentials.password._withCString( encodedAs: UTF16.self - ) { domainW throws(SubprocessError) in - try applicationName.withOptionalNTPathRepresentation { applicationNameW throws(SubprocessError) in - try commandAndArgs._withCString( - encodedAs: UTF16.self - ) { commandAndArgsW throws(SubprocessError) in - try environment._withCString( + ) { passwordW throws(SubprocessError) in + try userCredentials.domain.withOptionalCString( + encodedAs: UTF16.self + ) { domainW throws(SubprocessError) in + try applicationName.withOptionalNTPathRepresentation { applicationNameW throws(SubprocessError) in + try commandAndArgs._withCString( encodedAs: UTF16.self - ) { environmentW throws(SubprocessError) in - try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW throws(SubprocessError) in - var processInfo = PROCESS_INFORMATION() - let created = CreateProcessWithLogonW( - usernameW, - domainW, - passwordW, - DWORD(LOGON_WITH_PROFILE), - applicationNameW, - UnsafeMutablePointer(mutating: commandAndArgsW), - spawnContext.createProcessFlags, - UnsafeMutableRawPointer(mutating: environmentW), - intendedWorkingDirW, - spawnContext.startupInfo.pointer(to: \.StartupInfo)!, - &processInfo - ) - return (created, processInfo, GetLastError()) + ) { commandAndArgsW throws(SubprocessError) in + try environment._withCString( + encodedAs: UTF16.self + ) { environmentW throws(SubprocessError) in + try intendedWorkingDir.withOptionalNTPathRepresentation { intendedWorkingDirW throws(SubprocessError) in + var processInfo = PROCESS_INFORMATION() + let created = CreateProcessWithLogonW( + usernameW, + domainW, + passwordW, + DWORD(LOGON_WITH_PROFILE), + applicationNameW, + UnsafeMutablePointer(mutating: commandAndArgsW), + spawnContext.createProcessFlags, + UnsafeMutableRawPointer(mutating: environmentW), + intendedWorkingDirW, + spawnContext.startupInfo.pointer(to: \.StartupInfo)!, + &processInfo + ) + return (created, processInfo, GetLastError(), userManagesResume) + } } } } @@ -369,6 +453,16 @@ extension Configuration { } } } + } catch { + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) + throw error } guard created else { @@ -399,10 +493,32 @@ extension Configuration { ) } + do { + try Self.assignChildToJobObjectAndResume( + jobHandle: jobHandle, + processInfo: processInfo, + resumeThread: !userManagesResume + ) + } catch { + try self.safelyCloseMultiple( + inputRead: inputReadFileDescriptor, + inputWrite: inputWriteFileDescriptor, + outputRead: outputReadFileDescriptor, + outputWrite: outputWriteFileDescriptor, + errorRead: errorReadFileDescriptor, + errorWrite: errorWriteFileDescriptor + ) + throw error + } + + // Transfer ownership of the job handle to the `ProcessIdentifier`. + jobHandleOwned = false + let pid = ProcessIdentifier( value: processInfo.dwProcessId, processDescriptor: processInfo.hProcess, - threadHandle: processInfo.hThread + threadHandle: processInfo.hThread, + jobHandle: jobHandle ) do { @@ -563,6 +679,24 @@ public struct PlatformOptions: Sendable { /// modification of the `dwCreationFlags` creation flag /// and startup info `STARTUPINFOW` before /// they are sent to `CreateProcessW()`. + /// + /// - Important: Subprocess assigns every spawned child to + /// an internal Job Object before any user code runs in the child, + /// so teardown sequences targeting the process group can terminate + /// the child and all of its descendants together. To make this + /// assignment atomic, Subprocess always spawns children with + /// `CREATE_SUSPENDED` set in `dwCreationFlags`, regardless of the + /// value the configurator leaves in `dwCreationFlags`. By default, + /// Subprocess resumes the child after the assignment completes. + /// If the configurator sets `CREATE_SUSPENDED` in `dwCreationFlags`, + /// Subprocess treats this as a signal that user code will resume + /// the child explicitly, and therefore skips the internal `ResumeThread` + /// call. In that case the caller is responsible for calling + /// `ResumeThread` on the thread handle exposed through + /// `Execution.processIdentifier.threadHandle`. Otherwise, the child + /// will remain suspended indefinitely. If user code separately + /// assigns the child to its own Job Object after spawn, that + /// user-supplied job is nested inside Subprocess's job. public var preSpawnProcessConfigurator: ( @Sendable ( @@ -663,13 +797,31 @@ internal func monitorProcessTermination( 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(SubprocessError) { - guard TerminateProcess(self.processIdentifier.processDescriptor, exitCode) else { - throw SubprocessError.processControlFailed( - .terminate, - underlyingError: SubprocessError.WindowsError(rawValue: GetLastError()) - ) + /// - Parameters: + /// - exitCode: The exit code to use for the subprocess. + /// - toProcessGroup: When `true`, terminates the subprocess and any + /// descendants by terminating the Job Object that contains them. The + /// exit code is propagated to every process in the job. When `false` + /// (the default), terminates only the immediate child process, leaving + /// descendants unaffected. + public func terminate( + withExitCode exitCode: DWORD, + toProcessGroup: Bool = false + ) throws(SubprocessError) { + if toProcessGroup { + guard TerminateJobObject(self.processIdentifier.jobHandle, exitCode) else { + throw SubprocessError.processControlFailed( + .terminate, + underlyingError: SubprocessError.WindowsError(rawValue: GetLastError()) + ) + } + } else { + guard TerminateProcess(self.processIdentifier.processDescriptor, exitCode) else { + throw SubprocessError.processControlFailed( + .terminate, + underlyingError: SubprocessError.WindowsError(rawValue: GetLastError()) + ) + } } } @@ -964,11 +1116,19 @@ public struct ProcessIdentifier: Sendable, Hashable { public nonisolated(unsafe) let processDescriptor: HANDLE /// The main thread handle for this execution. public nonisolated(unsafe) let threadHandle: HANDLE - - internal init(value: DWORD, processDescriptor: HANDLE, threadHandle: HANDLE) { + /// The Job Object handle that contains this process and any descendants. + internal nonisolated(unsafe) let jobHandle: HANDLE + + internal init( + value: DWORD, + processDescriptor: HANDLE, + threadHandle: HANDLE, + jobHandle: HANDLE + ) { self.value = value self.processDescriptor = processDescriptor self.threadHandle = threadHandle + self.jobHandle = jobHandle } internal func close() { @@ -978,6 +1138,9 @@ public struct ProcessIdentifier: Sendable, Hashable { guard CloseHandle(self.processDescriptor) else { fatalError("Failed to close process HANDLE: \(SubprocessError.WindowsError(rawValue: GetLastError()))") } + guard CloseHandle(self.jobHandle) else { + fatalError("Failed to close job HANDLE: \(SubprocessError.WindowsError(rawValue: GetLastError()))") + } } } @@ -1190,6 +1353,79 @@ extension Configuration { return try await body(&info) } + /// Creates a Job Object that will contain a spawned child process and + /// any descendants. + /// + /// - Important: The handle is created with default security attributes, + /// which makes it non-inheritable. Descendants must not receive a duplicate + /// of the handle, so the parent can retain exclusive control over the job's + /// lifetime. + private static func createJobObject() throws(SubprocessError) -> HANDLE { + guard let jobHandle = CreateJobObjectW(nil, nil), + jobHandle != INVALID_HANDLE_VALUE + else { + throw SubprocessError.spawnFailed( + withUnderlyingError: SubprocessError.WindowsError(rawValue: GetLastError()), + reason: "Failed to create Job Object" + ) + } + return jobHandle + } + + /// Assigns the child process to the Job Object and, if requested, resumes + /// the child process thread. + /// + /// When `resumeThread` is `false`, the caller is responsible for resuming + /// the child using `ResumeThread` on the thread handle exposed through + /// `Execution.processIdentifier.threadHandle`. The child remains + /// suspended until then. + private static func assignChildToJobObjectAndResume( + jobHandle: HANDLE, + processInfo: PROCESS_INFORMATION, + resumeThread: Bool + ) throws(SubprocessError) { + // `CreateProcessW` succeeded. The child is suspended. Assign it to + // the Job Object before resuming, so it cannot run any user code + // outside the job. + guard AssignProcessToJobObject(jobHandle, processInfo.hProcess) else { + let assignError = SubprocessError.WindowsError(rawValue: GetLastError()) + _ = TerminateProcess(processInfo.hProcess, 1) + _ = CloseHandle(processInfo.hThread) + _ = CloseHandle(processInfo.hProcess) + + // Detect the most common cause of assignment failure: the parent + // process itself is in a non-nestable Job Object. + var isParentInJob: WindowsBool = false + var reason: String = "Failed to assign child process to Job Object" + if IsProcessInJob(GetCurrentProcess(), nil, &isParentInJob), + isParentInJob.boolValue + { + reason += " (likely because the parent process is in a Job Object that does not allow nesting)" + } + + throw SubprocessError.spawnFailed( + withUnderlyingError: assignError, + reason: reason + ) + } + + guard resumeThread else { + // The user opted into managing the resume themselves. + return + } + + guard ResumeThread(processInfo.hThread) != DWORD(bitPattern: -1) else { + let resumeError = SubprocessError.WindowsError(rawValue: GetLastError()) + _ = TerminateProcess(processInfo.hProcess, 1) + _ = CloseHandle(processInfo.hThread) + _ = CloseHandle(processInfo.hProcess) + throw SubprocessError.spawnFailed( + withUnderlyingError: resumeError, + reason: "Failed to resume child process thread after Job Object assignment" + ) + } + } + private func generateWindowsCommandAndArguments( withPossibleExecutablePath executablePath: String ) throws(SubprocessError) -> ( diff --git a/Sources/Subprocess/Teardown.swift b/Sources/Subprocess/Teardown.swift index 282a110..71d931e 100644 --- a/Sources/Subprocess/Teardown.swift +++ b/Sources/Subprocess/Teardown.swift @@ -220,7 +220,8 @@ extension Execution { case .kill: #if os(Windows) try? self.terminate( - withExitCode: 0 + withExitCode: 0, + toProcessGroup: killProcessGroup ) #else try? self.send(signal: .kill, toProcessGroup: killProcessGroup) diff --git a/Tests/SubprocessTests/ProcessMonitoringTests.swift b/Tests/SubprocessTests/ProcessMonitoringTests.swift index 74ee78d..98fbf70 100644 --- a/Tests/SubprocessTests/ProcessMonitoringTests.swift +++ b/Tests/SubprocessTests/ProcessMonitoringTests.swift @@ -208,7 +208,10 @@ extension SubprocessProcessMonitoringTests { #if os(Windows) let underlying = SubprocessError.WindowsError(rawValue: DWORD(ERROR_INVALID_PARAMETER)) let processIdentifier = ProcessIdentifier( - value: .max, processDescriptor: INVALID_HANDLE_VALUE, threadHandle: INVALID_HANDLE_VALUE + value: .max, + processDescriptor: INVALID_HANDLE_VALUE, + threadHandle: INVALID_HANDLE_VALUE, + jobHandle: INVALID_HANDLE_VALUE ) #elseif os(Linux) || os(Android) || os(FreeBSD) || os(OpenBSD) let underlying = Errno(rawValue: ECHILD) diff --git a/Tests/SubprocessTests/WindowsTests.swift b/Tests/SubprocessTests/WindowsTests.swift index 7abe744..831a671 100644 --- a/Tests/SubprocessTests/WindowsTests.swift +++ b/Tests/SubprocessTests/WindowsTests.swift @@ -279,7 +279,9 @@ extension SubprocessWindowsTests { #expect(stuckProcess.terminationStatus.isSuccess) } - /// Tests a use case for Windows platform handles by assigning the newly created process to a Job Object + /// Tests a use case for Windows platform handles by assigning the newly + /// created process to a user-supplied Job Object, nested inside + /// Subprocess's internal job. /// - see: https://devblogs.microsoft.com/oldnewthing/20131209-00/ @Test func testPlatformHandles() async throws { let hJob = CreateJobObjectW(nil, nil) @@ -288,26 +290,149 @@ extension SubprocessWindowsTests { info.BasicLimitInformation.LimitFlags = DWORD(JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE) #expect(SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &info, DWORD(MemoryLayout.size))) - var platformOptions = PlatformOptions() - platformOptions.preSpawnProcessConfigurator = { (createProcessFlags, startupInfo) in - createProcessFlags |= DWORD(CREATE_SUSPENDED) - } - let result = try await Subprocess.run( self.cmdExe, arguments: ["/c", "echo"], - platformOptions: platformOptions, input: .none, output: .discarded, error: .discarded ) { execution in + // Nest the child inside a user-supplied Job Object. Subprocess + // already assigned it to its internal job. This nests further. guard AssignProcessToJobObject(hJob, execution.processIdentifier.processDescriptor) else { throw SubprocessError.WindowsError(rawValue: GetLastError()) } - guard ResumeThread(execution.processIdentifier.threadHandle) != DWORD(bitPattern: -1) else { - throw SubprocessError.WindowsError(rawValue: GetLastError()) + } + #expect(result.terminationStatus.isSuccess) + } +} + +// MARK: - Job Object Termination Tests +extension SubprocessWindowsTests { + @Test func testTerminateToProcessGroupKillsJobMembers() async throws { + _ = try await Subprocess.run( + self.cmdExe, + arguments: ["/c", "ping -n 60 127.0.0.1 > nul"], + input: .none, + output: .sequence, + error: .discarded + ) { execution in + let grandchildPid = try #require( + await Self.waitForChildPid(of: execution.processIdentifier.value, named: "ping.exe"), + "ping.exe did not appear as a child of cmd.exe" + ) + let grandchildHandle = try #require( + OpenProcess( + DWORD(SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION), + false, + grandchildPid + ), + "Failed to open handle to grandchild process" + ) + defer { _ = CloseHandle(grandchildHandle) } + + #expect(Self.isProcessAlive(handle: grandchildHandle)) + + // Terminate the immediate child and any job members. + try execution.terminate(withExitCode: 0, toProcessGroup: true) + + let exited = await Self.waitForProcessExit(handle: grandchildHandle) + #expect(exited, "Grandchild should have been terminated by TerminateJobObject") + + for try await _ in execution.standardOutput {} + } + } + + @Test func testTerminateWithoutProcessGroupLeavesJobMembers() async throws { + // The complement of `testTerminateToProcessGroupKillsJobMembers`. + // With `toProcessGroup: false`, `TerminateProcess` only affects the + // immediate child. Other job members, including grandchildren the + // child spawned, must survive and remain in the Job Object. + var grandchildHandleToCleanup: HANDLE? + defer { + if let handle = grandchildHandleToCleanup { + _ = TerminateProcess(handle, 0) + _ = CloseHandle(handle) } } + + _ = try await Subprocess.run( + self.cmdExe, + arguments: ["/c", "ping -n 60 127.0.0.1 > nul"], + input: .none, + output: .sequence, + error: .discarded + ) { execution in + let grandchildPid = try #require( + await Self.waitForChildPid(of: execution.processIdentifier.value, named: "ping.exe"), + "ping.exe did not appear as a child of cmd.exe" + ) + let grandchildHandle = try #require( + OpenProcess( + DWORD(SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_TERMINATE), + false, + grandchildPid + ), + "Failed to open handle to grandchild process" + ) + grandchildHandleToCleanup = grandchildHandle + + #expect(Self.isProcessAlive(handle: grandchildHandle)) + + // Terminate only the immediate child. + try execution.terminate(withExitCode: 0, toProcessGroup: false) + + // Give Windows a moment, then verify the grandchild is still alive + // and still a member of a Job Object. Membership confirms that + // Subprocess's internal job survived the per-process termination + // and continues to track descendants. + try await Task.sleep(for: .milliseconds(250)) + #expect( + Self.isProcessAlive(handle: grandchildHandle), + "Grandchild should remain alive after toProcessGroup: false termination" + ) + + var inJob: WindowsBool = false + #expect( + IsProcessInJob(grandchildHandle, nil, &inJob), + "IsProcessInJob failed" + ) + #expect( + inJob.boolValue, + "Grandchild should still be a job member after toProcessGroup: false termination" + ) + + for try await _ in execution.standardOutput {} + } + } + + @Test func testPreSpawnConfiguratorCanOptIntoManualResume() async throws { + // Setting `CREATE_SUSPENDED` from `preSpawnProcessConfigurator` + // signals that user code will resume the child explicitly. + // Subprocess itself must not call `ResumeThread` in that case. + // + // Verify this by calling `ResumeThread` from the body closure and + // check the previous suspend count it returns. `1` means the child + // was still suspended (correct), and `0` means Subprocess has already + // resumed it (incorrect). + var platformOptions = PlatformOptions() + platformOptions.preSpawnProcessConfigurator = { creationFlags, _ in + creationFlags |= DWORD(CREATE_SUSPENDED) + } + + let result = try await Subprocess.run( + self.cmdExe, + arguments: ["/c", "echo hello"], + platformOptions: platformOptions, + input: .none, + output: .discarded, + error: .discarded + ) { execution in + let previousSuspendCount = ResumeThread(execution.processIdentifier.threadHandle) + // Expect `1` since a greater suspend count means someone + // suspended the thread twice, and that should not happen. + #expect(previousSuspendCount == 1) + } #expect(result.terminationStatus.isSuccess) } } @@ -411,6 +536,215 @@ extension SubprocessWindowsTests { // `isAdmin` is a `WindowsBool` and we need `Bool` return isAdmin.boolValue } + + /// Finds the PID of a child of the given parent PID, optionally filtered + /// by executable name (case-insensitive), and returns the first match. + private static func findChildPid( + of parentPid: DWORD, + named name: String? = nil + ) throws -> DWORD? { + guard let snapshot = CreateToolhelp32Snapshot(DWORD(TH32CS_SNAPPROCESS), 0), + snapshot != INVALID_HANDLE_VALUE + else { + throw SubprocessError.WindowsError(rawValue: GetLastError()) + } + defer { _ = CloseHandle(snapshot) } + + var entry = PROCESSENTRY32W() + entry.dwSize = DWORD(MemoryLayout.size) + + guard Process32FirstW(snapshot, &entry) else { + return nil + } + repeat { + if entry.th32ParentProcessID == parentPid { + if let name = name { + let exeName = withUnsafePointer(to: entry.szExeFile) { + $0.withMemoryRebound( + to: WCHAR.self, + capacity: Int(MAX_PATH) + ) { ptr in + String(decodingCString: ptr, as: UTF16.self) + } + } + if exeName.lowercased() == name.lowercased() { + return entry.th32ProcessID + } + } else { + return entry.th32ProcessID + } + } + } while Process32NextW(snapshot, &entry) + + return nil + } + + /// Polls until a child of the given parent PID appears, or the timeout + /// elapses. + private static func waitForChildPid( + of parentPid: DWORD, + named name: String? = nil, + timeout: Duration = .seconds(5) + ) async throws -> DWORD? { + let clock = ContinuousClock() + let deadline = clock.now.advanced(by: timeout) + while clock.now < deadline { + if let pid = try findChildPid(of: parentPid, named: name) { + return pid + } + try? await Task.sleep(for: .milliseconds(50)) + } + return nil + } + + /// Returns `true` if `WaitForSingleObject` on the handle reports the + /// process as still running. + private static func isProcessAlive(handle: HANDLE) -> Bool { + return WaitForSingleObject(handle, 0) == WAIT_TIMEOUT + } + + /// Asynchronously waits for the process referenced by the given handle + /// to exit, or for the given timeout to elapse. Returns `true` if the + /// process exited within the timeout. + private static func waitForProcessExit( + handle: HANDLE, + timeout: Duration = .seconds(5) + ) async -> Bool { + let handleBits = Int(bitPattern: handle) + return await withTaskGroup(of: Bool.self, returning: Bool.self) { group in + group.addTask { + let handle = HANDLE(bitPattern: handleBits)! + await waitForHandleSignal(handle: handle) + return true + } + group.addTask { + try? await Task.sleep(for: timeout) + return false + } + // First child task to finish wins. Cancel the other. + // `waitForHandleSignal` honors task cancellation and unwinds + // promptly so the group can return. + let first = await group.next()! + group.cancelAll() + return first + } + } + + /// Asynchronously waits for the given handle to be signaled, using + /// `RegisterWaitForSingleObject`. Honors task cancellation by + /// unregistering the wait synchronously. + private static func waitForHandleSignal(handle: HANDLE) async { + // Shared mutable state between the registration scope, the C + // callback, and the cancellation handler. Access is serialized by + // a Win32 critical section since the callback runs on a thread-pool + // thread outside Swift Concurrency. + final class State: @unchecked Sendable { + var lock = SRWLOCK() + var waitHandle: HANDLE? + var continuation: CheckedContinuation? + var contextRetained: UnsafeMutableRawPointer? + + init() { + InitializeSRWLock(&lock) + } + + /// Resumes the continuation if it hasn't been resumed yet, and + /// releases the retained `Unmanaged` continuation context. + /// Returns `true` if this call performed the resume. + func resumeOnce() -> Bool { + AcquireSRWLockExclusive(&lock) + defer { ReleaseSRWLockExclusive(&lock) } + guard let continuation = self.continuation else { + return false + } + self.continuation = nil + if let context = self.contextRetained { + Unmanaged.fromOpaque(context).release() + self.contextRetained = nil + } + continuation.resume() + return true + } + } + + let state = State() + + await withTaskCancellationHandler { + await withCheckedContinuation { (continuation: CheckedContinuation) in + AcquireSRWLockExclusive(&state.lock) + state.continuation = continuation + ReleaseSRWLockExclusive(&state.lock) + + // Retain a reference to `state` for the C callback. The + // callback releases this retain after resuming. + let context = Unmanaged.passRetained(state).toOpaque() + AcquireSRWLockExclusive(&state.lock) + state.contextRetained = context + ReleaseSRWLockExclusive(&state.lock) + + let callback: WAITORTIMERCALLBACK = { context, _ in + let state = Unmanaged.fromOpaque(context!) + .takeRetainedValue() + // Clear `contextRetained` under lock since we just + // released our own retain with `takeRetainedValue()`. + AcquireSRWLockExclusive(&state.lock) + state.contextRetained = nil + let continuation = state.continuation + state.continuation = nil + ReleaseSRWLockExclusive(&state.lock) + continuation?.resume() + } + + var waitHandle: HANDLE? + let flags = ULONG(WT_EXECUTEONLYONCE | WT_EXECUTELONGFUNCTION) + guard + RegisterWaitForSingleObject( + &waitHandle, + handle, + callback, + context, + INFINITE, + flags + ) + else { + // Registration failed. Resume immediately. The caller's + // timeout path handles the apparent non-exit. + _ = state.resumeOnce() + return + } + + AcquireSRWLockExclusive(&state.lock) + state.waitHandle = waitHandle + ReleaseSRWLockExclusive(&state.lock) + } + + // Successful exit path. The callback fired and resumed us. + // The wait handle still needs unregistering. `INVALID_HANDLE_VALUE` + // tells `UnregisterWaitEx` to wait synchronously for any + // in-flight callbacks (there are none at this point, but it's + // the safe form). + AcquireSRWLockExclusive(&state.lock) + let waitHandle = state.waitHandle + state.waitHandle = nil + ReleaseSRWLockExclusive(&state.lock) + if let waitHandle { + _ = UnregisterWaitEx(waitHandle, INVALID_HANDLE_VALUE) + } + } onCancel: { + // Cancellation path. Unregister the wait synchronously + // (`INVALID_HANDLE_VALUE` blocks until any in-flight callback + // completes), then resume the continuation if it wasn't already + // resumed by the callback racing us. + AcquireSRWLockExclusive(&state.lock) + let waitHandle = state.waitHandle + state.waitHandle = nil + ReleaseSRWLockExclusive(&state.lock) + if let waitHandle { + _ = UnregisterWaitEx(waitHandle, INVALID_HANDLE_VALUE) + } + _ = state.resumeOnce() + } + } } extension FileDescriptor {