From c6b4c45392875bdb62bb2f6e9e806b7b33f140ca Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:52:28 -0700 Subject: [PATCH 01/26] test: reproduce remote stdin broken pipe abort --- .../RemoteSessionProcessRunnerTests.swift | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index fbcc0665e2ca..273a822ca247 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -70,6 +70,22 @@ struct RemoteSessionProcessRunnerTests { #expect(result.stdout == "hello-stdin") } + @Test("A child closing stdin before a large write does not abort the runner") + func childClosingStdinDoesNotAbortRunner() throws { + let runner = RemoteSessionProcessRunner() + let result = try runner.run( + RemoteProcessRequest( + executable: "/usr/bin/true", + arguments: [], + stdin: Data(repeating: 0x41, count: 1_048_576), + timeout: 5 + ), + operation: nil + ) + + #expect(result.status == 0) + } + @Test("Streams a local file through stdin") func streamsFileStdin() throws { let fileManager = FileManager.default From 2a63ff6d73f13b1e82f9e63d1cc489123398d505 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:53:57 -0700 Subject: [PATCH 02/26] fix: ignore remote stdin broken pipes --- .../Process/RemoteSessionProcessRunner.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 05b46faf9129..e95f4d0690c6 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -188,7 +188,9 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { defer { operation?.clearCancellationHandler() } if let stdin, let pipe = process.standardInput as? Pipe { - pipe.fileHandleForWriting.write(stdin) + // Child exit can close the read end before this best-effort write. + // The POSIX helper reports that race instead of delivering SIGPIPE. + try? pipe.fileHandleForWriting.writeProcessPipeInput(stdin) try? pipe.fileHandleForWriting.close() } From ef6c786aaf2c77ed59d493452a33944e8f08b49b Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:24:14 -0700 Subject: [PATCH 03/26] test: reject unexpected remote stdin errors --- .../RemoteSessionProcessRunnerTests.swift | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 273a822ca247..a2ad7b83d204 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -86,6 +86,32 @@ struct RemoteSessionProcessRunnerTests { #expect(result.status == 0) } + @Test("An unexpected stdin write error fails the runner") + func unexpectedStdinWriteErrorFailsRunner() { + let runner = RemoteSessionProcessRunner(writeStdin: { _, _ in + throw POSIXError(.EIO) + }) + + #expect { + try runner.run( + RemoteProcessRequest( + executable: "/bin/sleep", + arguments: ["30"], + stdin: Data("payload".utf8), + timeout: 5 + ), + operation: nil + ) + } throws: { error in + let nsError = error as NSError + let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? POSIXError + return nsError.domain == "cmux.remote.process" + && nsError.code == 3 + && nsError.localizedDescription.hasPrefix("Failed to write stdin for sleep:") + && underlyingError?.code == .EIO + } + } + @Test("Streams a local file through stdin") func streamsFileStdin() throws { let fileManager = FileManager.default From 462e80bf48dd6f803b652167acb03eddb9fda927 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:25:23 -0700 Subject: [PATCH 04/26] fix: propagate unexpected remote stdin errors --- .../Process/RemoteSessionProcessRunner.swift | 46 +++++++++++++++---- .../Process/RemoteSessionProcessRunning.swift | 4 +- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index e95f4d0690c6..517089dde8c0 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -20,6 +20,8 @@ internal import CMUXDebugLog /// synchronized by the capture `DispatchGroup` before any read-back, exactly /// like the legacy local-variable captures. public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { + typealias StdinWriter = @Sendable (FileHandle, Data) throws -> Void + /// Test observation seam (package tests only): invoked right after the /// stdout/stderr capture readers are installed, with the pipe read /// handles. Return `true` when the hook closes both handles, so the @@ -27,14 +29,24 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// The capture-survives-teardown regression test uses that to prove /// `run` still completes; production constructs the runner without a hook. let readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? + private let writeStdin: StdinWriter /// Creates the production runner. public init() { self.readHandlesDidInstall = nil + self.writeStdin = { handle, data in + try handle.writeProcessPipeInput(data) + } } - init(readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)?) { + init( + readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, + writeStdin: @escaping StdinWriter = { handle, data in + try handle.writeProcessPipeInput(data) + } + ) { self.readHandlesDidInstall = readHandlesDidInstall + self.writeStdin = writeStdin } // Mutable capture-state shared between the two background pipe readers @@ -187,13 +199,6 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } defer { operation?.clearCancellationHandler() } - if let stdin, let pipe = process.standardInput as? Pipe { - // Child exit can close the read end before this best-effort write. - // The POSIX helper reports that race instead of delivering SIGPIPE. - try? pipe.fileHandleForWriting.writeProcessPipeInput(stdin) - try? pipe.fileHandleForWriting.close() - } - func terminateProcessAndWait() { process.terminate() let terminatedGracefully = exitSemaphore.wait(timeout: .now() + 2.0) == .success @@ -203,6 +208,31 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } + if let stdin, let pipe = process.standardInput as? Pipe { + let inputHandle = pipe.fileHandleForWriting + do { + try writeStdin(inputHandle, stdin) + } catch let error as POSIXError where error.code == .EPIPE { + // A child may exit before consuming optional stdin. The POSIX + // helper turns that expected race into EPIPE instead of SIGPIPE. + } catch { + try? inputHandle.close() + if process.isRunning { + terminateProcessAndWait() + } + finishCaptureAndCloseReadHandles() + debugLog( + "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + + "error=\(error.localizedDescription)" + ) + throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ + NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(error.localizedDescription)", + NSUnderlyingErrorKey: error, + ]) + } + try? inputHandle.close() + } + let didExitBeforeTimeout = exitSemaphore.wait(timeout: .now() + max(0, timeout)) == .success if !didExitBeforeTimeout, process.isRunning { if let operation, operation.isCancelled { diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunning.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunning.swift index 327b0629e69d..b2897963091a 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunning.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunning.swift @@ -15,8 +15,8 @@ public protocol RemoteSessionProcessRunning: Sendable { /// - operation: Optional transfer-cancellation token; when it cancels, /// the process is terminated and `operation.cancellationError` is /// thrown. - /// - Throws: The legacy launch-failure (`cmux.remote.process` code 1) and - /// timeout (code 2) errors, or `operation.cancellationError`. + /// - Throws: Launch-failure (`cmux.remote.process` code 1), timeout + /// (code 2), stdin-write (code 3), or `operation.cancellationError`. func run( _ request: RemoteProcessRequest, operation: (any RemoteTransferCancelling)? From 5b2cd967e1d241f7c7cf9afeb5e072d2a40afa63 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:47:59 -0700 Subject: [PATCH 05/26] refactor: remove remote stdin test injection --- .../Process/RemoteSessionProcessRunner.swift | 44 +++++++++++-------- .../RemoteSessionProcessRunnerTests.swift | 22 ++++------ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 517089dde8c0..aef6b408812d 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -20,7 +20,25 @@ internal import CMUXDebugLog /// synchronized by the capture `DispatchGroup` before any read-back, exactly /// like the legacy local-variable captures. public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { - typealias StdinWriter = @Sendable (FileHandle, Data) throws -> Void + enum StdinWriter { + static func write( + _ data: Data, + to handle: FileHandle, + executableName: String + ) throws { + do { + try handle.writeProcessPipeInput(data) + } catch let error as POSIXError where error.code == .EPIPE { + // A child may exit before consuming optional stdin. The POSIX + // helper turns that expected race into EPIPE instead of SIGPIPE. + } catch { + throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ + NSLocalizedDescriptionKey: "Failed to write stdin for \(executableName): \(error.localizedDescription)", + NSUnderlyingErrorKey: error, + ]) + } + } + } /// Test observation seam (package tests only): invoked right after the /// stdout/stderr capture readers are installed, with the pipe read @@ -29,24 +47,16 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// The capture-survives-teardown regression test uses that to prove /// `run` still completes; production constructs the runner without a hook. let readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? - private let writeStdin: StdinWriter /// Creates the production runner. public init() { self.readHandlesDidInstall = nil - self.writeStdin = { handle, data in - try handle.writeProcessPipeInput(data) - } } init( - readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, - writeStdin: @escaping StdinWriter = { handle, data in - try handle.writeProcessPipeInput(data) - } + readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil ) { self.readHandlesDidInstall = readHandlesDidInstall - self.writeStdin = writeStdin } // Mutable capture-state shared between the two background pipe readers @@ -211,10 +221,11 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { if let stdin, let pipe = process.standardInput as? Pipe { let inputHandle = pipe.fileHandleForWriting do { - try writeStdin(inputHandle, stdin) - } catch let error as POSIXError where error.code == .EPIPE { - // A child may exit before consuming optional stdin. The POSIX - // helper turns that expected race into EPIPE instead of SIGPIPE. + try StdinWriter.write( + stdin, + to: inputHandle, + executableName: URL(fileURLWithPath: executable).lastPathComponent + ) } catch { try? inputHandle.close() if process.isRunning { @@ -225,10 +236,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + "error=\(error.localizedDescription)" ) - throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ - NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(error.localizedDescription)", - NSUnderlyingErrorKey: error, - ]) + throw error } try? inputHandle.close() } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index a2ad7b83d204..34fb87434ad0 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -86,21 +86,15 @@ struct RemoteSessionProcessRunnerTests { #expect(result.status == 0) } - @Test("An unexpected stdin write error fails the runner") - func unexpectedStdinWriteErrorFailsRunner() { - let runner = RemoteSessionProcessRunner(writeStdin: { _, _ in - throw POSIXError(.EIO) - }) + @Test("An unexpected stdin write error keeps the pinned runner error") + func unexpectedStdinWriteErrorKeepsPinnedRunnerError() { + let invalidHandle = FileHandle(fileDescriptor: -1, closeOnDealloc: false) #expect { - try runner.run( - RemoteProcessRequest( - executable: "/bin/sleep", - arguments: ["30"], - stdin: Data("payload".utf8), - timeout: 5 - ), - operation: nil + try RemoteSessionProcessRunner.StdinWriter.write( + Data("payload".utf8), + to: invalidHandle, + executableName: "sleep" ) } throws: { error in let nsError = error as NSError @@ -108,7 +102,7 @@ struct RemoteSessionProcessRunnerTests { return nsError.domain == "cmux.remote.process" && nsError.code == 3 && nsError.localizedDescription.hasPrefix("Failed to write stdin for sleep:") - && underlyingError?.code == .EIO + && underlyingError?.code == .EBADF } } From 8ff8d2e49c34d8a5022136fdec9464fdb77f5283 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:00:29 -0700 Subject: [PATCH 06/26] refactor: inject remote process stdin writer --- .../Process/RemoteProcessStdinWriting.swift | 18 +++++++ .../Process/RemoteSessionProcessRunner.swift | 51 +++++++------------ .../RemoteSessionProcessRunnerTests.swift | 31 ++++++++--- 3 files changed, 60 insertions(+), 40 deletions(-) create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift new file mode 100644 index 000000000000..98d01790d0b1 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift @@ -0,0 +1,18 @@ +internal import CmuxFoundation +internal import Foundation + +protocol RemoteProcessStdinWriting: Sendable { + func write( + _ data: Data, + to handle: FileHandle + ) throws +} + +struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { + func write( + _ data: Data, + to handle: FileHandle + ) throws { + try handle.writeProcessPipeInput(data) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index aef6b408812d..568763e40a56 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -14,32 +14,13 @@ internal import CMUXDebugLog /// and timeout NSError domain/codes/messages, the capture/close ordering, /// stdin handling, and the debug-log lines are all pinned behavior. /// -/// Isolation: the struct is stateless (one immutable test hook); each `run` -/// call owns its process-local state. The capture readers run on the global -/// utility pool and hand their results through a small queue-confined box, -/// synchronized by the capture `DispatchGroup` before any read-back, exactly -/// like the legacy local-variable captures. +/// Isolation: the struct owns immutable process-input behavior and one optional +/// read-handle observation hook; each `run` call owns its process-local state. +/// The capture readers run on the global utility pool and hand their results +/// through a small queue-confined box, synchronized by the capture +/// `DispatchGroup` before any read-back, exactly like the legacy local-variable +/// captures. public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { - enum StdinWriter { - static func write( - _ data: Data, - to handle: FileHandle, - executableName: String - ) throws { - do { - try handle.writeProcessPipeInput(data) - } catch let error as POSIXError where error.code == .EPIPE { - // A child may exit before consuming optional stdin. The POSIX - // helper turns that expected race into EPIPE instead of SIGPIPE. - } catch { - throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ - NSLocalizedDescriptionKey: "Failed to write stdin for \(executableName): \(error.localizedDescription)", - NSUnderlyingErrorKey: error, - ]) - } - } - } - /// Test observation seam (package tests only): invoked right after the /// stdout/stderr capture readers are installed, with the pipe read /// handles. Return `true` when the hook closes both handles, so the @@ -47,16 +28,20 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// The capture-survives-teardown regression test uses that to prove /// `run` still completes; production constructs the runner without a hook. let readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? + private let stdinWriter: any RemoteProcessStdinWriting /// Creates the production runner. public init() { self.readHandlesDidInstall = nil + self.stdinWriter = RemoteProcessStdinWriter() } init( - readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil + readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, + stdinWriter: any RemoteProcessStdinWriting = RemoteProcessStdinWriter() ) { self.readHandlesDidInstall = readHandlesDidInstall + self.stdinWriter = stdinWriter } // Mutable capture-state shared between the two background pipe readers @@ -221,11 +206,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { if let stdin, let pipe = process.standardInput as? Pipe { let inputHandle = pipe.fileHandleForWriting do { - try StdinWriter.write( - stdin, - to: inputHandle, - executableName: URL(fileURLWithPath: executable).lastPathComponent - ) + try stdinWriter.write(stdin, to: inputHandle) + } catch let error as POSIXError where error.code == .EPIPE { + // A child may exit before consuming optional stdin. The POSIX + // helper turns that expected race into EPIPE instead of SIGPIPE. } catch { try? inputHandle.close() if process.isRunning { @@ -236,7 +220,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + "error=\(error.localizedDescription)" ) - throw error + throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ + NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(error.localizedDescription)", + NSUnderlyingErrorKey: error, + ]) } try? inputHandle.close() } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 34fb87434ad0..c7c21053effb 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -17,6 +17,15 @@ import Testing extension RemoteSubprocessTests { @Suite("RemoteSessionProcessRunner") struct RemoteSessionProcessRunnerTests { + private struct FailingStdinWriter: RemoteProcessStdinWriting { + func write( + _ data: Data, + to handle: FileHandle + ) throws { + throw POSIXError(.EIO) + } + } + @Test("Capture survives the pipe read handles being torn down mid-run") func captureSurvivesPipeReadHandleTeardown() throws { let didCloseReadHandles = DispatchSemaphore(value: 0) @@ -86,15 +95,20 @@ struct RemoteSessionProcessRunnerTests { #expect(result.status == 0) } - @Test("An unexpected stdin write error keeps the pinned runner error") - func unexpectedStdinWriteErrorKeepsPinnedRunnerError() { - let invalidHandle = FileHandle(fileDescriptor: -1, closeOnDealloc: false) + @Test("An unexpected stdin write error terminates the child and keeps the pinned runner error") + func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() { + let runner = RemoteSessionProcessRunner(stdinWriter: FailingStdinWriter()) + let startedAt = ProcessInfo.processInfo.systemUptime #expect { - try RemoteSessionProcessRunner.StdinWriter.write( - Data("payload".utf8), - to: invalidHandle, - executableName: "sleep" + try runner.run( + RemoteProcessRequest( + executable: "/bin/sleep", + arguments: ["30"], + stdin: Data("payload".utf8), + timeout: 5 + ), + operation: nil ) } throws: { error in let nsError = error as NSError @@ -102,8 +116,9 @@ struct RemoteSessionProcessRunnerTests { return nsError.domain == "cmux.remote.process" && nsError.code == 3 && nsError.localizedDescription.hasPrefix("Failed to write stdin for sleep:") - && underlyingError?.code == .EBADF + && underlyingError?.code == .EIO } + #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } @Test("Streams a local file through stdin") From ab8bc391ae8bc5069d13dd9428ced0ec8901732b Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:21:55 -0700 Subject: [PATCH 07/26] refactor: split remote stdin writer types --- .../Process/RemoteProcessStdinWriter.swift | 11 +++++++++++ .../Process/RemoteProcessStdinWriting.swift | 9 --------- .../FailingRemoteProcessStdinWriter.swift | 11 +++++++++++ .../RemoteSessionProcessRunnerTests.swift | 11 +---------- 4 files changed, 23 insertions(+), 19 deletions(-) create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift new file mode 100644 index 000000000000..bc104585d47c --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift @@ -0,0 +1,11 @@ +internal import CmuxFoundation +internal import Foundation + +struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { + func write( + _ data: Data, + to handle: FileHandle + ) throws { + try handle.writeProcessPipeInput(data) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift index 98d01790d0b1..1bbb3b0d5438 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift @@ -7,12 +7,3 @@ protocol RemoteProcessStdinWriting: Sendable { to handle: FileHandle ) throws } - -struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { - func write( - _ data: Data, - to handle: FileHandle - ) throws { - try handle.writeProcessPipeInput(data) - } -} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift new file mode 100644 index 000000000000..1202761e4dc4 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift @@ -0,0 +1,11 @@ +import Foundation +@testable import CmuxRemoteSession + +struct FailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { + func write( + _ data: Data, + to handle: FileHandle + ) throws { + throw POSIXError(.EIO) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index c7c21053effb..73c4b8c87c88 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -17,15 +17,6 @@ import Testing extension RemoteSubprocessTests { @Suite("RemoteSessionProcessRunner") struct RemoteSessionProcessRunnerTests { - private struct FailingStdinWriter: RemoteProcessStdinWriting { - func write( - _ data: Data, - to handle: FileHandle - ) throws { - throw POSIXError(.EIO) - } - } - @Test("Capture survives the pipe read handles being torn down mid-run") func captureSurvivesPipeReadHandleTeardown() throws { let didCloseReadHandles = DispatchSemaphore(value: 0) @@ -97,7 +88,7 @@ struct RemoteSessionProcessRunnerTests { @Test("An unexpected stdin write error terminates the child and keeps the pinned runner error") func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() { - let runner = RemoteSessionProcessRunner(stdinWriter: FailingStdinWriter()) + let runner = RemoteSessionProcessRunner(stdinWriter: FailingRemoteProcessStdinWriter()) let startedAt = ProcessInfo.processInfo.systemUptime #expect { From 0e668b47944331fa9a1eda70ef6936319f456e2a Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:38:03 -0700 Subject: [PATCH 08/26] test: bound unread remote stdin --- .../RemoteSessionProcessRunnerTests.swift | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 73c4b8c87c88..ba0d90e56104 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -112,6 +112,30 @@ struct RemoteSessionProcessRunnerTests { #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } + @Test("A child retaining unread stdin cannot delay the process timeout") + func unreadStdinCannotDelayProcessTimeout() { + let runner = RemoteSessionProcessRunner() + let startedAt = ProcessInfo.processInfo.systemUptime + + #expect { + try runner.run( + RemoteProcessRequest( + executable: "/bin/sleep", + arguments: ["5"], + stdin: Data(repeating: 0x41, count: 1_048_576), + timeout: 1 + ), + operation: nil + ) + } throws: { error in + let nsError = error as NSError + return nsError.domain == "cmux.remote.process" + && nsError.code == 2 + && nsError.localizedDescription == "sleep timed out after 1s" + } + #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) + } + @Test("Streams a local file through stdin") func streamsFileStdin() throws { let fileManager = FileManager.default From 3c8f6ec40b36f00c443d0abf98be54ef6ead24a4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:38:47 -0700 Subject: [PATCH 09/26] fix: bound remote stdin delivery --- .../Process/RemoteSessionProcessRunner.swift | 103 ++++++++++++++---- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 568763e40a56..25cb144fec72 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -57,6 +57,35 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { var stderrReadError: (any Error)? } + private final class ProcessCompletionState: @unchecked Sendable { + struct Snapshot { + let didExit: Bool + let stdinWriteError: (any Error)? + } + + private let lock = NSLock() + private var didExit = false + private var stdinWriteError: (any Error)? + + func markExited() { + lock.withLock { + didExit = true + } + } + + func markStdinWriteFailed(_ error: any Error) { + lock.withLock { + stdinWriteError = error + } + } + + func snapshot() -> Snapshot { + lock.withLock { + Snapshot(didExit: didExit, stdinWriteError: stdinWriteError) + } + } + } + /// Runs the request to completion on the calling thread; see /// ``RemoteSessionProcessRunning/run(_:operation:)``. public func run( @@ -106,10 +135,14 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let stderrHandle = stderrPipe.fileHandleForReading let captureQueue = DispatchQueue(label: "cmux.remote.process.capture") let exitSemaphore = DispatchSemaphore(value: 0) + let lifecycleSemaphore = DispatchSemaphore(value: 0) + let completionState = ProcessCompletionState() let captureState = PipeCaptureState() let captureGroup = DispatchGroup() process.terminationHandler = { _ in + completionState.markExited() exitSemaphore.signal() + lifecycleSemaphore.signal() } // Duplicate the descriptors on the calling thread, while the handles // are guaranteed open, and drain the duplicates. The contract (pinned @@ -203,39 +236,66 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } + let stdinWriteGroup = DispatchGroup() if let stdin, let pipe = process.standardInput as? Pipe { let inputHandle = pipe.fileHandleForWriting - do { - try stdinWriter.write(stdin, to: inputHandle) - } catch let error as POSIXError where error.code == .EPIPE { - // A child may exit before consuming optional stdin. The POSIX - // helper turns that expected race into EPIPE instead of SIGPIPE. - } catch { - try? inputHandle.close() - if process.isRunning { - terminateProcessAndWait() + stdinWriteGroup.enter() + DispatchQueue.global(qos: .utility).async { + defer { + try? inputHandle.close() + stdinWriteGroup.leave() + lifecycleSemaphore.signal() } - finishCaptureAndCloseReadHandles() - debugLog( - "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + - "error=\(error.localizedDescription)" - ) - throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ - NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(error.localizedDescription)", - NSUnderlyingErrorKey: error, - ]) + do { + try stdinWriter.write(stdin, to: inputHandle) + } catch let error as POSIXError where error.code == .EPIPE { + // A child may exit before consuming optional stdin. The + // POSIX helper turns that race into EPIPE instead of SIGPIPE. + } catch { + completionState.markStdinWriteFailed(error) + } + } + } + + let timeoutDeadline = DispatchTime.now() + max(0, timeout) + var didTimeOut = false + while true { + let state = completionState.snapshot() + if state.didExit || state.stdinWriteError != nil { + break + } + if lifecycleSemaphore.wait(timeout: timeoutDeadline) == .timedOut { + let stateAtDeadline = completionState.snapshot() + didTimeOut = !stateAtDeadline.didExit && stateAtDeadline.stdinWriteError == nil + break } - try? inputHandle.close() } - let didExitBeforeTimeout = exitSemaphore.wait(timeout: .now() + max(0, timeout)) == .success - if !didExitBeforeTimeout, process.isRunning { + if let stdinWriteError = completionState.snapshot().stdinWriteError { + if process.isRunning { + terminateProcessAndWait() + } + stdinWriteGroup.wait() + finishCaptureAndCloseReadHandles() + debugLog( + "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + + "error=\(stdinWriteError.localizedDescription)" + ) + throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ + NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(stdinWriteError.localizedDescription)", + NSUnderlyingErrorKey: stdinWriteError, + ]) + } + + if didTimeOut, process.isRunning { if let operation, operation.isCancelled { terminateProcessAndWait() + stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() throw operation.cancellationError } terminateProcessAndWait() + stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() debugLog( "remote.proc.timeout exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + @@ -246,6 +306,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { ]) } + stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() let stdout = String(data: captureState.stdoutData, encoding: .utf8) ?? "" let stderr = String(data: captureState.stderrData, encoding: .utf8) ?? "" From aa1992eeb328d06af293e6cf3ee914f9f71dafc2 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:48:31 -0700 Subject: [PATCH 10/26] test: retain late remote stdin failures --- ...layedFailingRemoteProcessStdinWriter.swift | 12 ++++++++++ .../RemoteSessionProcessRunnerTests.swift | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift new file mode 100644 index 000000000000..4b761ae6012f --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift @@ -0,0 +1,12 @@ +import Foundation +@testable import CmuxRemoteSession + +struct DelayedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { + func write( + _ data: Data, + to handle: FileHandle + ) throws { + Thread.sleep(forTimeInterval: 0.1) + throw POSIXError(.EIO) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index ba0d90e56104..43fa9617a718 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -112,6 +112,29 @@ struct RemoteSessionProcessRunnerTests { #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } + @Test("A late stdin write error wins over an earlier child exit") + func lateStdinWriteErrorWinsOverChildExit() { + let runner = RemoteSessionProcessRunner(stdinWriter: DelayedFailingRemoteProcessStdinWriter()) + + #expect { + try runner.run( + RemoteProcessRequest( + executable: "/usr/bin/true", + arguments: [], + stdin: Data("payload".utf8), + timeout: 5 + ), + operation: nil + ) + } throws: { error in + let nsError = error as NSError + let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? POSIXError + return nsError.domain == "cmux.remote.process" + && nsError.code == 3 + && underlyingError?.code == .EIO + } + } + @Test("A child retaining unread stdin cannot delay the process timeout") func unreadStdinCannotDelayProcessTimeout() { let runner = RemoteSessionProcessRunner() From e0b644a8efecff5e08659975354dca995274726e Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:49:06 -0700 Subject: [PATCH 11/26] fix: retain late remote stdin failures --- .../Process/RemoteSessionProcessRunner.swift | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 25cb144fec72..ff365095991a 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -236,6 +236,17 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } + func stdinWriteFailure(_ error: any Error) -> NSError { + debugLog( + "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + + "error=\(error.localizedDescription)" + ) + return NSError(domain: "cmux.remote.process", code: 3, userInfo: [ + NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(error.localizedDescription)", + NSUnderlyingErrorKey: error, + ]) + } + let stdinWriteGroup = DispatchGroup() if let stdin, let pipe = process.standardInput as? Pipe { let inputHandle = pipe.fileHandleForWriting @@ -277,14 +288,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() - debugLog( - "remote.proc.stdinWriteFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + - "error=\(stdinWriteError.localizedDescription)" - ) - throw NSError(domain: "cmux.remote.process", code: 3, userInfo: [ - NSLocalizedDescriptionKey: "Failed to write stdin for \(URL(fileURLWithPath: executable).lastPathComponent): \(stdinWriteError.localizedDescription)", - NSUnderlyingErrorKey: stdinWriteError, - ]) + throw stdinWriteFailure(stdinWriteError) } if didTimeOut, process.isRunning { @@ -307,6 +311,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } stdinWriteGroup.wait() + if let stdinWriteError = completionState.snapshot().stdinWriteError { + finishCaptureAndCloseReadHandles() + throw stdinWriteFailure(stdinWriteError) + } finishCaptureAndCloseReadHandles() let stdout = String(data: captureState.stdoutData, encoding: .utf8) ?? "" let stderr = String(data: captureState.stderrData, encoding: .utf8) ?? "" From ca1105e1b15872f1dd4be0b2054c85fcb3987e6b Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:00:20 -0700 Subject: [PATCH 12/26] test: bound inherited remote stdin --- .../RemoteSessionProcessRunnerTests.swift | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 43fa9617a718..c02914dcb928 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -159,6 +159,30 @@ struct RemoteSessionProcessRunnerTests { #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } + @Test("A descendant retaining unread stdin cannot outlive the process timeout") + func inheritedUnreadStdinCannotOutliveProcessTimeout() { + let runner = RemoteSessionProcessRunner() + let startedAt = ProcessInfo.processInfo.systemUptime + + #expect { + try runner.run( + RemoteProcessRequest( + executable: "/bin/sh", + arguments: ["-c", "sleep 5 <&0 >/dev/null 2>&1 &"], + stdin: Data(repeating: 0x41, count: 1_048_576), + timeout: 1 + ), + operation: nil + ) + } throws: { error in + let nsError = error as NSError + return nsError.domain == "cmux.remote.process" + && nsError.code == 2 + && nsError.localizedDescription == "sh timed out after 1s" + } + #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) + } + @Test("Streams a local file through stdin") func streamsFileStdin() throws { let fileManager = FileManager.default From 0e0f1598ecd586533072630f6e12ea59e00af19f Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:02:27 -0700 Subject: [PATCH 13/26] fix: make remote stdin delivery deadline-aware --- .../Process/RemoteProcessStdinWriter.swift | 54 ++++++++++++++- .../Process/RemoteProcessStdinWriting.swift | 3 +- .../Process/RemoteSessionProcessRunner.swift | 65 ++++++++++++++++--- ...layedFailingRemoteProcessStdinWriter.swift | 3 +- .../FailingRemoteProcessStdinWriter.swift | 3 +- 5 files changed, 113 insertions(+), 15 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift index bc104585d47c..a5dc53f3de05 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift @@ -1,11 +1,59 @@ -internal import CmuxFoundation +internal import Darwin internal import Foundation struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { func write( _ data: Data, - to handle: FileHandle + to handle: FileHandle, + shouldStop: @escaping @Sendable () -> Bool ) throws { - try handle.writeProcessPipeInput(data) + let descriptor = handle.fileDescriptor + guard fcntl(descriptor, F_SETNOSIGPIPE, 1) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + let originalFlags = fcntl(descriptor, F_GETFL) + guard originalFlags >= 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + guard fcntl(descriptor, F_SETFL, originalFlags | O_NONBLOCK) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + defer { _ = fcntl(descriptor, F_SETFL, originalFlags) } + + try data.withUnsafeBytes { bytes in + guard let baseAddress = bytes.baseAddress else { return } + var offset = 0 + while offset < bytes.count { + if shouldStop() { return } + + let written = Darwin.write( + descriptor, + baseAddress.advanced(by: offset), + bytes.count - offset + ) + if written > 0 { + offset += written + continue + } + + let code = written == 0 ? EIO : errno + if code == EINTR { + continue + } + if code == EAGAIN || code == EWOULDBLOCK { + var readiness = pollfd( + fd: descriptor, + events: Int16(POLLOUT), + revents: 0 + ) + let pollResult = Darwin.poll(&readiness, 1, 25) + if pollResult >= 0 || errno == EINTR { + continue + } + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) + } + } } } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift index 1bbb3b0d5438..a6ee9edd7c7e 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift @@ -4,6 +4,7 @@ internal import Foundation protocol RemoteProcessStdinWriting: Sendable { func write( _ data: Data, - to handle: FileHandle + to handle: FileHandle, + shouldStop: @escaping @Sendable () -> Bool ) throws } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index ff365095991a..71b159e14eae 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -61,11 +61,18 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { struct Snapshot { let didExit: Bool let stdinWriteError: (any Error)? + let stdinWriteFinished: Bool } private let lock = NSLock() private var didExit = false private var stdinWriteError: (any Error)? + private var stdinWriteFinished: Bool + private var shouldStopStdinWrite = false + + init(stdinWriteFinished: Bool) { + self.stdinWriteFinished = stdinWriteFinished + } func markExited() { lock.withLock { @@ -79,9 +86,31 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } + func markStdinWriteFinished() { + lock.withLock { + stdinWriteFinished = true + } + } + + func requestStdinWriteStop() { + lock.withLock { + shouldStopStdinWrite = true + } + } + + func stdinWriteShouldStop() -> Bool { + lock.withLock { + shouldStopStdinWrite + } + } + func snapshot() -> Snapshot { lock.withLock { - Snapshot(didExit: didExit, stdinWriteError: stdinWriteError) + Snapshot( + didExit: didExit, + stdinWriteError: stdinWriteError, + stdinWriteFinished: stdinWriteFinished + ) } } } @@ -123,9 +152,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { process.standardOutput = stdoutPipe process.standardError = stderrPipe + let writesInlineStdin = stdinFileHandle == nil && stdin != nil if let stdinFileHandle { process.standardInput = stdinFileHandle - } else if stdin != nil { + } else if writesInlineStdin { process.standardInput = Pipe() } else { process.standardInput = FileHandle.nullDevice @@ -136,7 +166,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let captureQueue = DispatchQueue(label: "cmux.remote.process.capture") let exitSemaphore = DispatchSemaphore(value: 0) let lifecycleSemaphore = DispatchSemaphore(value: 0) - let completionState = ProcessCompletionState() + let completionState = ProcessCompletionState(stdinWriteFinished: !writesInlineStdin) let captureState = PipeCaptureState() let captureGroup = DispatchGroup() process.terminationHandler = { _ in @@ -221,6 +251,8 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() operation?.installCancellationHandler { + completionState.requestStdinWriteStop() + lifecycleSemaphore.signal() if process.isRunning { process.terminate() } @@ -254,11 +286,18 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { try? inputHandle.close() + completionState.markStdinWriteFinished() stdinWriteGroup.leave() lifecycleSemaphore.signal() } do { - try stdinWriter.write(stdin, to: inputHandle) + try stdinWriter.write( + stdin, + to: inputHandle, + shouldStop: { + completionState.stdinWriteShouldStop() + } + ) } catch let error as POSIXError where error.code == .EPIPE { // A child may exit before consuming optional stdin. The // POSIX helper turns that race into EPIPE instead of SIGPIPE. @@ -272,12 +311,16 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { var didTimeOut = false while true { let state = completionState.snapshot() - if state.didExit || state.stdinWriteError != nil { + if state.stdinWriteError != nil || (state.didExit && state.stdinWriteFinished) { break } if lifecycleSemaphore.wait(timeout: timeoutDeadline) == .timedOut { let stateAtDeadline = completionState.snapshot() - didTimeOut = !stateAtDeadline.didExit && stateAtDeadline.stdinWriteError == nil + didTimeOut = stateAtDeadline.stdinWriteError == nil + && !(stateAtDeadline.didExit && stateAtDeadline.stdinWriteFinished) + if didTimeOut { + completionState.requestStdinWriteStop() + } break } } @@ -291,14 +334,18 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { throw stdinWriteFailure(stdinWriteError) } - if didTimeOut, process.isRunning { + if didTimeOut { if let operation, operation.isCancelled { - terminateProcessAndWait() + if process.isRunning { + terminateProcessAndWait() + } stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() throw operation.cancellationError } - terminateProcessAndWait() + if process.isRunning { + terminateProcessAndWait() + } stdinWriteGroup.wait() finishCaptureAndCloseReadHandles() debugLog( diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift index 4b761ae6012f..3acca4905a1c 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift @@ -4,7 +4,8 @@ import Foundation struct DelayedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { func write( _ data: Data, - to handle: FileHandle + to handle: FileHandle, + shouldStop: @escaping @Sendable () -> Bool ) throws { Thread.sleep(forTimeInterval: 0.1) throw POSIXError(.EIO) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift index 1202761e4dc4..3da052219209 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift @@ -4,7 +4,8 @@ import Foundation struct FailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { func write( _ data: Data, - to handle: FileHandle + to handle: FileHandle, + shouldStop: @escaping @Sendable () -> Bool ) throws { throw POSIXError(.EIO) } From 40bd24ebe28437e885e260846b71824525f060fc Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:15:43 -0700 Subject: [PATCH 14/26] test: prove failed stdin writers reap children --- .../Process/RemoteSessionProcessRunner.swift | 7 ++++ .../RemoteSessionProcessRunnerTests.swift | 40 +++++++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 71b159e14eae..7f4ea3db13a1 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -28,19 +28,25 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// The capture-survives-teardown regression test uses that to prove /// `run` still completes; production constructs the runner without a hook. let readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? + /// Test observation seam (package tests only): records the launched child + /// identifier so tests can prove error cleanup reaped that exact process. + let processDidLaunch: (@Sendable (pid_t) -> Void)? private let stdinWriter: any RemoteProcessStdinWriting /// Creates the production runner. public init() { self.readHandlesDidInstall = nil + self.processDidLaunch = nil self.stdinWriter = RemoteProcessStdinWriter() } init( readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, + processDidLaunch: (@Sendable (pid_t) -> Void)? = nil, stdinWriter: any RemoteProcessStdinWriting = RemoteProcessStdinWriter() ) { self.readHandlesDidInstall = readHandlesDidInstall + self.processDidLaunch = processDidLaunch self.stdinWriter = stdinWriter } @@ -236,6 +242,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { do { try operation?.throwIfCancelled() try process.run() + processDidLaunch?(process.processIdentifier) } catch { try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index c02914dcb928..0666f7c193a4 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -1,3 +1,4 @@ +import Darwin import Foundation import Testing @testable import CmuxRemoteSession @@ -87,9 +88,12 @@ struct RemoteSessionProcessRunnerTests { } @Test("An unexpected stdin write error terminates the child and keeps the pinned runner error") - func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() { - let runner = RemoteSessionProcessRunner(stdinWriter: FailingRemoteProcessStdinWriter()) - let startedAt = ProcessInfo.processInfo.systemUptime + func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() throws { + let launchedProcess = LaunchedProcessRecorder() + let runner = RemoteSessionProcessRunner( + processDidLaunch: { launchedProcess.record($0) }, + stdinWriter: FailingRemoteProcessStdinWriter() + ) #expect { try runner.run( @@ -109,7 +113,8 @@ struct RemoteSessionProcessRunnerTests { && nsError.localizedDescription.hasPrefix("Failed to write stdin for sleep:") && underlyingError?.code == .EIO } - #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) + let processIdentifier = try #require(launchedProcess.processIdentifier) + #expect(waitForProcessExit(processIdentifier, timeout: 2)) } @Test("A late stdin write error wins over an earlier child exit") @@ -249,3 +254,30 @@ struct RemoteSessionProcessRunnerTests { } } } + +private final class LaunchedProcessRecorder: @unchecked Sendable { + private let lock = NSLock() + private var storedProcessIdentifier: pid_t? + + var processIdentifier: pid_t? { + lock.withLock { storedProcessIdentifier } + } + + func record(_ processIdentifier: pid_t) { + lock.withLock { + storedProcessIdentifier = processIdentifier + } + } +} + +private func waitForProcessExit(_ processIdentifier: pid_t, timeout: TimeInterval) -> Bool { + let deadline = DispatchTime.now() + timeout + repeat { + errno = 0 + if kill(processIdentifier, 0) == -1, errno == ESRCH { + return true + } + Thread.sleep(forTimeInterval: 0.01) + } while DispatchTime.now() < deadline + return false +} From fdaab0839eced562f3406e97b1b51b000a7e7510 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:18:46 -0700 Subject: [PATCH 15/26] test: bound inherited remote output pipes --- .../RemoteSessionProcessRunnerTests.swift | 71 +++++++++++++++---- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 0666f7c193a4..184a4ef53d41 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -165,27 +165,30 @@ struct RemoteSessionProcessRunnerTests { } @Test("A descendant retaining unread stdin cannot outlive the process timeout") - func inheritedUnreadStdinCannotOutliveProcessTimeout() { + func inheritedUnreadStdinCannotOutliveProcessTimeout() throws { let runner = RemoteSessionProcessRunner() - let startedAt = ProcessInfo.processInfo.systemUptime + let outcome = try #require(runProcess( + runner, + request: RemoteProcessRequest( + executable: "/bin/sh", + arguments: ["-c", "sleep 5 <&0 &"], + stdin: Data(repeating: 0x41, count: 1_048_576), + timeout: 1 + ), + completingWithin: 4 + )) - #expect { - try runner.run( - RemoteProcessRequest( - executable: "/bin/sh", - arguments: ["-c", "sleep 5 <&0 >/dev/null 2>&1 &"], - stdin: Data(repeating: 0x41, count: 1_048_576), - timeout: 1 - ), - operation: nil - ) - } throws: { error in + switch outcome { + case .success: + Issue.record("Expected the inherited stdin request to time out") + case .failure(let error): let nsError = error as NSError - return nsError.domain == "cmux.remote.process" + #expect( + nsError.domain == "cmux.remote.process" && nsError.code == 2 && nsError.localizedDescription == "sh timed out after 1s" + ) } - #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } @Test("Streams a local file through stdin") @@ -281,3 +284,41 @@ private func waitForProcessExit(_ processIdentifier: pid_t, timeout: TimeInterva } while DispatchTime.now() < deadline return false } + +private final class RemoteProcessRunRecorder: @unchecked Sendable { + private let lock = NSLock() + private var storedResult: Result? + + var result: Result? { + lock.withLock { storedResult } + } + + func record(_ result: Result) { + lock.withLock { + storedResult = result + } + } +} + +private func runProcess( + _ runner: RemoteSessionProcessRunner, + request: RemoteProcessRequest, + completingWithin timeout: TimeInterval +) -> Result? { + let recorder = RemoteProcessRunRecorder() + let completed = DispatchSemaphore(value: 0) + DispatchQueue.global(qos: .userInitiated).async { + recorder.record(Result { + try runner.run(request, operation: nil) + }) + completed.signal() + } + + guard completed.wait(timeout: .now() + timeout) == .success else { + // The fixture exits after five seconds, so reap the test-owned work + // before returning a failure and starting another subprocess test. + completed.wait() + return nil + } + return recorder.result +} From 3bd488108779b214f06f1cfb3c05fa8d5ab1e563 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:22:05 -0700 Subject: [PATCH 16/26] fix: bound inherited remote output capture --- .../ProcessPipe/ProcessPipeEndRead.swift | 82 +++++++++++++++++++ .../FileHandleProcessPipeReadingTests.swift | 18 ++++ .../Process/RemoteSessionProcessRunner.swift | 22 ++++- 3 files changed, 120 insertions(+), 2 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift index a1108085dadf..60a95f0b9c29 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift @@ -1,3 +1,4 @@ +import Darwin public import Foundation /// The outcome of draining a pipe to end-of-file: every byte read before the @@ -62,4 +63,85 @@ public struct ProcessPipeEndRead: Equatable, Sendable { } } } + + /// Drains a process pipe until EOF or until the owner requests a bounded + /// stop. While running, the reader polls so it can observe `shouldStop` + /// even when a descendant keeps the pipe's write end open. On stop it + /// preserves bytes already buffered in the pipe, then returns without + /// waiting for a future writer or an inherited descriptor to close. + public static func reading( + fileDescriptor: Int32, + chunkSize: Int = FileHandle.processPipeReadChunkSize, + pollIntervalMilliseconds: Int32 = 25, + shouldStop: @Sendable () -> Bool + ) -> ProcessPipeEndRead { + var data = Data() + var stopDrainCount = 0 + let maximumStopDrainCount = 16 + + while true { + let isStopping = shouldStop() + var descriptor = pollfd( + fd: fileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ) + let pollResult = Darwin.poll( + &descriptor, + 1, + isStopping ? 0 : max(1, pollIntervalMilliseconds) + ) + if pollResult == 0 { + if isStopping { + return ProcessPipeEndRead(data: data, readError: nil) + } + continue + } + if pollResult < 0 { + let code = errno + if code == EINTR { + continue + } + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.poll", + errnoCode: code + ) + ) + } + if (descriptor.revents & Int16(POLLNVAL)) != 0 { + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.poll", + errnoCode: EBADF + ) + ) + } + + switch ProcessPipeAvailableRead.readAvailableOnce( + fileDescriptor: fileDescriptor, + maxLength: chunkSize, + operation: "readDataToEndOfFile" + ) { + case .success(.data(let chunk)): + data.append(chunk) + if isStopping { + stopDrainCount += 1 + if stopDrainCount >= maximumStopDrainCount { + return ProcessPipeEndRead(data: data, readError: nil) + } + } + case .success(.wouldBlock): + if isStopping { + return ProcessPipeEndRead(data: data, readError: nil) + } + case .success(.endOfFile): + return ProcessPipeEndRead(data: data, readError: nil) + case .failure(let error): + return ProcessPipeEndRead(data: data, readError: error) + } + } + } } diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift index 5a8e4fc3e6ee..9bb985024dc3 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift @@ -64,6 +64,24 @@ struct FileHandleProcessPipeReadingTests { #expect(result.readError == readError) } + @Test("A stopped end read drains buffered bytes without waiting for EOF") + func stoppedEndReadDrainsBufferedBytesWithoutEOF() throws { + let pipe = Pipe() + defer { + try? pipe.fileHandleForWriting.close() + try? pipe.fileHandleForReading.close() + } + try pipe.fileHandleForWriting.write(contentsOf: Data("buffered".utf8)) + + let result = ProcessPipeEndRead.reading( + fileDescriptor: pipe.fileHandleForReading.fileDescriptor, + shouldStop: { true } + ) + + #expect(result.data == Data("buffered".utf8)) + #expect(result.readError == nil) + } + @Test("readDataToEndOfFileOrEmpty drains a closed pipe") func drainsClosedPipe() throws { let pipe = Pipe() diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 7f4ea3db13a1..1668a2e967b3 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -75,6 +75,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { private var stdinWriteError: (any Error)? private var stdinWriteFinished: Bool private var shouldStopStdinWrite = false + private var shouldStopCapture = false init(stdinWriteFinished: Bool) { self.stdinWriteFinished = stdinWriteFinished @@ -83,6 +84,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { func markExited() { lock.withLock { didExit = true + shouldStopCapture = true } } @@ -110,6 +112,12 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } + func captureShouldStop() -> Bool { + lock.withLock { + shouldStopCapture + } + } + func snapshot() -> Snapshot { lock.withLock { Snapshot( @@ -198,7 +206,12 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { captureGroup.leave() } defer { _ = Darwin.close(stdoutDescriptor) } - let result = ProcessPipeEndRead.reading(fileDescriptor: stdoutDescriptor) + let result = ProcessPipeEndRead.reading( + fileDescriptor: stdoutDescriptor, + shouldStop: { + completionState.captureShouldStop() + } + ) captureQueue.sync { captureState.stdoutData = result.data captureState.stdoutReadError = result.readError @@ -208,7 +221,12 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { captureGroup.leave() } defer { _ = Darwin.close(stderrDescriptor) } - let result = ProcessPipeEndRead.reading(fileDescriptor: stderrDescriptor) + let result = ProcessPipeEndRead.reading( + fileDescriptor: stderrDescriptor, + shouldStop: { + completionState.captureShouldStop() + } + ) captureQueue.sync { captureState.stderrData = result.data captureState.stderrReadError = result.readError From 134bd4f163380aa2e215de19b21110a8a960d4b3 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:04:49 -0700 Subject: [PATCH 17/26] fix: wake remote process capture on exit --- .../ProcessPipe/ProcessPipeEndRead.swift | 128 ++++++++++++------ .../FileHandleProcessPipeReadingTests.swift | 38 +++++- .../Process/RemoteSessionProcessRunner.swift | 45 +++--- ...layedFailingRemoteProcessStdinWriter.swift | 8 +- .../RemoteSessionProcessRunnerTests.swift | 8 +- 5 files changed, 164 insertions(+), 63 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift index 60a95f0b9c29..e6a9fbcd9af9 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift @@ -1,6 +1,51 @@ import Darwin public import Foundation +/// A one-shot signal that wakes process-pipe readers without periodic polling. +/// +/// Closing the pipe's write end leaves the read end permanently hung up, so +/// every reader polling the same signal wakes even when another reader returns +/// first. The owner must retain the signal until all readers have stopped. +public final class ProcessPipeStopSignal: @unchecked Sendable { + public let readFileDescriptor: Int32 + + private let lock = NSLock() + private var writeFileDescriptor: Int32 + + public init() throws { + var descriptors: [Int32] = [-1, -1] + guard Darwin.pipe(&descriptors) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + + readFileDescriptor = descriptors[0] + writeFileDescriptor = descriptors[1] + guard fcntl(readFileDescriptor, F_SETFD, FD_CLOEXEC) == 0, + fcntl(writeFileDescriptor, F_SETFD, FD_CLOEXEC) == 0 else { + let code = errno + Darwin.close(readFileDescriptor) + Darwin.close(writeFileDescriptor) + throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) + } + } + + public func signal() { + let descriptor = lock.withLock { + let descriptor = writeFileDescriptor + writeFileDescriptor = -1 + return descriptor + } + if descriptor >= 0 { + Darwin.close(descriptor) + } + } + + deinit { + signal() + Darwin.close(readFileDescriptor) + } +} + /// The outcome of draining a pipe to end-of-file: every byte read before the /// stream ended, plus the read error that interrupted the drain, if any. public struct ProcessPipeEndRead: Equatable, Sendable { @@ -64,39 +109,32 @@ public struct ProcessPipeEndRead: Equatable, Sendable { } } - /// Drains a process pipe until EOF or until the owner requests a bounded - /// stop. While running, the reader polls so it can observe `shouldStop` - /// even when a descendant keeps the pipe's write end open. On stop it - /// preserves bytes already buffered in the pipe, then returns without - /// waiting for a future writer or an inherited descriptor to close. + /// Drains a process pipe until EOF or until `stopFileDescriptor` becomes + /// readable or hung up. On stop it preserves bytes already buffered in the + /// pipe, then returns without waiting for a future writer or an inherited + /// descriptor to close. public static func reading( fileDescriptor: Int32, chunkSize: Int = FileHandle.processPipeReadChunkSize, - pollIntervalMilliseconds: Int32 = 25, - shouldStop: @Sendable () -> Bool + stopFileDescriptor: Int32 ) -> ProcessPipeEndRead { var data = Data() - var stopDrainCount = 0 let maximumStopDrainCount = 16 while true { - let isStopping = shouldStop() - var descriptor = pollfd( - fd: fileDescriptor, - events: Int16(POLLIN | POLLERR | POLLHUP), - revents: 0 - ) - let pollResult = Darwin.poll( - &descriptor, - 1, - isStopping ? 0 : max(1, pollIntervalMilliseconds) - ) - if pollResult == 0 { - if isStopping { - return ProcessPipeEndRead(data: data, readError: nil) - } - continue - } + var descriptors = [ + pollfd( + fd: fileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + pollfd( + fd: stopFileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + ] + let pollResult = Darwin.poll(&descriptors, nfds_t(descriptors.count), -1) if pollResult < 0 { let code = errno if code == EINTR { @@ -110,7 +148,7 @@ public struct ProcessPipeEndRead: Equatable, Sendable { ) ) } - if (descriptor.revents & Int16(POLLNVAL)) != 0 { + if (descriptors[0].revents & Int16(POLLNVAL)) != 0 { return ProcessPipeEndRead( data: data, readError: ProcessPipeReadError( @@ -120,27 +158,33 @@ public struct ProcessPipeEndRead: Equatable, Sendable { ) } - switch ProcessPipeAvailableRead.readAvailableOnce( - fileDescriptor: fileDescriptor, - maxLength: chunkSize, - operation: "readDataToEndOfFile" - ) { - case .success(.data(let chunk)): - data.append(chunk) - if isStopping { - stopDrainCount += 1 - if stopDrainCount >= maximumStopDrainCount { + let isStopping = descriptors[1].revents != 0 + var stopDrainCount = 0 + repeat { + switch ProcessPipeAvailableRead.readOnceIfReady( + fileDescriptor: fileDescriptor, + maxLength: chunkSize, + operation: "readDataToEndOfFile" + ) { + case .success(.data(let chunk)): + data.append(chunk) + if isStopping { + stopDrainCount += 1 + } + case .success(.wouldBlock): + if isStopping { return ProcessPipeEndRead(data: data, readError: nil) } - } - case .success(.wouldBlock): - if isStopping { + break + case .success(.endOfFile): return ProcessPipeEndRead(data: data, readError: nil) + case .failure(let error): + return ProcessPipeEndRead(data: data, readError: error) } - case .success(.endOfFile): + } while isStopping && stopDrainCount < maximumStopDrainCount + + if isStopping { return ProcessPipeEndRead(data: data, readError: nil) - case .failure(let error): - return ProcessPipeEndRead(data: data, readError: error) } } } diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift index 9bb985024dc3..40eabb9737e1 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift @@ -67,21 +67,57 @@ struct FileHandleProcessPipeReadingTests { @Test("A stopped end read drains buffered bytes without waiting for EOF") func stoppedEndReadDrainsBufferedBytesWithoutEOF() throws { let pipe = Pipe() + let stopSignal = try ProcessPipeStopSignal() defer { try? pipe.fileHandleForWriting.close() try? pipe.fileHandleForReading.close() } try pipe.fileHandleForWriting.write(contentsOf: Data("buffered".utf8)) + stopSignal.signal() let result = ProcessPipeEndRead.reading( fileDescriptor: pipe.fileHandleForReading.fileDescriptor, - shouldStop: { true } + stopFileDescriptor: stopSignal.readFileDescriptor ) #expect(result.data == Data("buffered".utf8)) #expect(result.readError == nil) } + @Test("One stop signal wakes every blocked pipe reader") + func stopSignalBroadcastsToBlockedReaders() throws { + let firstPipe = Pipe() + let secondPipe = Pipe() + let stopSignal = try ProcessPipeStopSignal() + defer { + try? firstPipe.fileHandleForWriting.close() + try? firstPipe.fileHandleForReading.close() + try? secondPipe.fileHandleForWriting.close() + try? secondPipe.fileHandleForReading.close() + } + + stopSignal.signal() + let firstFinished = DispatchSemaphore(value: 0) + let secondFinished = DispatchSemaphore(value: 0) + DispatchQueue.global(qos: .utility).async { + _ = ProcessPipeEndRead.reading( + fileDescriptor: firstPipe.fileHandleForReading.fileDescriptor, + stopFileDescriptor: stopSignal.readFileDescriptor + ) + firstFinished.signal() + } + DispatchQueue.global(qos: .utility).async { + _ = ProcessPipeEndRead.reading( + fileDescriptor: secondPipe.fileHandleForReading.fileDescriptor, + stopFileDescriptor: stopSignal.readFileDescriptor + ) + secondFinished.signal() + } + + #expect(firstFinished.wait(timeout: .now() + 1) == .success) + #expect(secondFinished.wait(timeout: .now() + 1) == .success) + } + @Test("readDataToEndOfFileOrEmpty drains a closed pipe") func drainsClosedPipe() throws { let pipe = Pipe() diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 1668a2e967b3..8bf7af415ef7 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -31,22 +31,28 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// Test observation seam (package tests only): records the launched child /// identifier so tests can prove error cleanup reaped that exact process. let processDidLaunch: (@Sendable (pid_t) -> Void)? + /// Test observation seam (package tests only): runs after the child has + /// definitively exited, including the deadline reconciliation path. + let processDidExit: (@Sendable () -> Void)? private let stdinWriter: any RemoteProcessStdinWriting /// Creates the production runner. public init() { self.readHandlesDidInstall = nil self.processDidLaunch = nil + self.processDidExit = nil self.stdinWriter = RemoteProcessStdinWriter() } init( readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, processDidLaunch: (@Sendable (pid_t) -> Void)? = nil, + processDidExit: (@Sendable () -> Void)? = nil, stdinWriter: any RemoteProcessStdinWriting = RemoteProcessStdinWriter() ) { self.readHandlesDidInstall = readHandlesDidInstall self.processDidLaunch = processDidLaunch + self.processDidExit = processDidExit self.stdinWriter = stdinWriter } @@ -75,16 +81,17 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { private var stdinWriteError: (any Error)? private var stdinWriteFinished: Bool private var shouldStopStdinWrite = false - private var shouldStopCapture = false init(stdinWriteFinished: Bool) { self.stdinWriteFinished = stdinWriteFinished } - func markExited() { + @discardableResult + func markExited() -> Bool { lock.withLock { + guard !didExit else { return false } didExit = true - shouldStopCapture = true + return true } } @@ -112,12 +119,6 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } } - func captureShouldStop() -> Bool { - lock.withLock { - shouldStopCapture - } - } - func snapshot() -> Snapshot { lock.withLock { Snapshot( @@ -180,11 +181,19 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let captureQueue = DispatchQueue(label: "cmux.remote.process.capture") let exitSemaphore = DispatchSemaphore(value: 0) let lifecycleSemaphore = DispatchSemaphore(value: 0) + let captureStopSignal = try ProcessPipeStopSignal() let completionState = ProcessCompletionState(stdinWriteFinished: !writesInlineStdin) let captureState = PipeCaptureState() let captureGroup = DispatchGroup() + let noteProcessExit: @Sendable () -> Void = { + let isFirstObservation = completionState.markExited() + captureStopSignal.signal() + if isFirstObservation { + processDidExit?() + } + } process.terminationHandler = { _ in - completionState.markExited() + noteProcessExit() exitSemaphore.signal() lifecycleSemaphore.signal() } @@ -208,9 +217,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { defer { _ = Darwin.close(stdoutDescriptor) } let result = ProcessPipeEndRead.reading( fileDescriptor: stdoutDescriptor, - shouldStop: { - completionState.captureShouldStop() - } + stopFileDescriptor: captureStopSignal.readFileDescriptor ) captureQueue.sync { captureState.stdoutData = result.data @@ -223,9 +230,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { defer { _ = Darwin.close(stderrDescriptor) } let result = ProcessPipeEndRead.reading( fileDescriptor: stderrDescriptor, - shouldStop: { - completionState.captureShouldStop() - } + stopFileDescriptor: captureStopSignal.readFileDescriptor ) captureQueue.sync { captureState.stderrData = result.data @@ -264,6 +269,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } catch { try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() + captureStopSignal.signal() finishCaptureAndCloseReadHandles() debugLog( "remote.proc.launchFailed exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + @@ -340,7 +346,12 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { break } if lifecycleSemaphore.wait(timeout: timeoutDeadline) == .timedOut { - let stateAtDeadline = completionState.snapshot() + var stateAtDeadline = completionState.snapshot() + if !stateAtDeadline.didExit, !process.isRunning { + process.waitUntilExit() + noteProcessExit() + stateAtDeadline = completionState.snapshot() + } didTimeOut = stateAtDeadline.stdinWriteError == nil && !(stateAtDeadline.didExit && stateAtDeadline.stdinWriteFinished) if didTimeOut { diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift index 3acca4905a1c..c99f0559d2a6 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift @@ -1,13 +1,17 @@ import Foundation @testable import CmuxRemoteSession -struct DelayedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { +struct ExitGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { + let processDidExit: DispatchSemaphore + func write( _ data: Data, to handle: FileHandle, shouldStop: @escaping @Sendable () -> Bool ) throws { - Thread.sleep(forTimeInterval: 0.1) + guard processDidExit.wait(timeout: .now() + 2) == .success else { + throw POSIXError(.ETIMEDOUT) + } throw POSIXError(.EIO) } } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 184a4ef53d41..8fff99ca2c44 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -119,7 +119,13 @@ struct RemoteSessionProcessRunnerTests { @Test("A late stdin write error wins over an earlier child exit") func lateStdinWriteErrorWinsOverChildExit() { - let runner = RemoteSessionProcessRunner(stdinWriter: DelayedFailingRemoteProcessStdinWriter()) + let processDidExit = DispatchSemaphore(value: 0) + let runner = RemoteSessionProcessRunner( + processDidExit: { processDidExit.signal() }, + stdinWriter: ExitGatedFailingRemoteProcessStdinWriter( + processDidExit: processDidExit + ) + ) #expect { try runner.run( From c9bb20bd9ee96475aa92312169ada8aac24a4591 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:51:15 -0700 Subject: [PATCH 18/26] fix: make remote process completion event driven --- .../ProcessPipe/ProcessPipeEndRead.swift | 90 +++++++-------- .../ProcessPipe/ProcessPipeStopSignal.swift | 56 +++++++++ .../FileHandleProcessPipeReadingTests.swift | 22 ++++ .../Process/RemoteProcessStdinWriter.swift | 40 +++++-- .../Process/RemoteProcessStdinWriting.swift | 2 +- .../Process/RemoteSessionProcessRunner.swift | 109 ++++++------------ ...layedFailingRemoteProcessStdinWriter.swift | 17 --- .../FailingRemoteProcessStdinWriter.swift | 12 -- ...GatedFailingRemoteProcessStdinWriter.swift | 51 ++++++++ .../RemoteSessionProcessRunnerTests.swift | 96 ++++++++------- 10 files changed, 285 insertions(+), 210 deletions(-) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift delete mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift delete mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift index e6a9fbcd9af9..42bd917dbe12 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift @@ -1,54 +1,16 @@ import Darwin public import Foundation -/// A one-shot signal that wakes process-pipe readers without periodic polling. -/// -/// Closing the pipe's write end leaves the read end permanently hung up, so -/// every reader polling the same signal wakes even when another reader returns -/// first. The owner must retain the signal until all readers have stopped. -public final class ProcessPipeStopSignal: @unchecked Sendable { - public let readFileDescriptor: Int32 - - private let lock = NSLock() - private var writeFileDescriptor: Int32 - - public init() throws { - var descriptors: [Int32] = [-1, -1] - guard Darwin.pipe(&descriptors) == 0 else { - throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) - } - - readFileDescriptor = descriptors[0] - writeFileDescriptor = descriptors[1] - guard fcntl(readFileDescriptor, F_SETFD, FD_CLOEXEC) == 0, - fcntl(writeFileDescriptor, F_SETFD, FD_CLOEXEC) == 0 else { - let code = errno - Darwin.close(readFileDescriptor) - Darwin.close(writeFileDescriptor) - throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) - } - } - - public func signal() { - let descriptor = lock.withLock { - let descriptor = writeFileDescriptor - writeFileDescriptor = -1 - return descriptor - } - if descriptor >= 0 { - Darwin.close(descriptor) - } - } - - deinit { - signal() - Darwin.close(readFileDescriptor) - } -} - /// The outcome of draining a pipe to end-of-file: every byte read before the /// stream ended, plus the read error that interrupted the drain, if any. public struct ProcessPipeEndRead: Equatable, Sendable { + // Darwin's FIONREAD macro is not imported into Swift because it expands + // through `_IOR`. This is `_IOR('f', 127, Int32)` from sys/filio.h. + private static let bytesAvailableIOControlRequest = + UInt(0x4000_0000) + | (UInt(MemoryLayout.size) << 16) + | UInt(0x66_7f) + /// The bytes successfully read before EOF or the failure. public let data: Data /// The error that ended the drain early, or `nil` on a clean EOF. @@ -119,7 +81,6 @@ public struct ProcessPipeEndRead: Equatable, Sendable { stopFileDescriptor: Int32 ) -> ProcessPipeEndRead { var data = Data() - let maximumStopDrainCount = 16 while true { var descriptors = [ @@ -159,17 +120,44 @@ public struct ProcessPipeEndRead: Equatable, Sendable { } let isStopping = descriptors[1].revents != 0 - var stopDrainCount = 0 + var bytesRemainingAtStop: Int? + if isStopping { + var availableBytes: Int32 = 0 + guard Darwin.ioctl( + fileDescriptor, + Self.bytesAvailableIOControlRequest, + &availableBytes + ) == 0 else { + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.ioctl", + errnoCode: errno + ) + ) + } + bytesRemainingAtStop = Int(availableBytes) + if availableBytes == 0 { + return ProcessPipeEndRead(data: data, readError: nil) + } + } + repeat { + let maximumReadLength = bytesRemainingAtStop.map { + min(chunkSize, $0) + } ?? chunkSize switch ProcessPipeAvailableRead.readOnceIfReady( fileDescriptor: fileDescriptor, - maxLength: chunkSize, + maxLength: maximumReadLength, operation: "readDataToEndOfFile" ) { case .success(.data(let chunk)): data.append(chunk) - if isStopping { - stopDrainCount += 1 + if let remaining = bytesRemainingAtStop.map({ $0 - chunk.count }) { + if remaining <= 0 { + return ProcessPipeEndRead(data: data, readError: nil) + } + bytesRemainingAtStop = remaining } case .success(.wouldBlock): if isStopping { @@ -181,7 +169,7 @@ public struct ProcessPipeEndRead: Equatable, Sendable { case .failure(let error): return ProcessPipeEndRead(data: data, readError: error) } - } while isStopping && stopDrainCount < maximumStopDrainCount + } while isStopping if isStopping { return ProcessPipeEndRead(data: data, readError: nil) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift new file mode 100644 index 000000000000..1dbe8a0cb471 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift @@ -0,0 +1,56 @@ +import Darwin +import Foundation +import os + +/// A one-shot, pollable signal for stopping process-pipe I/O. +/// +/// Closing the signal's write end leaves its read end permanently hung up, so +/// every reader polling the same descriptor wakes. Retain the signal until all +/// polling operations have returned; the read descriptor closes at deinit. +public final class ProcessPipeStopSignal: Sendable { + /// The stable descriptor callers include in their `poll(2)` descriptor set. + /// + /// The signal owns this descriptor. Callers must not close it and must stop + /// polling before releasing the signal. + public let readFileDescriptor: Int32 + + private let writeFileDescriptor: OSAllocatedUnfairLock + + /// Creates a close-on-exec pipe used exclusively as a stop signal. + /// + /// - Throws: A ``POSIXError`` when the pipe or close-on-exec flags cannot + /// be created. + public init() throws { + var descriptors: [Int32] = [-1, -1] + guard Darwin.pipe(&descriptors) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + + guard fcntl(descriptors[0], F_SETFD, FD_CLOEXEC) == 0, + fcntl(descriptors[1], F_SETFD, FD_CLOEXEC) == 0 else { + let code = errno + Darwin.close(descriptors[0]) + Darwin.close(descriptors[1]) + throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) + } + readFileDescriptor = descriptors[0] + writeFileDescriptor = OSAllocatedUnfairLock(initialState: descriptors[1]) + } + + /// Wakes every poller. Repeated calls are safe and have no effect. + public func signal() { + let descriptor = writeFileDescriptor.withLock { + let descriptor = $0 + $0 = -1 + return descriptor + } + if descriptor >= 0 { + Darwin.close(descriptor) + } + } + + deinit { + signal() + Darwin.close(readFileDescriptor) + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift index 40eabb9737e1..94f4bf12bb1b 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift @@ -84,6 +84,28 @@ struct FileHandleProcessPipeReadingTests { #expect(result.readError == nil) } + @Test("A stopped end read drains every byte present at the stop snapshot") + func stoppedEndReadDrainsSnapshotLargerThanSixteenChunks() throws { + let pipe = Pipe() + let stopSignal = try ProcessPipeStopSignal() + let payload = Data((0..<128).map { UInt8($0) }) + defer { + try? pipe.fileHandleForWriting.close() + try? pipe.fileHandleForReading.close() + } + try pipe.fileHandleForWriting.write(contentsOf: payload) + stopSignal.signal() + + let result = ProcessPipeEndRead.reading( + fileDescriptor: pipe.fileHandleForReading.fileDescriptor, + chunkSize: 1, + stopFileDescriptor: stopSignal.readFileDescriptor + ) + + #expect(result.data == payload) + #expect(result.readError == nil) + } + @Test("One stop signal wakes every blocked pipe reader") func stopSignalBroadcastsToBlockedReaders() throws { let firstPipe = Pipe() diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift index a5dc53f3de05..77cd43d980cc 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriter.swift @@ -5,7 +5,7 @@ struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { func write( _ data: Data, to handle: FileHandle, - shouldStop: @escaping @Sendable () -> Bool + stopFileDescriptor: Int32 ) throws { let descriptor = handle.fileDescriptor guard fcntl(descriptor, F_SETNOSIGPIPE, 1) == 0 else { @@ -24,7 +24,32 @@ struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { guard let baseAddress = bytes.baseAddress else { return } var offset = 0 while offset < bytes.count { - if shouldStop() { return } + var readiness = [ + pollfd( + fd: descriptor, + events: Int16(POLLOUT | POLLERR | POLLHUP), + revents: 0 + ), + pollfd( + fd: stopFileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + ] + let pollResult = Darwin.poll(&readiness, nfds_t(readiness.count), -1) + if pollResult < 0 { + let code = errno + if code == EINTR { + continue + } + throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) + } + if readiness[1].revents != 0 { + return + } + if (readiness[0].revents & Int16(POLLNVAL)) != 0 { + throw POSIXError(.EBADF) + } let written = Darwin.write( descriptor, @@ -41,16 +66,7 @@ struct RemoteProcessStdinWriter: RemoteProcessStdinWriting { continue } if code == EAGAIN || code == EWOULDBLOCK { - var readiness = pollfd( - fd: descriptor, - events: Int16(POLLOUT), - revents: 0 - ) - let pollResult = Darwin.poll(&readiness, 1, 25) - if pollResult >= 0 || errno == EINTR { - continue - } - throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + continue } throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift index a6ee9edd7c7e..67ce56146e98 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriting.swift @@ -5,6 +5,6 @@ protocol RemoteProcessStdinWriting: Sendable { func write( _ data: Data, to handle: FileHandle, - shouldStop: @escaping @Sendable () -> Bool + stopFileDescriptor: Int32 ) throws } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 8bf7af415ef7..f1f19f39c765 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -1,6 +1,7 @@ internal import CmuxFoundation internal import Darwin -public import Foundation +internal import Foundation +internal import os #if DEBUG internal import CMUXDebugLog #endif @@ -28,31 +29,19 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { /// The capture-survives-teardown regression test uses that to prove /// `run` still completes; production constructs the runner without a hook. let readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? - /// Test observation seam (package tests only): records the launched child - /// identifier so tests can prove error cleanup reaped that exact process. - let processDidLaunch: (@Sendable (pid_t) -> Void)? - /// Test observation seam (package tests only): runs after the child has - /// definitively exited, including the deadline reconciliation path. - let processDidExit: (@Sendable () -> Void)? private let stdinWriter: any RemoteProcessStdinWriting /// Creates the production runner. public init() { self.readHandlesDidInstall = nil - self.processDidLaunch = nil - self.processDidExit = nil self.stdinWriter = RemoteProcessStdinWriter() } init( readHandlesDidInstall: (@Sendable (FileHandle, FileHandle) -> Bool)? = nil, - processDidLaunch: (@Sendable (pid_t) -> Void)? = nil, - processDidExit: (@Sendable () -> Void)? = nil, stdinWriter: any RemoteProcessStdinWriting = RemoteProcessStdinWriter() ) { self.readHandlesDidInstall = readHandlesDidInstall - self.processDidLaunch = processDidLaunch - self.processDidExit = processDidExit self.stdinWriter = stdinWriter } @@ -69,64 +58,31 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { var stderrReadError: (any Error)? } - private final class ProcessCompletionState: @unchecked Sendable { + private struct ProcessCompletionState { struct Snapshot { let didExit: Bool let stdinWriteError: (any Error)? let stdinWriteFinished: Bool } - private let lock = NSLock() - private var didExit = false - private var stdinWriteError: (any Error)? - private var stdinWriteFinished: Bool - private var shouldStopStdinWrite = false + var didExit = false + var stdinWriteError: (any Error)? + var stdinWriteFinished: Bool init(stdinWriteFinished: Bool) { self.stdinWriteFinished = stdinWriteFinished } - @discardableResult - func markExited() -> Bool { - lock.withLock { - guard !didExit else { return false } - didExit = true - return true - } - } - - func markStdinWriteFailed(_ error: any Error) { - lock.withLock { - stdinWriteError = error - } - } - - func markStdinWriteFinished() { - lock.withLock { - stdinWriteFinished = true - } - } - - func requestStdinWriteStop() { - lock.withLock { - shouldStopStdinWrite = true - } + mutating func markExited() { + didExit = true } - func stdinWriteShouldStop() -> Bool { - lock.withLock { - shouldStopStdinWrite - } - } - - func snapshot() -> Snapshot { - lock.withLock { - Snapshot( - didExit: didExit, - stdinWriteError: stdinWriteError, - stdinWriteFinished: stdinWriteFinished - ) - } + var snapshot: Snapshot { + Snapshot( + didExit: didExit, + stdinWriteError: stdinWriteError, + stdinWriteFinished: stdinWriteFinished + ) } } @@ -182,15 +138,19 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let exitSemaphore = DispatchSemaphore(value: 0) let lifecycleSemaphore = DispatchSemaphore(value: 0) let captureStopSignal = try ProcessPipeStopSignal() - let completionState = ProcessCompletionState(stdinWriteFinished: !writesInlineStdin) + let stdinStopSignal = try ProcessPipeStopSignal() + // Process termination, the stdin writer, and the blocking caller use + // synchronous callbacks on different threads. This lock protects only + // their small completion snapshot; an actor would require unstructured + // tasks merely to bridge those non-async callbacks. + let completionState = OSAllocatedUnfairLock( + initialState: ProcessCompletionState(stdinWriteFinished: !writesInlineStdin) + ) let captureState = PipeCaptureState() let captureGroup = DispatchGroup() let noteProcessExit: @Sendable () -> Void = { - let isFirstObservation = completionState.markExited() + completionState.withLock { $0.markExited() } captureStopSignal.signal() - if isFirstObservation { - processDidExit?() - } } process.terminationHandler = { _ in noteProcessExit() @@ -265,7 +225,6 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { do { try operation?.throwIfCancelled() try process.run() - processDidLaunch?(process.processIdentifier) } catch { try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() @@ -282,7 +241,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() operation?.installCancellationHandler { - completionState.requestStdinWriteStop() + stdinStopSignal.signal() lifecycleSemaphore.signal() if process.isRunning { process.terminate() @@ -317,7 +276,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { try? inputHandle.close() - completionState.markStdinWriteFinished() + completionState.withLock { $0.stdinWriteFinished = true } stdinWriteGroup.leave() lifecycleSemaphore.signal() } @@ -325,15 +284,13 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { try stdinWriter.write( stdin, to: inputHandle, - shouldStop: { - completionState.stdinWriteShouldStop() - } + stopFileDescriptor: stdinStopSignal.readFileDescriptor ) } catch let error as POSIXError where error.code == .EPIPE { // A child may exit before consuming optional stdin. The // POSIX helper turns that race into EPIPE instead of SIGPIPE. } catch { - completionState.markStdinWriteFailed(error) + completionState.withLock { $0.stdinWriteError = error } } } } @@ -341,27 +298,27 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let timeoutDeadline = DispatchTime.now() + max(0, timeout) var didTimeOut = false while true { - let state = completionState.snapshot() + let state = completionState.withLock { $0.snapshot } if state.stdinWriteError != nil || (state.didExit && state.stdinWriteFinished) { break } if lifecycleSemaphore.wait(timeout: timeoutDeadline) == .timedOut { - var stateAtDeadline = completionState.snapshot() + var stateAtDeadline = completionState.withLock { $0.snapshot } if !stateAtDeadline.didExit, !process.isRunning { process.waitUntilExit() noteProcessExit() - stateAtDeadline = completionState.snapshot() + stateAtDeadline = completionState.withLock { $0.snapshot } } didTimeOut = stateAtDeadline.stdinWriteError == nil && !(stateAtDeadline.didExit && stateAtDeadline.stdinWriteFinished) if didTimeOut { - completionState.requestStdinWriteStop() + stdinStopSignal.signal() } break } } - if let stdinWriteError = completionState.snapshot().stdinWriteError { + if let stdinWriteError = completionState.withLock({ $0.snapshot }).stdinWriteError { if process.isRunning { terminateProcessAndWait() } @@ -394,7 +351,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } stdinWriteGroup.wait() - if let stdinWriteError = completionState.snapshot().stdinWriteError { + if let stdinWriteError = completionState.withLock({ $0.snapshot }).stdinWriteError { finishCaptureAndCloseReadHandles() throw stdinWriteFailure(stdinWriteError) } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift deleted file mode 100644 index c99f0559d2a6..000000000000 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/DelayedFailingRemoteProcessStdinWriter.swift +++ /dev/null @@ -1,17 +0,0 @@ -import Foundation -@testable import CmuxRemoteSession - -struct ExitGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { - let processDidExit: DispatchSemaphore - - func write( - _ data: Data, - to handle: FileHandle, - shouldStop: @escaping @Sendable () -> Bool - ) throws { - guard processDidExit.wait(timeout: .now() + 2) == .success else { - throw POSIXError(.ETIMEDOUT) - } - throw POSIXError(.EIO) - } -} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift deleted file mode 100644 index 3da052219209..000000000000 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/FailingRemoteProcessStdinWriter.swift +++ /dev/null @@ -1,12 +0,0 @@ -import Foundation -@testable import CmuxRemoteSession - -struct FailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { - func write( - _ data: Data, - to handle: FileHandle, - shouldStop: @escaping @Sendable () -> Bool - ) throws { - throw POSIXError(.EIO) - } -} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift new file mode 100644 index 000000000000..4d77924012a3 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift @@ -0,0 +1,51 @@ +import Darwin +import Foundation +@testable import CmuxRemoteSession + +struct MarkerGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { + enum Gate: Equatable, Sendable { + case launched + case exited + } + + let markerURL: URL + let gate: Gate + + func write( + _ data: Data, + to handle: FileHandle, + stopFileDescriptor: Int32 + ) throws { + let processIdentifier = try waitForProcessMarker() + if gate == .exited, !waitForProcessExit(processIdentifier) { + throw POSIXError(.ETIMEDOUT) + } + throw POSIXError(.EIO) + } + + private func waitForProcessMarker() throws -> pid_t { + let deadline = DispatchTime.now() + 2 + repeat { + if let contents = try? String(contentsOf: markerURL, encoding: .utf8), + let processIdentifier = pid_t( + contents.trimmingCharacters(in: .whitespacesAndNewlines) + ) { + return processIdentifier + } + Thread.sleep(forTimeInterval: 0.01) + } while DispatchTime.now() < deadline + throw POSIXError(.ETIMEDOUT) + } + + private func waitForProcessExit(_ processIdentifier: pid_t) -> Bool { + let deadline = DispatchTime.now() + 2 + repeat { + errno = 0 + if kill(processIdentifier, 0) == -1, errno == ESRCH { + return true + } + Thread.sleep(forTimeInterval: 0.01) + } while DispatchTime.now() < deadline + return false + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 8fff99ca2c44..499568f7240f 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -89,17 +89,24 @@ struct RemoteSessionProcessRunnerTests { @Test("An unexpected stdin write error terminates the child and keeps the pinned runner error") func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() throws { - let launchedProcess = LaunchedProcessRecorder() + let markerURL = processMarkerURL() + defer { try? FileManager.default.removeItem(at: markerURL) } let runner = RemoteSessionProcessRunner( - processDidLaunch: { launchedProcess.record($0) }, - stdinWriter: FailingRemoteProcessStdinWriter() + stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( + markerURL: markerURL, + gate: .launched + ) ) #expect { try runner.run( RemoteProcessRequest( - executable: "/bin/sleep", - arguments: ["30"], + executable: "/bin/sh", + arguments: [ + "-c", + "echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exec /bin/sleep 30", + ], + environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], stdin: Data("payload".utf8), timeout: 5 ), @@ -110,28 +117,33 @@ struct RemoteSessionProcessRunnerTests { let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? POSIXError return nsError.domain == "cmux.remote.process" && nsError.code == 3 - && nsError.localizedDescription.hasPrefix("Failed to write stdin for sleep:") + && nsError.localizedDescription.hasPrefix("Failed to write stdin for sh:") && underlyingError?.code == .EIO } - let processIdentifier = try #require(launchedProcess.processIdentifier) + let processIdentifier = try recordedProcessIdentifier(at: markerURL) #expect(waitForProcessExit(processIdentifier, timeout: 2)) } @Test("A late stdin write error wins over an earlier child exit") - func lateStdinWriteErrorWinsOverChildExit() { - let processDidExit = DispatchSemaphore(value: 0) + func lateStdinWriteErrorWinsOverChildExit() throws { + let markerURL = processMarkerURL() + defer { try? FileManager.default.removeItem(at: markerURL) } let runner = RemoteSessionProcessRunner( - processDidExit: { processDidExit.signal() }, - stdinWriter: ExitGatedFailingRemoteProcessStdinWriter( - processDidExit: processDidExit + stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( + markerURL: markerURL, + gate: .exited ) ) #expect { try runner.run( RemoteProcessRequest( - executable: "/usr/bin/true", - arguments: [], + executable: "/bin/sh", + arguments: [ + "-c", + "echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exit 0", + ], + environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], stdin: Data("payload".utf8), timeout: 5 ), @@ -147,27 +159,30 @@ struct RemoteSessionProcessRunnerTests { } @Test("A child retaining unread stdin cannot delay the process timeout") - func unreadStdinCannotDelayProcessTimeout() { + func unreadStdinCannotDelayProcessTimeout() throws { let runner = RemoteSessionProcessRunner() - let startedAt = ProcessInfo.processInfo.systemUptime + let outcome = try #require(runProcess( + runner, + request: RemoteProcessRequest( + executable: "/bin/sleep", + arguments: ["5"], + stdin: Data(repeating: 0x41, count: 1_048_576), + timeout: 1 + ), + completingWithin: 4 + )) - #expect { - try runner.run( - RemoteProcessRequest( - executable: "/bin/sleep", - arguments: ["5"], - stdin: Data(repeating: 0x41, count: 1_048_576), - timeout: 1 - ), - operation: nil - ) - } throws: { error in + switch outcome { + case .success: + Issue.record("Expected the unread stdin request to time out") + case .failure(let error): let nsError = error as NSError - return nsError.domain == "cmux.remote.process" + #expect( + nsError.domain == "cmux.remote.process" && nsError.code == 2 && nsError.localizedDescription == "sleep timed out after 1s" + ) } - #expect(ProcessInfo.processInfo.systemUptime - startedAt < 4) } @Test("A descendant retaining unread stdin cannot outlive the process timeout") @@ -264,19 +279,18 @@ struct RemoteSessionProcessRunnerTests { } } -private final class LaunchedProcessRecorder: @unchecked Sendable { - private let lock = NSLock() - private var storedProcessIdentifier: pid_t? - - var processIdentifier: pid_t? { - lock.withLock { storedProcessIdentifier } - } +private func processMarkerURL() -> URL { + FileManager.default.temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(UUID().uuidString).pid", + isDirectory: false + ) +} - func record(_ processIdentifier: pid_t) { - lock.withLock { - storedProcessIdentifier = processIdentifier - } - } +private func recordedProcessIdentifier(at markerURL: URL) throws -> pid_t { + let contents = try String(contentsOf: markerURL, encoding: .utf8) + return try #require( + pid_t(contents.trimmingCharacters(in: .whitespacesAndNewlines)) + ) } private func waitForProcessExit(_ processIdentifier: pid_t, timeout: TimeInterval) -> Bool { From 80b9a0869539443d5f339093aac2807563bc4e5c Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:18:29 -0700 Subject: [PATCH 19/26] fix: make remote completion lock free --- .../ProcessPipe/ProcessPipeStopSignal.swift | 16 +-- .../Process/RemoteProcessExitSignal.swift | 27 +++++ .../RemoteProcessPipeCaptureState.swift | 10 ++ .../RemoteProcessStdinWriteState.swift | 20 ++++ .../Process/RemoteSessionProcessRunner.swift | 110 ++++-------------- ...GatedFailingRemoteProcessStdinWriter.swift | 7 +- .../RemoteProcessRunRecorder.swift | 17 +++ .../RemoteProcessStdinWriterFailureGate.swift | 6 + .../RemoteSessionProcessRunnerTests.swift | 15 --- 9 files changed, 107 insertions(+), 121 deletions(-) create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessPipeCaptureState.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriteState.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessStdinWriterFailureGate.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift index 1dbe8a0cb471..eafb205b0840 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopSignal.swift @@ -1,6 +1,5 @@ import Darwin import Foundation -import os /// A one-shot, pollable signal for stopping process-pipe I/O. /// @@ -14,7 +13,8 @@ public final class ProcessPipeStopSignal: Sendable { /// polling before releasing the signal. public let readFileDescriptor: Int32 - private let writeFileDescriptor: OSAllocatedUnfairLock + private let writeFileDescriptor: Int32 + private let didSignal = AtomicBooleanGate(false) /// Creates a close-on-exec pipe used exclusively as a stop signal. /// @@ -34,19 +34,15 @@ public final class ProcessPipeStopSignal: Sendable { throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) } readFileDescriptor = descriptors[0] - writeFileDescriptor = OSAllocatedUnfairLock(initialState: descriptors[1]) + writeFileDescriptor = descriptors[1] } /// Wakes every poller. Repeated calls are safe and have no effect. public func signal() { - let descriptor = writeFileDescriptor.withLock { - let descriptor = $0 - $0 = -1 - return descriptor - } - if descriptor >= 0 { - Darwin.close(descriptor) + guard didSignal.compareExchange(expected: false, desired: true) else { + return } + Darwin.close(writeFileDescriptor) } deinit { diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift new file mode 100644 index 000000000000..0b4b8946e2fb --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift @@ -0,0 +1,27 @@ +internal import CmuxFoundation +internal import Foundation + +/// One-shot process-exit notification with deadline-aware blocking waits. +/// +/// Foundation can report `isRunning == false` just before its termination +/// callback runs. `recordExit()` is therefore idempotent so the blocking owner +/// may publish that observed exit without racing the callback. +final class RemoteProcessExitSignal: Sendable { + private let didExit = AtomicBooleanGate(false) + private let group = DispatchGroup() + + init() { + group.enter() + } + + func recordExit() { + guard didExit.compareExchange(expected: false, desired: true) else { + return + } + group.leave() + } + + func wait(until deadline: DispatchTime) -> Bool { + group.wait(timeout: deadline) == .success + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessPipeCaptureState.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessPipeCaptureState.swift new file mode 100644 index 000000000000..efe5ced08ab8 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessPipeCaptureState.swift @@ -0,0 +1,10 @@ +internal import Foundation + +/// Process-local capture storage whose mutations are confined to the runner's +/// serial capture queue and read only after its capture group completes. +final class RemoteProcessPipeCaptureState: @unchecked Sendable { + var stdoutData = Data() + var stderrData = Data() + var stdoutReadError: (any Error)? + var stderrReadError: (any Error)? +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriteState.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriteState.swift new file mode 100644 index 000000000000..3c4e82b081cb --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessStdinWriteState.swift @@ -0,0 +1,20 @@ +internal import CmuxFoundation + +/// Lock-free publication of the stdin writer's terminal error. +/// +/// The writer stores one immutable error before release-publishing the gate. +/// The blocking owner uses an acquire load before reading that value. +final class RemoteProcessStdinWriteState: @unchecked Sendable { + private let hasRecordedError = AtomicBooleanGate(false) + nonisolated(unsafe) private var error: (any Error)? + + func record(error: any Error) { + self.error = error + hasRecordedError.storeRelease(true) + } + + var recordedError: (any Error)? { + guard hasRecordedError.loadAcquire() else { return nil } + return error + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index f1f19f39c765..150bcde06cc5 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -1,7 +1,6 @@ internal import CmuxFoundation internal import Darwin internal import Foundation -internal import os #if DEBUG internal import CMUXDebugLog #endif @@ -45,47 +44,6 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { self.stdinWriter = stdinWriter } - // Mutable capture-state shared between the two background pipe readers - // and the blocking caller. Writes are confined to the serial - // `captureQueue`; the caller only reads after `captureGroup.wait()` - // ordered every writer before it. `@unchecked Sendable` because the - // compiler cannot see that confinement (the legacy code expressed the - // same contract with captured local `var`s). - private final class PipeCaptureState: @unchecked Sendable { - var stdoutData = Data() - var stderrData = Data() - var stdoutReadError: (any Error)? - var stderrReadError: (any Error)? - } - - private struct ProcessCompletionState { - struct Snapshot { - let didExit: Bool - let stdinWriteError: (any Error)? - let stdinWriteFinished: Bool - } - - var didExit = false - var stdinWriteError: (any Error)? - var stdinWriteFinished: Bool - - init(stdinWriteFinished: Bool) { - self.stdinWriteFinished = stdinWriteFinished - } - - mutating func markExited() { - didExit = true - } - - var snapshot: Snapshot { - Snapshot( - didExit: didExit, - stdinWriteError: stdinWriteError, - stdinWriteFinished: stdinWriteFinished - ) - } - } - /// Runs the request to completion on the calling thread; see /// ``RemoteSessionProcessRunning/run(_:operation:)``. public func run( @@ -135,27 +93,18 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let stdoutHandle = stdoutPipe.fileHandleForReading let stderrHandle = stderrPipe.fileHandleForReading let captureQueue = DispatchQueue(label: "cmux.remote.process.capture") - let exitSemaphore = DispatchSemaphore(value: 0) - let lifecycleSemaphore = DispatchSemaphore(value: 0) + let processExitSignal = RemoteProcessExitSignal() let captureStopSignal = try ProcessPipeStopSignal() let stdinStopSignal = try ProcessPipeStopSignal() - // Process termination, the stdin writer, and the blocking caller use - // synchronous callbacks on different threads. This lock protects only - // their small completion snapshot; an actor would require unstructured - // tasks merely to bridge those non-async callbacks. - let completionState = OSAllocatedUnfairLock( - initialState: ProcessCompletionState(stdinWriteFinished: !writesInlineStdin) - ) - let captureState = PipeCaptureState() + let stdinWriteState = RemoteProcessStdinWriteState() + let captureState = RemoteProcessPipeCaptureState() let captureGroup = DispatchGroup() let noteProcessExit: @Sendable () -> Void = { - completionState.withLock { $0.markExited() } captureStopSignal.signal() + processExitSignal.recordExit() } process.terminationHandler = { _ in noteProcessExit() - exitSemaphore.signal() - lifecycleSemaphore.signal() } // Duplicate the descriptors on the calling thread, while the handles // are guaranteed open, and drain the duplicates. The contract (pinned @@ -226,6 +175,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { try operation?.throwIfCancelled() try process.run() } catch { + processExitSignal.recordExit() try? stdoutPipe.fileHandleForWriting.close() try? stderrPipe.fileHandleForWriting.close() captureStopSignal.signal() @@ -242,7 +192,6 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { try? stderrPipe.fileHandleForWriting.close() operation?.installCancellationHandler { stdinStopSignal.signal() - lifecycleSemaphore.signal() if process.isRunning { process.terminate() } @@ -251,10 +200,11 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { func terminateProcessAndWait() { process.terminate() - let terminatedGracefully = exitSemaphore.wait(timeout: .now() + 2.0) == .success + let terminatedGracefully = processExitSignal.wait(until: .now() + 2.0) if !terminatedGracefully, process.isRunning { _ = Darwin.kill(process.processIdentifier, SIGKILL) process.waitUntilExit() + noteProcessExit() } } @@ -276,9 +226,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { try? inputHandle.close() - completionState.withLock { $0.stdinWriteFinished = true } stdinWriteGroup.leave() - lifecycleSemaphore.signal() } do { try stdinWriter.write( @@ -290,44 +238,27 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { // A child may exit before consuming optional stdin. The // POSIX helper turns that race into EPIPE instead of SIGPIPE. } catch { - completionState.withLock { $0.stdinWriteError = error } + stdinWriteState.record(error: error) + if process.isRunning { + process.terminate() + } } } } let timeoutDeadline = DispatchTime.now() + max(0, timeout) - var didTimeOut = false - while true { - let state = completionState.withLock { $0.snapshot } - if state.stdinWriteError != nil || (state.didExit && state.stdinWriteFinished) { - break - } - if lifecycleSemaphore.wait(timeout: timeoutDeadline) == .timedOut { - var stateAtDeadline = completionState.withLock { $0.snapshot } - if !stateAtDeadline.didExit, !process.isRunning { - process.waitUntilExit() - noteProcessExit() - stateAtDeadline = completionState.withLock { $0.snapshot } - } - didTimeOut = stateAtDeadline.stdinWriteError == nil - && !(stateAtDeadline.didExit && stateAtDeadline.stdinWriteFinished) - if didTimeOut { - stdinStopSignal.signal() - } - break - } + var didTimeOut = !processExitSignal.wait(until: timeoutDeadline) + if didTimeOut, !process.isRunning { + process.waitUntilExit() + noteProcessExit() + didTimeOut = false } - - if let stdinWriteError = completionState.withLock({ $0.snapshot }).stdinWriteError { - if process.isRunning { - terminateProcessAndWait() - } - stdinWriteGroup.wait() - finishCaptureAndCloseReadHandles() - throw stdinWriteFailure(stdinWriteError) + if !didTimeOut { + didTimeOut = stdinWriteGroup.wait(timeout: timeoutDeadline) == .timedOut } if didTimeOut { + stdinStopSignal.signal() if let operation, operation.isCancelled { if process.isRunning { terminateProcessAndWait() @@ -350,8 +281,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { ]) } - stdinWriteGroup.wait() - if let stdinWriteError = completionState.withLock({ $0.snapshot }).stdinWriteError { + if let stdinWriteError = stdinWriteState.recordedError { finishCaptureAndCloseReadHandles() throw stdinWriteFailure(stdinWriteError) } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift index 4d77924012a3..48a7e6c8eb75 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift @@ -3,13 +3,8 @@ import Foundation @testable import CmuxRemoteSession struct MarkerGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { - enum Gate: Equatable, Sendable { - case launched - case exited - } - let markerURL: URL - let gate: Gate + let gate: RemoteProcessStdinWriterFailureGate func write( _ data: Data, diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift new file mode 100644 index 000000000000..33b6bfc7578a --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift @@ -0,0 +1,17 @@ +import Foundation +@testable import CmuxRemoteSession + +final class RemoteProcessRunRecorder: @unchecked Sendable { + private let lock = NSLock() + private var storedResult: Result? + + var result: Result? { + lock.withLock { storedResult } + } + + func record(_ result: Result) { + lock.withLock { + storedResult = result + } + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessStdinWriterFailureGate.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessStdinWriterFailureGate.swift new file mode 100644 index 000000000000..b3962c0c561a --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessStdinWriterFailureGate.swift @@ -0,0 +1,6 @@ +import Foundation + +enum RemoteProcessStdinWriterFailureGate: Equatable, Sendable { + case launched + case exited +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 499568f7240f..ae14435d99b9 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -305,21 +305,6 @@ private func waitForProcessExit(_ processIdentifier: pid_t, timeout: TimeInterva return false } -private final class RemoteProcessRunRecorder: @unchecked Sendable { - private let lock = NSLock() - private var storedResult: Result? - - var result: Result? { - lock.withLock { storedResult } - } - - func record(_ result: Result) { - lock.withLock { - storedResult = result - } - } -} - private func runProcess( _ runner: RemoteSessionProcessRunner, request: RemoteProcessRequest, From ab34ffad8a4ca3ab0544d6a792dc02188b5f1afe Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:35:21 -0700 Subject: [PATCH 20/26] test: preserve stdin failure through forced kill --- .../RemoteSessionProcessRunnerTests.swift | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index ae14435d99b9..74a502251272 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -124,6 +124,43 @@ struct RemoteSessionProcessRunnerTests { #expect(waitForProcessExit(processIdentifier, timeout: 2)) } + @Test("A stdin write error wins when the child ignores termination") + func stdinWriteErrorWinsWhenChildIgnoresTermination() throws { + let markerURL = processMarkerURL() + defer { try? FileManager.default.removeItem(at: markerURL) } + let runner = RemoteSessionProcessRunner( + stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( + markerURL: markerURL, + gate: .launched + ) + ) + + let outcome = try #require(runProcess( + runner, + request: RemoteProcessRequest( + executable: "/bin/sh", + arguments: [ + "-c", + "trap '' TERM; echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exec /bin/sleep 30", + ], + environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], + stdin: Data("payload".utf8), + timeout: 10 + ), + completingWithin: 4 + )) + + guard case .failure(let error) = outcome else { + Issue.record("Expected the stdin write to fail") + return + } + let nsError = error as NSError + let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? POSIXError + #expect(nsError.domain == "cmux.remote.process") + #expect(nsError.code == 3) + #expect(underlyingError?.code == .EIO) + } + @Test("A late stdin write error wins over an earlier child exit") func lateStdinWriteErrorWinsOverChildExit() throws { let markerURL = processMarkerURL() From 5c12025f8d5e9c1912295a6b49f61a44900ff338 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:47:29 -0700 Subject: [PATCH 21/26] fix: preserve stdin failure through forced kill --- .../RemoteProcessCompletionWaitOutcome.swift | 6 ++ .../RemoteProcessCompletionWaiter.swift | 57 +++++++++++++++++++ .../Process/RemoteProcessExitSignal.swift | 9 ++- .../Process/RemoteSessionProcessRunner.swift | 40 ++++++++++++- 4 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaitOutcome.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaiter.swift diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaitOutcome.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaitOutcome.swift new file mode 100644 index 000000000000..b834723ed776 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaitOutcome.swift @@ -0,0 +1,6 @@ +enum RemoteProcessCompletionWaitOutcome: Equatable, Sendable { + case processExited + case stdinWriteFailed + case timedOut + case waitFailed(Int32) +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaiter.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaiter.swift new file mode 100644 index 000000000000..1f5d552d44cd --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessCompletionWaiter.swift @@ -0,0 +1,57 @@ +internal import Darwin +internal import Foundation + +struct RemoteProcessCompletionWaiter: Sendable { + let processExitFileDescriptor: Int32 + let stdinFailureFileDescriptor: Int32 + + func wait( + until deadline: DispatchTime + ) -> RemoteProcessCompletionWaitOutcome { + while true { + let now = DispatchTime.now().uptimeNanoseconds + guard now < deadline.uptimeNanoseconds else { + return .timedOut + } + let remainingNanoseconds = deadline.uptimeNanoseconds - now + let wholeMilliseconds = remainingNanoseconds / 1_000_000 + let roundedMilliseconds = wholeMilliseconds + + (remainingNanoseconds % 1_000_000 == 0 ? 0 : 1) + let timeoutMilliseconds = Int32( + min(roundedMilliseconds, UInt64(Int32.max)) + ) + var descriptors = [ + pollfd( + fd: processExitFileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + pollfd( + fd: stdinFailureFileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + ] + let result = Darwin.poll( + &descriptors, + nfds_t(descriptors.count), + timeoutMilliseconds + ) + if result > 0 { + if descriptors[1].revents != 0 { + return .stdinWriteFailed + } + if descriptors[0].revents != 0 { + return .processExited + } + continue + } + if result == 0 { + return .timedOut + } + if errno != EINTR { + return .waitFailed(errno) + } + } + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift index 0b4b8946e2fb..dca6d6d6eb9f 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessExitSignal.swift @@ -9,15 +9,22 @@ internal import Foundation final class RemoteProcessExitSignal: Sendable { private let didExit = AtomicBooleanGate(false) private let group = DispatchGroup() + private let pollSignal: ProcessPipeStopSignal - init() { + init() throws { + pollSignal = try ProcessPipeStopSignal() group.enter() } + var readFileDescriptor: Int32 { + pollSignal.readFileDescriptor + } + func recordExit() { guard didExit.compareExchange(expected: false, desired: true) else { return } + pollSignal.signal() group.leave() } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 150bcde06cc5..5c84d4d0ff7c 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -93,9 +93,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let stdoutHandle = stdoutPipe.fileHandleForReading let stderrHandle = stderrPipe.fileHandleForReading let captureQueue = DispatchQueue(label: "cmux.remote.process.capture") - let processExitSignal = RemoteProcessExitSignal() + let processExitSignal = try RemoteProcessExitSignal() let captureStopSignal = try ProcessPipeStopSignal() let stdinStopSignal = try ProcessPipeStopSignal() + let stdinFailureSignal = try ProcessPipeStopSignal() let stdinWriteState = RemoteProcessStdinWriteState() let captureState = RemoteProcessPipeCaptureState() let captureGroup = DispatchGroup() @@ -239,6 +240,7 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { // POSIX helper turns that race into EPIPE instead of SIGPIPE. } catch { stdinWriteState.record(error: error) + stdinFailureSignal.signal() if process.isRunning { process.terminate() } @@ -247,7 +249,33 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } let timeoutDeadline = DispatchTime.now() + max(0, timeout) - var didTimeOut = !processExitSignal.wait(until: timeoutDeadline) + let completionOutcome = RemoteProcessCompletionWaiter( + processExitFileDescriptor: processExitSignal.readFileDescriptor, + stdinFailureFileDescriptor: stdinFailureSignal.readFileDescriptor + ).wait(until: timeoutDeadline) + var didTimeOut = false + switch completionOutcome { + case .processExited: + break + case .stdinWriteFailed: + let stdinWriteError = stdinWriteState.recordedError ?? POSIXError(.EIO) + if process.isRunning { + terminateProcessAndWait() + } + stdinWriteGroup.wait() + finishCaptureAndCloseReadHandles() + throw stdinWriteFailure(stdinWriteError) + case .timedOut: + didTimeOut = true + case .waitFailed(let code): + stdinStopSignal.signal() + if process.isRunning { + terminateProcessAndWait() + } + stdinWriteGroup.wait() + finishCaptureAndCloseReadHandles() + throw POSIXError(POSIXErrorCode(rawValue: code) ?? .EIO) + } if didTimeOut, !process.isRunning { process.waitUntilExit() noteProcessExit() @@ -258,6 +286,14 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { } if didTimeOut { + if let stdinWriteError = stdinWriteState.recordedError { + if process.isRunning { + terminateProcessAndWait() + } + stdinWriteGroup.wait() + finishCaptureAndCloseReadHandles() + throw stdinWriteFailure(stdinWriteError) + } stdinStopSignal.signal() if let operation, operation.isCancelled { if process.isRunning { From fe9f1b236c193826c7f211501e4e3e07c3d34c7e Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:55:18 -0700 Subject: [PATCH 22/26] refactor: give stop-aware pipe reads an owner --- .../ProcessPipe/ProcessPipeEndRead.swift | 113 -------------- .../ProcessPipeStopAwareReader.swift | 143 ++++++++++++++++++ .../FileHandleProcessPipeReadingTests.swift | 16 +- .../Process/RemoteSessionProcessRunner.swift | 8 +- 4 files changed, 155 insertions(+), 125 deletions(-) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift index 42bd917dbe12..e9b08247722c 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeEndRead.swift @@ -1,16 +1,8 @@ -import Darwin public import Foundation /// The outcome of draining a pipe to end-of-file: every byte read before the /// stream ended, plus the read error that interrupted the drain, if any. public struct ProcessPipeEndRead: Equatable, Sendable { - // Darwin's FIONREAD macro is not imported into Swift because it expands - // through `_IOR`. This is `_IOR('f', 127, Int32)` from sys/filio.h. - private static let bytesAvailableIOControlRequest = - UInt(0x4000_0000) - | (UInt(MemoryLayout.size) << 16) - | UInt(0x66_7f) - /// The bytes successfully read before EOF or the failure. public let data: Data /// The error that ended the drain early, or `nil` on a clean EOF. @@ -71,109 +63,4 @@ public struct ProcessPipeEndRead: Equatable, Sendable { } } - /// Drains a process pipe until EOF or until `stopFileDescriptor` becomes - /// readable or hung up. On stop it preserves bytes already buffered in the - /// pipe, then returns without waiting for a future writer or an inherited - /// descriptor to close. - public static func reading( - fileDescriptor: Int32, - chunkSize: Int = FileHandle.processPipeReadChunkSize, - stopFileDescriptor: Int32 - ) -> ProcessPipeEndRead { - var data = Data() - - while true { - var descriptors = [ - pollfd( - fd: fileDescriptor, - events: Int16(POLLIN | POLLERR | POLLHUP), - revents: 0 - ), - pollfd( - fd: stopFileDescriptor, - events: Int16(POLLIN | POLLERR | POLLHUP), - revents: 0 - ), - ] - let pollResult = Darwin.poll(&descriptors, nfds_t(descriptors.count), -1) - if pollResult < 0 { - let code = errno - if code == EINTR { - continue - } - return ProcessPipeEndRead( - data: data, - readError: ProcessPipeReadError( - operation: "readDataToEndOfFile.poll", - errnoCode: code - ) - ) - } - if (descriptors[0].revents & Int16(POLLNVAL)) != 0 { - return ProcessPipeEndRead( - data: data, - readError: ProcessPipeReadError( - operation: "readDataToEndOfFile.poll", - errnoCode: EBADF - ) - ) - } - - let isStopping = descriptors[1].revents != 0 - var bytesRemainingAtStop: Int? - if isStopping { - var availableBytes: Int32 = 0 - guard Darwin.ioctl( - fileDescriptor, - Self.bytesAvailableIOControlRequest, - &availableBytes - ) == 0 else { - return ProcessPipeEndRead( - data: data, - readError: ProcessPipeReadError( - operation: "readDataToEndOfFile.ioctl", - errnoCode: errno - ) - ) - } - bytesRemainingAtStop = Int(availableBytes) - if availableBytes == 0 { - return ProcessPipeEndRead(data: data, readError: nil) - } - } - - repeat { - let maximumReadLength = bytesRemainingAtStop.map { - min(chunkSize, $0) - } ?? chunkSize - switch ProcessPipeAvailableRead.readOnceIfReady( - fileDescriptor: fileDescriptor, - maxLength: maximumReadLength, - operation: "readDataToEndOfFile" - ) { - case .success(.data(let chunk)): - data.append(chunk) - if let remaining = bytesRemainingAtStop.map({ $0 - chunk.count }) { - if remaining <= 0 { - return ProcessPipeEndRead(data: data, readError: nil) - } - bytesRemainingAtStop = remaining - } - case .success(.wouldBlock): - if isStopping { - return ProcessPipeEndRead(data: data, readError: nil) - } - break - case .success(.endOfFile): - return ProcessPipeEndRead(data: data, readError: nil) - case .failure(let error): - return ProcessPipeEndRead(data: data, readError: error) - } - } while isStopping - - if isStopping { - return ProcessPipeEndRead(data: data, readError: nil) - } - } - } } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift new file mode 100644 index 000000000000..24102e3301f5 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift @@ -0,0 +1,143 @@ +import Darwin +public import Foundation + +/// Drains a process pipe until EOF or a separate stop descriptor fires. +public struct ProcessPipeStopAwareReader: Sendable { + // Darwin's FIONREAD macro is not imported into Swift because it expands + // through `_IOR`. This is `_IOR('f', 127, Int32)` from sys/filio.h. + private static let bytesAvailableIOControlRequest = + UInt(0x4000_0000) + | (UInt(MemoryLayout.size) << 16) + | UInt(0x66_7f) + + private let fileDescriptor: Int32 + private let chunkSize: Int + private let stopFileDescriptor: Int32 + + /// Creates a stop-aware pipe reader. + /// + /// - Parameters: + /// - fileDescriptor: The process-pipe descriptor to drain. + /// - chunkSize: The maximum bytes read per operation. + /// - stopFileDescriptor: A descriptor whose readiness ends the drain + /// after bytes already buffered in `fileDescriptor` are preserved. + public init( + fileDescriptor: Int32, + chunkSize: Int = FileHandle.processPipeReadChunkSize, + stopFileDescriptor: Int32 + ) { + self.fileDescriptor = fileDescriptor + self.chunkSize = chunkSize + self.stopFileDescriptor = stopFileDescriptor + } + + /// Drains the pipe, preserving bytes buffered when the stop signal fires. + /// + /// - Returns: The captured bytes and any read failure. + public func readToEnd() -> ProcessPipeEndRead { + var data = Data() + + while true { + var descriptors = [ + pollfd( + fd: fileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + pollfd( + fd: stopFileDescriptor, + events: Int16(POLLIN | POLLERR | POLLHUP), + revents: 0 + ), + ] + let pollResult = Darwin.poll( + &descriptors, + nfds_t(descriptors.count), + -1 + ) + if pollResult < 0 { + let code = errno + if code == EINTR { + continue + } + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.poll", + errnoCode: code + ) + ) + } + if (descriptors[0].revents & Int16(POLLNVAL)) != 0 { + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.poll", + errnoCode: EBADF + ) + ) + } + + let isStopping = descriptors[1].revents != 0 + var bytesRemainingAtStop: Int? + if isStopping { + var availableBytes: Int32 = 0 + guard Darwin.ioctl( + fileDescriptor, + Self.bytesAvailableIOControlRequest, + &availableBytes + ) == 0 else { + return ProcessPipeEndRead( + data: data, + readError: ProcessPipeReadError( + operation: "readDataToEndOfFile.ioctl", + errnoCode: errno + ) + ) + } + bytesRemainingAtStop = Int(availableBytes) + if availableBytes == 0 { + return ProcessPipeEndRead(data: data, readError: nil) + } + } + + repeat { + let maximumReadLength = bytesRemainingAtStop.map { + min(chunkSize, $0) + } ?? chunkSize + switch ProcessPipeAvailableRead.readOnceIfReady( + fileDescriptor: fileDescriptor, + maxLength: maximumReadLength, + operation: "readDataToEndOfFile" + ) { + case .success(.data(let chunk)): + data.append(chunk) + if let remaining = bytesRemainingAtStop.map({ + $0 - chunk.count + }) { + if remaining <= 0 { + return ProcessPipeEndRead( + data: data, + readError: nil + ) + } + bytesRemainingAtStop = remaining + } + case .success(.wouldBlock): + if isStopping { + return ProcessPipeEndRead(data: data, readError: nil) + } + break + case .success(.endOfFile): + return ProcessPipeEndRead(data: data, readError: nil) + case .failure(let error): + return ProcessPipeEndRead(data: data, readError: error) + } + } while isStopping + + if isStopping { + return ProcessPipeEndRead(data: data, readError: nil) + } + } + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift index 94f4bf12bb1b..b2996e4fbb51 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift @@ -75,10 +75,10 @@ struct FileHandleProcessPipeReadingTests { try pipe.fileHandleForWriting.write(contentsOf: Data("buffered".utf8)) stopSignal.signal() - let result = ProcessPipeEndRead.reading( + let result = ProcessPipeStopAwareReader( fileDescriptor: pipe.fileHandleForReading.fileDescriptor, stopFileDescriptor: stopSignal.readFileDescriptor - ) + ).readToEnd() #expect(result.data == Data("buffered".utf8)) #expect(result.readError == nil) @@ -96,11 +96,11 @@ struct FileHandleProcessPipeReadingTests { try pipe.fileHandleForWriting.write(contentsOf: payload) stopSignal.signal() - let result = ProcessPipeEndRead.reading( + let result = ProcessPipeStopAwareReader( fileDescriptor: pipe.fileHandleForReading.fileDescriptor, chunkSize: 1, stopFileDescriptor: stopSignal.readFileDescriptor - ) + ).readToEnd() #expect(result.data == payload) #expect(result.readError == nil) @@ -122,17 +122,17 @@ struct FileHandleProcessPipeReadingTests { let firstFinished = DispatchSemaphore(value: 0) let secondFinished = DispatchSemaphore(value: 0) DispatchQueue.global(qos: .utility).async { - _ = ProcessPipeEndRead.reading( + _ = ProcessPipeStopAwareReader( fileDescriptor: firstPipe.fileHandleForReading.fileDescriptor, stopFileDescriptor: stopSignal.readFileDescriptor - ) + ).readToEnd() firstFinished.signal() } DispatchQueue.global(qos: .utility).async { - _ = ProcessPipeEndRead.reading( + _ = ProcessPipeStopAwareReader( fileDescriptor: secondPipe.fileHandleForReading.fileDescriptor, stopFileDescriptor: stopSignal.readFileDescriptor - ) + ).readToEnd() secondFinished.signal() } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 5c84d4d0ff7c..70de409feb9d 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -125,10 +125,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { captureGroup.leave() } defer { _ = Darwin.close(stdoutDescriptor) } - let result = ProcessPipeEndRead.reading( + let result = ProcessPipeStopAwareReader( fileDescriptor: stdoutDescriptor, stopFileDescriptor: captureStopSignal.readFileDescriptor - ) + ).readToEnd() captureQueue.sync { captureState.stdoutData = result.data captureState.stdoutReadError = result.readError @@ -138,10 +138,10 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { DispatchQueue.global(qos: .utility).async { defer { captureGroup.leave() } defer { _ = Darwin.close(stderrDescriptor) } - let result = ProcessPipeEndRead.reading( + let result = ProcessPipeStopAwareReader( fileDescriptor: stderrDescriptor, stopFileDescriptor: captureStopSignal.readFileDescriptor - ) + ).readToEnd() captureQueue.sync { captureState.stderrData = result.data captureState.stderrReadError = result.readError From 155a2512046a74693e28abf4e182f03ed5a019f0 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:10:53 -0700 Subject: [PATCH 23/26] test: reject non-positive pipe read chunks --- .../FileHandleProcessPipeReadingTests.swift | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift index b2996e4fbb51..9444fe87043a 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileHandleProcessPipeReadingTests.swift @@ -106,6 +106,33 @@ struct FileHandleProcessPipeReadingTests { #expect(result.readError == nil) } + @Test( + "A non-positive chunk size still drains the stopped snapshot", + arguments: [0, -1] + ) + func nonPositiveChunkSizeDrainsStoppedSnapshot( + chunkSize: Int + ) throws { + let pipe = Pipe() + let stopSignal = try ProcessPipeStopSignal() + let payload = Data("buffered".utf8) + defer { + try? pipe.fileHandleForWriting.close() + try? pipe.fileHandleForReading.close() + } + try pipe.fileHandleForWriting.write(contentsOf: payload) + stopSignal.signal() + + let result = ProcessPipeStopAwareReader( + fileDescriptor: pipe.fileHandleForReading.fileDescriptor, + chunkSize: chunkSize, + stopFileDescriptor: stopSignal.readFileDescriptor + ).readToEnd() + + #expect(result.data == payload) + #expect(result.readError == nil) + } + @Test("One stop signal wakes every blocked pipe reader") func stopSignalBroadcastsToBlockedReaders() throws { let firstPipe = Pipe() From e65b0c9f10b405e71116b3579c527398c8b04678 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:17:42 -0700 Subject: [PATCH 24/26] fix: normalize pipe read chunk sizes --- .../ProcessPipe/ProcessPipeStopAwareReader.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift index 24102e3301f5..2d4da8d0dcbc 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ProcessPipe/ProcessPipeStopAwareReader.swift @@ -18,7 +18,8 @@ public struct ProcessPipeStopAwareReader: Sendable { /// /// - Parameters: /// - fileDescriptor: The process-pipe descriptor to drain. - /// - chunkSize: The maximum bytes read per operation. + /// - chunkSize: The requested maximum bytes read per operation. + /// Non-positive values are normalized to one byte. /// - stopFileDescriptor: A descriptor whose readiness ends the drain /// after bytes already buffered in `fileDescriptor` are preserved. public init( @@ -27,7 +28,7 @@ public struct ProcessPipeStopAwareReader: Sendable { stopFileDescriptor: Int32 ) { self.fileDescriptor = fileDescriptor - self.chunkSize = chunkSize + self.chunkSize = max(1, chunkSize) self.stopFileDescriptor = stopFileDescriptor } From 4388ec6accb3bd65a46905e3b79567e7541e4fbd Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:57:55 -0700 Subject: [PATCH 25/26] test: make remote process races event driven --- ...GatedFailingRemoteProcessStdinWriter.swift | 48 +++-- .../RemoteProcessRunRecorder.swift | 17 -- .../RemoteSessionProcessRunnerTests.swift | 176 +++++++++++------- 3 files changed, 132 insertions(+), 109 deletions(-) delete mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift index 48a7e6c8eb75..e3d7972e0b6a 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/MarkerGatedFailingRemoteProcessStdinWriter.swift @@ -3,6 +3,8 @@ import Foundation @testable import CmuxRemoteSession struct MarkerGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { + let readyFIFOURL: URL + let exitFIFOURL: URL let markerURL: URL let gate: RemoteProcessStdinWriterFailureGate @@ -11,36 +13,28 @@ struct MarkerGatedFailingRemoteProcessStdinWriter: RemoteProcessStdinWriting { to handle: FileHandle, stopFileDescriptor: Int32 ) throws { - let processIdentifier = try waitForProcessMarker() - if gate == .exited, !waitForProcessExit(processIdentifier) { - throw POSIXError(.ETIMEDOUT) + let processIdentifier = try readProcessIdentifier() + try Data("\(processIdentifier)".utf8).write(to: markerURL) + if gate == .exited { + let exitHandle = try FileHandle(forReadingFrom: exitFIFOURL) + defer { try? exitHandle.close() } + _ = exitHandle.readDataToEndOfFile() } throw POSIXError(.EIO) } - private func waitForProcessMarker() throws -> pid_t { - let deadline = DispatchTime.now() + 2 - repeat { - if let contents = try? String(contentsOf: markerURL, encoding: .utf8), - let processIdentifier = pid_t( - contents.trimmingCharacters(in: .whitespacesAndNewlines) - ) { - return processIdentifier - } - Thread.sleep(forTimeInterval: 0.01) - } while DispatchTime.now() < deadline - throw POSIXError(.ETIMEDOUT) - } - - private func waitForProcessExit(_ processIdentifier: pid_t) -> Bool { - let deadline = DispatchTime.now() + 2 - repeat { - errno = 0 - if kill(processIdentifier, 0) == -1, errno == ESRCH { - return true - } - Thread.sleep(forTimeInterval: 0.01) - } while DispatchTime.now() < deadline - return false + private func readProcessIdentifier() throws -> pid_t { + let readyHandle = try FileHandle(forReadingFrom: readyFIFOURL) + defer { try? readyHandle.close() } + let contents = String( + decoding: readyHandle.readDataToEndOfFile(), + as: UTF8.self + ) + guard let processIdentifier = pid_t( + contents.trimmingCharacters(in: .whitespacesAndNewlines) + ) else { + throw POSIXError(.EINVAL) + } + return processIdentifier } } diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift deleted file mode 100644 index 33b6bfc7578a..000000000000 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteProcessRunRecorder.swift +++ /dev/null @@ -1,17 +0,0 @@ -import Foundation -@testable import CmuxRemoteSession - -final class RemoteProcessRunRecorder: @unchecked Sendable { - private let lock = NSLock() - private var storedResult: Result? - - var result: Result? { - lock.withLock { storedResult } - } - - func record(_ result: Result) { - lock.withLock { - storedResult = result - } - } -} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 74a502251272..f6fef78b288f 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -89,11 +89,12 @@ struct RemoteSessionProcessRunnerTests { @Test("An unexpected stdin write error terminates the child and keeps the pinned runner error") func unexpectedStdinWriteErrorTerminatesChildAndKeepsPinnedRunnerError() throws { - let markerURL = processMarkerURL() - defer { try? FileManager.default.removeItem(at: markerURL) } + let marker = try ProcessMarkerFixture() let runner = RemoteSessionProcessRunner( stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( - markerURL: markerURL, + readyFIFOURL: marker.readyFIFOURL, + exitFIFOURL: marker.exitFIFOURL, + markerURL: marker.markerURL, gate: .launched ) ) @@ -104,9 +105,11 @@ struct RemoteSessionProcessRunnerTests { executable: "/bin/sh", arguments: [ "-c", - "echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exec /bin/sleep 30", + "echo $$ > \"$CMUX_TEST_PROCESS_READY\"; exec /bin/sleep 30", + ], + environment: [ + "CMUX_TEST_PROCESS_READY": marker.readyFIFOURL.path, ], - environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], stdin: Data("payload".utf8), timeout: 5 ), @@ -120,30 +123,32 @@ struct RemoteSessionProcessRunnerTests { && nsError.localizedDescription.hasPrefix("Failed to write stdin for sh:") && underlyingError?.code == .EIO } - let processIdentifier = try recordedProcessIdentifier(at: markerURL) - #expect(waitForProcessExit(processIdentifier, timeout: 2)) + #expect(try marker.recordedProcessHasExited()) } @Test("A stdin write error wins when the child ignores termination") - func stdinWriteErrorWinsWhenChildIgnoresTermination() throws { - let markerURL = processMarkerURL() - defer { try? FileManager.default.removeItem(at: markerURL) } + func stdinWriteErrorWinsWhenChildIgnoresTermination() async throws { + let marker = try ProcessMarkerFixture() let runner = RemoteSessionProcessRunner( stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( - markerURL: markerURL, + readyFIFOURL: marker.readyFIFOURL, + exitFIFOURL: marker.exitFIFOURL, + markerURL: marker.markerURL, gate: .launched ) ) - let outcome = try #require(runProcess( + let outcome = try #require(await runProcess( runner, request: RemoteProcessRequest( executable: "/bin/sh", arguments: [ "-c", - "trap '' TERM; echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exec /bin/sleep 30", + "trap '' TERM; echo $$ > \"$CMUX_TEST_PROCESS_READY\"; exec /bin/sleep 30", + ], + environment: [ + "CMUX_TEST_PROCESS_READY": marker.readyFIFOURL.path, ], - environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], stdin: Data("payload".utf8), timeout: 10 ), @@ -163,11 +168,12 @@ struct RemoteSessionProcessRunnerTests { @Test("A late stdin write error wins over an earlier child exit") func lateStdinWriteErrorWinsOverChildExit() throws { - let markerURL = processMarkerURL() - defer { try? FileManager.default.removeItem(at: markerURL) } + let marker = try ProcessMarkerFixture() let runner = RemoteSessionProcessRunner( stdinWriter: MarkerGatedFailingRemoteProcessStdinWriter( - markerURL: markerURL, + readyFIFOURL: marker.readyFIFOURL, + exitFIFOURL: marker.exitFIFOURL, + markerURL: marker.markerURL, gate: .exited ) ) @@ -178,9 +184,12 @@ struct RemoteSessionProcessRunnerTests { executable: "/bin/sh", arguments: [ "-c", - "echo $$ > \"$CMUX_TEST_PROCESS_MARKER\"; exit 0", + "echo $$ > \"$CMUX_TEST_PROCESS_READY\"; exec 3>\"$CMUX_TEST_PROCESS_EXIT\"; exit 0", + ], + environment: [ + "CMUX_TEST_PROCESS_READY": marker.readyFIFOURL.path, + "CMUX_TEST_PROCESS_EXIT": marker.exitFIFOURL.path, ], - environment: ["CMUX_TEST_PROCESS_MARKER": markerURL.path], stdin: Data("payload".utf8), timeout: 5 ), @@ -196,9 +205,9 @@ struct RemoteSessionProcessRunnerTests { } @Test("A child retaining unread stdin cannot delay the process timeout") - func unreadStdinCannotDelayProcessTimeout() throws { + func unreadStdinCannotDelayProcessTimeout() async throws { let runner = RemoteSessionProcessRunner() - let outcome = try #require(runProcess( + let outcome = try #require(await runProcess( runner, request: RemoteProcessRequest( executable: "/bin/sleep", @@ -223,9 +232,9 @@ struct RemoteSessionProcessRunnerTests { } @Test("A descendant retaining unread stdin cannot outlive the process timeout") - func inheritedUnreadStdinCannotOutliveProcessTimeout() throws { + func inheritedUnreadStdinCannotOutliveProcessTimeout() async throws { let runner = RemoteSessionProcessRunner() - let outcome = try #require(runProcess( + let outcome = try #require(await runProcess( runner, request: RemoteProcessRequest( executable: "/bin/sh", @@ -313,54 +322,91 @@ struct RemoteSessionProcessRunnerTests { && nsError.localizedDescription == "sh timed out after 1s" } } -} -} -private func processMarkerURL() -> URL { - FileManager.default.temporaryDirectory.appendingPathComponent( - "cmux-remote-process-\(UUID().uuidString).pid", - isDirectory: false - ) -} + private final class ProcessMarkerFixture { + let readyFIFOURL: URL + let exitFIFOURL: URL + let markerURL: URL -private func recordedProcessIdentifier(at markerURL: URL) throws -> pid_t { - let contents = try String(contentsOf: markerURL, encoding: .utf8) - return try #require( - pid_t(contents.trimmingCharacters(in: .whitespacesAndNewlines)) - ) -} + init() throws { + let temporaryDirectory = FileManager.default.temporaryDirectory + let identifier = UUID().uuidString + readyFIFOURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).ready", + isDirectory: false + ) + exitFIFOURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).exit", + isDirectory: false + ) + markerURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).pid", + isDirectory: false + ) -private func waitForProcessExit(_ processIdentifier: pid_t, timeout: TimeInterval) -> Bool { - let deadline = DispatchTime.now() + timeout - repeat { - errno = 0 - if kill(processIdentifier, 0) == -1, errno == ESRCH { - return true + guard Darwin.mkfifo(readyFIFOURL.path, 0o600) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + guard Darwin.mkfifo(exitFIFOURL.path, 0o600) == 0 else { + let code = POSIXErrorCode(rawValue: errno) ?? .EIO + try? FileManager.default.removeItem(at: readyFIFOURL) + throw POSIXError(code) + } } - Thread.sleep(forTimeInterval: 0.01) - } while DispatchTime.now() < deadline - return false -} -private func runProcess( - _ runner: RemoteSessionProcessRunner, - request: RemoteProcessRequest, - completingWithin timeout: TimeInterval -) -> Result? { - let recorder = RemoteProcessRunRecorder() - let completed = DispatchSemaphore(value: 0) - DispatchQueue.global(qos: .userInitiated).async { - recorder.record(Result { - try runner.run(request, operation: nil) - }) - completed.signal() + deinit { + try? FileManager.default.removeItem(at: readyFIFOURL) + try? FileManager.default.removeItem(at: exitFIFOURL) + try? FileManager.default.removeItem(at: markerURL) + } + + func recordedProcessHasExited() throws -> Bool { + let contents = try String(contentsOf: markerURL, encoding: .utf8) + let processIdentifier = try #require( + pid_t(contents.trimmingCharacters(in: .whitespacesAndNewlines)) + ) + errno = 0 + return Darwin.kill(processIdentifier, 0) == -1 && errno == ESRCH + } } - guard completed.wait(timeout: .now() + timeout) == .success else { - // The fixture exits after five seconds, so reap the test-owned work - // before returning a failure and starting another subprocess test. - completed.wait() - return nil + private enum ProcessRunEvent: Sendable { + case completed(Result) + case deadline } - return recorder.result + + private func runProcess( + _ runner: RemoteSessionProcessRunner, + request: RemoteProcessRequest, + completingWithin timeout: TimeInterval + ) async -> Result? { + await withTaskGroup( + of: ProcessRunEvent.self, + returning: Result?.self + ) { group in + group.addTask { + .completed(Result { + try runner.run(request, operation: nil) + }) + } + group.addTask { + try? await Task.sleep(for: .seconds(timeout)) + return .deadline + } + + guard let firstEvent = await group.next() else { + return nil + } + group.cancelAll() + switch firstEvent { + case .completed(let result): + return result + case .deadline: + // Structured concurrency reaps the test-owned blocking task + // before leaving this scope and starting another subprocess. + return nil + } + } + } +} } From 47d67cd752f690a7a57f0cfeac192dc1d17ea95c Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:05:12 -0700 Subject: [PATCH 26/26] test: split remote process fixtures --- .../ProcessMarkerFixture.swift | 50 +++++++++++++++++ .../ProcessRunEvent.swift | 7 +++ .../RemoteSessionProcessRunnerTests.swift | 53 ------------------- 3 files changed, 57 insertions(+), 53 deletions(-) create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessMarkerFixture.swift create mode 100644 Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessRunEvent.swift diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessMarkerFixture.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessMarkerFixture.swift new file mode 100644 index 000000000000..42c2dc479c7d --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessMarkerFixture.swift @@ -0,0 +1,50 @@ +import Darwin +import Foundation +import Testing + +final class ProcessMarkerFixture { + let readyFIFOURL: URL + let exitFIFOURL: URL + let markerURL: URL + + init() throws { + let temporaryDirectory = FileManager.default.temporaryDirectory + let identifier = UUID().uuidString + readyFIFOURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).ready", + isDirectory: false + ) + exitFIFOURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).exit", + isDirectory: false + ) + markerURL = temporaryDirectory.appendingPathComponent( + "cmux-remote-process-\(identifier).pid", + isDirectory: false + ) + + guard Darwin.mkfifo(readyFIFOURL.path, 0o600) == 0 else { + throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) + } + guard Darwin.mkfifo(exitFIFOURL.path, 0o600) == 0 else { + let code = POSIXErrorCode(rawValue: errno) ?? .EIO + try? FileManager.default.removeItem(at: readyFIFOURL) + throw POSIXError(code) + } + } + + deinit { + try? FileManager.default.removeItem(at: readyFIFOURL) + try? FileManager.default.removeItem(at: exitFIFOURL) + try? FileManager.default.removeItem(at: markerURL) + } + + func recordedProcessHasExited() throws -> Bool { + let contents = try String(contentsOf: markerURL, encoding: .utf8) + let processIdentifier = try #require( + pid_t(contents.trimmingCharacters(in: .whitespacesAndNewlines)) + ) + errno = 0 + return Darwin.kill(processIdentifier, 0) == -1 && errno == ESRCH + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessRunEvent.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessRunEvent.swift new file mode 100644 index 000000000000..a0abfdd368b8 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/ProcessRunEvent.swift @@ -0,0 +1,7 @@ +import Foundation +@testable import CmuxRemoteSession + +enum ProcessRunEvent: Sendable { + case completed(Result) + case deadline +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index f6fef78b288f..23ace48664a6 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -1,4 +1,3 @@ -import Darwin import Foundation import Testing @testable import CmuxRemoteSession @@ -323,58 +322,6 @@ struct RemoteSessionProcessRunnerTests { } } - private final class ProcessMarkerFixture { - let readyFIFOURL: URL - let exitFIFOURL: URL - let markerURL: URL - - init() throws { - let temporaryDirectory = FileManager.default.temporaryDirectory - let identifier = UUID().uuidString - readyFIFOURL = temporaryDirectory.appendingPathComponent( - "cmux-remote-process-\(identifier).ready", - isDirectory: false - ) - exitFIFOURL = temporaryDirectory.appendingPathComponent( - "cmux-remote-process-\(identifier).exit", - isDirectory: false - ) - markerURL = temporaryDirectory.appendingPathComponent( - "cmux-remote-process-\(identifier).pid", - isDirectory: false - ) - - guard Darwin.mkfifo(readyFIFOURL.path, 0o600) == 0 else { - throw POSIXError(POSIXErrorCode(rawValue: errno) ?? .EIO) - } - guard Darwin.mkfifo(exitFIFOURL.path, 0o600) == 0 else { - let code = POSIXErrorCode(rawValue: errno) ?? .EIO - try? FileManager.default.removeItem(at: readyFIFOURL) - throw POSIXError(code) - } - } - - deinit { - try? FileManager.default.removeItem(at: readyFIFOURL) - try? FileManager.default.removeItem(at: exitFIFOURL) - try? FileManager.default.removeItem(at: markerURL) - } - - func recordedProcessHasExited() throws -> Bool { - let contents = try String(contentsOf: markerURL, encoding: .utf8) - let processIdentifier = try #require( - pid_t(contents.trimmingCharacters(in: .whitespacesAndNewlines)) - ) - errno = 0 - return Darwin.kill(processIdentifier, 0) == -1 && errno == ESRCH - } - } - - private enum ProcessRunEvent: Sendable { - case completed(Result) - case deadline - } - private func runProcess( _ runner: RemoteSessionProcessRunner, request: RemoteProcessRequest,