From b73933eb2d7a621e8fac2cbdf129028b5ae47557 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <mvanhorn@users.noreply.github.com>
Date: Thu, 30 Apr 2026 10:28:27 -0700
Subject: [PATCH] Tests/CLITests: drain stdout/stderr concurrently in
 CLITest.run (#1471)

## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update

## Motivation and Context

`CLITest.run(arguments:stdin:currentDirectory:env:)` reads stdout to EOF
before stderr begins draining
(`Tests/CLITests/Utilities/CLITest.swift:182-183`). If a child process
writes more than a pipe-buffer's worth of data to stderr (~64 KB on
macOS), the child blocks in `write()` on stderr while we block in
`readDataToEndOfFile()` on stdout, and neither side makes progress until
`process.waitUntilExit()` returns. This is a latent deadlock that any
future test or CLI verbosity bump can trigger.

This PR drains both streams concurrently using `readabilityHandler`
closures backed by `Mutex<Data>` buffers. After
`process.waitUntilExit()` returns the handlers are cleared and
`readDataToEndOfFile()` flushes any bytes the kernel buffered between
the last handler invocation and exit. The error path also clears
handlers so a failed `process.run()` does not leak callbacks. `Mutex`
matches the locking primitive the file already uses for `commandSeq`.

Fixes #1456

## Testing

- [x] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs

`swift build --target CLITests` passes against the change. `make
swift-fmt-check` is clean. Behavior is unchanged for any test command
that previously fit within a single pipe buffer, so the existing
CLITests still exercise the helper end-to-end.

I did not add a new test that emits >64 KB to stderr because every
existing CLITest invocation goes through `executablePath` (the
`container` binary), and reproducing the deadlock requires a child that
emits a controllable amount on stderr. Happy to follow up with a small
refactor that extracts the drain into a static helper plus a regression
test that drives it via `/bin/sh` if that would be useful.

---------

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
---
 Tests/CLITests/Utilities/CLITest.swift | 30 ++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/Tests/CLITests/Utilities/CLITest.swift b/Tests/CLITests/Utilities/CLITest.swift
index 58adf6cc..7bbe63cf 100644
--- a/Tests/CLITests/Utilities/CLITest.swift
+++ b/Tests/CLITests/Utilities/CLITest.swift
@@ -165,23 +165,41 @@ class CLITest {
         }
 
         let inputPipe = Pipe()
-        let outputPipe = Pipe()
-        let errorPipe = Pipe()
         process.standardInput = inputPipe
-        process.standardOutput = outputPipe
-        process.standardError = errorPipe
 
         let outputData: Data
         let errorData: Data
         do {
+            // Redirect stdout/stderr to temp files so the child process never
+            // blocks on `write()` when one stream fills the kernel pipe buffer
+            // before the parent drains it (issue #1456).
+            let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+            try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true)
+            defer {
+                try? FileManager.default.removeItem(at: tempDir)
+            }
+
+            let stdoutURL = tempDir.appendingPathComponent("stdout")
+            let stderrURL = tempDir.appendingPathComponent("stderr")
+            FileManager.default.createFile(atPath: stdoutURL.path, contents: nil)
+            FileManager.default.createFile(atPath: stderrURL.path, contents: nil)
+
+            let stdoutHandle = try FileHandle(forWritingTo: stdoutURL)
+            defer { try? stdoutHandle.close() }
+            let stderrHandle = try FileHandle(forWritingTo: stderrURL)
+            defer { try? stderrHandle.close() }
+            process.standardOutput = stdoutHandle
+            process.standardError = stderrHandle
+
             try process.run()
             if let data = stdin {
                 inputPipe.fileHandleForWriting.write(data)
             }
             inputPipe.fileHandleForWriting.closeFile()
-            outputData = outputPipe.fileHandleForReading.readDataToEndOfFile()
-            errorData = errorPipe.fileHandleForReading.readDataToEndOfFile()
             process.waitUntilExit()
+
+            outputData = try Data(contentsOf: stdoutURL)
+            errorData = try Data(contentsOf: stderrURL)
         } catch {
             throw CLIError.executionFailed("Failed to run CLI: \(error)")
         }