From ccc6aaf06df14ce1ce5dbc545910526ce183d51c Mon Sep 17 00:00:00 2001
From: Nico Verbruggen <mail@nicoverbruggen.be>
Date: Sat, 29 Nov 2025 10:25:07 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20issue=20with=20concurrent?=
 =?UTF-8?q?=20output=20in=20RealShell.attach?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 phpmon/Common/Shell/RealShell.swift           | 39 +++++++++++--------
 phpmon/Common/Shell/ShellProtocol.swift       |  2 +-
 .../unit/Testables/Shell/RealShellTest.swift  | 39 +++++++++++++++++++
 3 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/phpmon/Common/Shell/RealShell.swift b/phpmon/Common/Shell/RealShell.swift
index 253be677..649475db 100644
--- a/phpmon/Common/Shell/RealShell.swift
+++ b/phpmon/Common/Shell/RealShell.swift
@@ -219,6 +219,7 @@ class RealShell: ShellProtocol {
         process.standardError = errorPipe
 
         let output = ShellOutput.empty()
+        let serialQueue = DispatchQueue(label: "com.nicoverbruggen.phpmon.shell_output")
 
         return try await withCheckedThrowingContinuation({ continuation in
             let timeoutTask = Task {
@@ -235,8 +236,10 @@ class RealShell: ShellProtocol {
             outputPipe.fileHandleForReading.readabilityHandler = { fileHandle in
                 let data = fileHandle.availableData
                 if !data.isEmpty, let string = String(data: data, encoding: .utf8) {
-                    output.out += string
-                    didReceiveOutput(string, .stdOut)
+                    serialQueue.async {
+                        output.out += string
+                        didReceiveOutput(string, .stdOut)
+                    }
                 }
             }
 
@@ -244,8 +247,10 @@ class RealShell: ShellProtocol {
             errorPipe.fileHandleForReading.readabilityHandler = { fileHandle in
                 let data = fileHandle.availableData
                 if !data.isEmpty, let string = String(data: data, encoding: .utf8) {
-                    output.err += string
-                    didReceiveOutput(string, .stdErr)
+                    serialQueue.async {
+                        output.err += string
+                        didReceiveOutput(string, .stdErr)
+                    }
                 }
             }
 
@@ -260,20 +265,22 @@ class RealShell: ShellProtocol {
                 let remainingOut = outputPipe.fileHandleForReading.readDataToEndOfFile()
                 let remainingErr = errorPipe.fileHandleForReading.readDataToEndOfFile()
 
-                if !remainingOut.isEmpty, let string = String(data: remainingOut, encoding: .utf8) {
-                    output.out += string
-                    didReceiveOutput(string, .stdOut)
-                }
+                serialQueue.async {
+                    if !remainingOut.isEmpty, let string = String(data: remainingOut, encoding: .utf8) {
+                        output.out += string
+                        didReceiveOutput(string, .stdOut)
+                    }
 
-                if !remainingErr.isEmpty, let string = String(data: remainingErr, encoding: .utf8) {
-                    output.err += string
-                    didReceiveOutput(string, .stdErr)
-                }
+                    if !remainingErr.isEmpty, let string = String(data: remainingErr, encoding: .utf8) {
+                        output.err += string
+                        didReceiveOutput(string, .stdErr)
+                    }
 
-                if !output.err.isEmpty {
-                    continuation.resume(returning: (process, .err(output.err)))
-                } else {
-                    continuation.resume(returning: (process, .out(output.out)))
+                    if !output.err.isEmpty {
+                        continuation.resume(returning: (process, .err(output.err)))
+                    } else {
+                        continuation.resume(returning: (process, .out(output.out)))
+                    }
                 }
             }
 
diff --git a/phpmon/Common/Shell/ShellProtocol.swift b/phpmon/Common/Shell/ShellProtocol.swift
index 39a8f7fd..0fa17f0b 100644
--- a/phpmon/Common/Shell/ShellProtocol.swift
+++ b/phpmon/Common/Shell/ShellProtocol.swift
@@ -67,7 +67,7 @@ enum ShellStream: Codable {
     case stdOut, stdErr, stdIn
 }
 
-class ShellOutput {
+class ShellOutput: @unchecked Sendable {
     var out: String
     var err: String
 
diff --git a/tests/unit/Testables/Shell/RealShellTest.swift b/tests/unit/Testables/Shell/RealShellTest.swift
index 38749c85..d7a80cf9 100644
--- a/tests/unit/Testables/Shell/RealShellTest.swift
+++ b/tests/unit/Testables/Shell/RealShellTest.swift
@@ -77,4 +77,43 @@ struct RealShellTest {
         let duration = start.duration(to: .now)
         #expect(duration < .milliseconds(2000)) // Should complete in ~700ms if parallel
     }
+
+    @Test func attach_handles_concurrent_stdout_stderr_writes_safely() async throws {
+        // This test verifies that concurrent writes to output.out and output.err
+        // from multiple readability handlers don't cause data races or crashes.
+        // Without the serial queue, rapid interleaved output causes undefined behavior.
+
+        let script = """
+        for i in {1..200}; do
+            echo "stdout-$i" >&1
+            echo "stderr-$i" >&2
+        done
+        """
+
+        var receivedChunks = 0
+        let (_, shellOutput) = try await container.shell.attach(
+            script,
+            didReceiveOutput: { _, _ in
+                receivedChunks += 1
+            },
+            withTimeout: 5.0
+        )
+
+        // Verify all output was captured without corruption
+        let stdoutLines = shellOutput.out
+            .components(separatedBy: "\n")
+            .filter { !$0.isEmpty }
+        let stderrLines = shellOutput.err
+            .components(separatedBy: "\n")
+            .filter { !$0.isEmpty }
+
+        #expect(stdoutLines.count == 200)
+        #expect(stderrLines.count == 200)
+
+        // Verify content integrity - each line should match the pattern
+        for i in 1...200 {
+            #expect(stdoutLines.contains("stdout-\(i)"))
+            #expect(stderrLines.contains("stderr-\(i)"))
+        }
+    }
 }