diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink+StderrDrain.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink+StderrDrain.swift new file mode 100644 index 000000000000..61eaf9e9cec5 --- /dev/null +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink+StderrDrain.swift @@ -0,0 +1,20 @@ +import Foundation + +extension CloudMachineLink { + /// Waits for the link client's stderr reader to reach EOF, so an exit error + /// reads its last lines: they can still be in flight when the process + /// exits. A child the client started can hold the pipe open, so the wait + /// is bounded. + nonisolated static func awaitStderrDrain(_ drain: Task, upTo limit: Duration = .seconds(1)) async { + let drained = CloudLinkFirstValue() + Task.detached { + await drain.value + drained.resolve(true) + } + Task.detached { + try? await Task.sleep(for: limit) + drained.resolve(false) + } + _ = await drained.result + } +} diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink.swift index 86270efbcb5a..a8264dc67ef4 100644 --- a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink.swift +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Link/CloudMachineLink.swift @@ -235,7 +235,7 @@ public actor CloudMachineLink { } self.process = process self.processExit = processExit - drainStderr(stderr.fileHandleForReading) + let stderrDrain = drainStderr(stderr.fileHandleForReading) // The first connection-snapshot line names the socket; later lines only update // transport topology and are ignored — but stdout keeps draining for the @@ -271,15 +271,14 @@ public actor CloudMachineLink { // client rejecting a flag, a refused dial). Report that exit and its // stderr, not the deadline it never reached. await Self.terminateAndWait(process, exit: processExit) - throw LinkError.exited( - status: process.terminationStatus, - output: stderrTail.joined(separator: "\n") - ) + await Self.awaitStderrDrain(stderrDrain) + throw LinkError.exited(status: process.terminationStatus, output: stderrTail.joined(separator: "\n")) case .timedOut?, nil: throw LinkError.timedOut } } guard process.isRunning else { + await Self.awaitStderrDrain(stderrDrain) throw LinkError.exited(status: process.terminationStatus, output: stderrTail.joined(separator: "\n")) } } catch { @@ -625,9 +624,9 @@ public actor CloudMachineLink { eventsRecoveryPhase = .healthy } - private func drainStderr(_ handle: FileHandle) { + private func drainStderr(_ handle: FileHandle) -> Task { let lines = CloudLinkPipe.lines(from: handle) - Task.detached { [weak self] in + return Task.detached { [weak self] in for await line in lines { await self?.recordStderr(line) } diff --git a/Packages/macOS/CmuxCloud/Tests/CmuxCloudTests/CloudMachineLinkExitDiagnosticsTests.swift b/Packages/macOS/CmuxCloud/Tests/CmuxCloudTests/CloudMachineLinkExitDiagnosticsTests.swift new file mode 100644 index 000000000000..8634ca1dfdac --- /dev/null +++ b/Packages/macOS/CmuxCloud/Tests/CmuxCloudTests/CloudMachineLinkExitDiagnosticsTests.swift @@ -0,0 +1,72 @@ +@testable import CmuxCloud +import CmuxCloudTui +import Foundation +import Testing + +/// A link client that exits before naming its socket fails the connect with its +/// own exit status and stderr. The stderr reader runs in its own task, so the +/// last lines can still be in flight when the process exits. +@Suite("Cloud machine link exit diagnostics") +struct CloudMachineLinkExitDiagnosticsTests { + @Test("An exit error carries stderr written until the pipe closes") + func exitErrorWaitsForStderrToClose() async throws { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-link-exit-stderr-\(UUID().uuidString.lowercased())", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let client = root.appendingPathComponent("fake-cmux-tui") + // The client exits at once. Once it is reaped, a child it started closes + // stdout and writes the last stderr line a moment later. Closing stdout + // earlier would let connect terminate the unreaped client's process + // group, child included, before the line is written. Without the wait, + // connect builds the error within milliseconds of stdout closing. + try """ + #!/bin/sh + (while kill -0 $$ 2>/dev/null; do sleep 0.01; done + exec >/dev/null; sleep 0.1; echo 'cmux-tui: route refused' >&2) & + exit 2 + """.write(to: client, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: client.path) + let link = CloudMachineLink(machineID: "test-machine", clientURL: client, paths: CloudTuiClientPaths(home: root)) + + do { + _ = try await link.connect(route: "ws://10.0.0.1:1337/v1/link", session: "main", carrier: true) + Issue.record("a client that exits before its socket line must fail the connect") + } catch CloudMachineLink.LinkError.exited(let status, let output) { + #expect(status == 2) + #expect(output.contains("route refused"), "stderr written before the pipe closed must reach the error: \(output)") + } catch { + Issue.record("expected LinkError.exited, got \(error)") + } + } + + @Test("An exit after the socket line carries stderr written until the pipe closes") + func exitAfterSocketLineWaitsForStderrToClose() async throws { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-link-exit-after-socket-\(UUID().uuidString.lowercased())", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let client = root.appendingPathComponent("fake-cmux-tui") + // The client exits at once. Once it is reaped, a child it started names + // the socket, closes stdout and writes the last stderr line a moment later. + try """ + #!/bin/sh + (while kill -0 $$ 2>/dev/null; do sleep 0.01; done + printf '%s\\n' '{"event":"connection-snapshot","local_socket":"/tmp/cmux-link-exit-test.sock"}' + exec >/dev/null; sleep 0.1; echo 'cmux-tui: route refused' >&2) & + exit 2 + """.write(to: client, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: client.path) + let link = CloudMachineLink(machineID: "test-machine", clientURL: client, paths: CloudTuiClientPaths(home: root)) + + do { + _ = try await link.connect(route: "ws://10.0.0.1:1337/v1/link", session: "main", carrier: true) + Issue.record("a client that exits after its socket line must fail the connect") + } catch CloudMachineLink.LinkError.exited(let status, let output) { + #expect(status == 2) + #expect(output.contains("route refused"), "stderr written before the pipe closed must reach the error: \(output)") + } catch { + Issue.record("expected LinkError.exited, got \(error)") + } + } +}