1
0
mirror of https://github.com/nicoverbruggen/phpmon.git synced 2025-12-21 03:10:06 +01:00

🐛 Fix issue with concurrent output in RealShell.attach

This commit is contained in:
2025-11-29 10:25:07 +01:00
parent 28a73f9273
commit ccc6aaf06d
3 changed files with 63 additions and 17 deletions

View File

@@ -219,6 +219,7 @@ class RealShell: ShellProtocol {
process.standardError = errorPipe process.standardError = errorPipe
let output = ShellOutput.empty() let output = ShellOutput.empty()
let serialQueue = DispatchQueue(label: "com.nicoverbruggen.phpmon.shell_output")
return try await withCheckedThrowingContinuation({ continuation in return try await withCheckedThrowingContinuation({ continuation in
let timeoutTask = Task { let timeoutTask = Task {
@@ -235,19 +236,23 @@ class RealShell: ShellProtocol {
outputPipe.fileHandleForReading.readabilityHandler = { fileHandle in outputPipe.fileHandleForReading.readabilityHandler = { fileHandle in
let data = fileHandle.availableData let data = fileHandle.availableData
if !data.isEmpty, let string = String(data: data, encoding: .utf8) { if !data.isEmpty, let string = String(data: data, encoding: .utf8) {
serialQueue.async {
output.out += string output.out += string
didReceiveOutput(string, .stdOut) didReceiveOutput(string, .stdOut)
} }
} }
}
// Set up background reading for stderr // Set up background reading for stderr
errorPipe.fileHandleForReading.readabilityHandler = { fileHandle in errorPipe.fileHandleForReading.readabilityHandler = { fileHandle in
let data = fileHandle.availableData let data = fileHandle.availableData
if !data.isEmpty, let string = String(data: data, encoding: .utf8) { if !data.isEmpty, let string = String(data: data, encoding: .utf8) {
serialQueue.async {
output.err += string output.err += string
didReceiveOutput(string, .stdErr) didReceiveOutput(string, .stdErr)
} }
} }
}
process.terminationHandler = { process in process.terminationHandler = { process in
timeoutTask.cancel() timeoutTask.cancel()
@@ -260,6 +265,7 @@ class RealShell: ShellProtocol {
let remainingOut = outputPipe.fileHandleForReading.readDataToEndOfFile() let remainingOut = outputPipe.fileHandleForReading.readDataToEndOfFile()
let remainingErr = errorPipe.fileHandleForReading.readDataToEndOfFile() let remainingErr = errorPipe.fileHandleForReading.readDataToEndOfFile()
serialQueue.async {
if !remainingOut.isEmpty, let string = String(data: remainingOut, encoding: .utf8) { if !remainingOut.isEmpty, let string = String(data: remainingOut, encoding: .utf8) {
output.out += string output.out += string
didReceiveOutput(string, .stdOut) didReceiveOutput(string, .stdOut)
@@ -276,6 +282,7 @@ class RealShell: ShellProtocol {
continuation.resume(returning: (process, .out(output.out))) continuation.resume(returning: (process, .out(output.out)))
} }
} }
}
process.launch() process.launch()
}) })

View File

@@ -67,7 +67,7 @@ enum ShellStream: Codable {
case stdOut, stdErr, stdIn case stdOut, stdErr, stdIn
} }
class ShellOutput { class ShellOutput: @unchecked Sendable {
var out: String var out: String
var err: String var err: String

View File

@@ -77,4 +77,43 @@ struct RealShellTest {
let duration = start.duration(to: .now) let duration = start.duration(to: .now)
#expect(duration < .milliseconds(2000)) // Should complete in ~700ms if parallel #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)"))
}
}
} }