Skip to content

Commit f40a368

Browse files
authored
Very rarely the last stderr output from a plugin subprocess could get lost (#4236)
This could happen because the Process termination handler only called `availableData` and not `readToEnd()`, so based on timing, there might be data that had been emitted by the plugin subprocess before it exited but that hadn't yet been received. This caused sporadic failures of the `testLocalAndRemoteToolDependencies` unit test. The fix is to switch to using `readToEnd()`. This was not a problem for messages from the plugin, because they were already being read (and not collected using `availableData`). rdar://89490777
1 parent e29c65d commit f40a368

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

Sources/Workspace/DefaultPluginScriptRunner.swift

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -446,14 +446,16 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
446446
let stderrPipe = Pipe()
447447
let stderrLock = Lock()
448448
var stderrData = Data()
449-
stderrPipe.fileHandleForReading.readabilityHandler = { fileHandle in
449+
let stderrHandler = { (data: Data) in
450450
// Pass on any available data to the delegate.
451-
stderrLock.withLock {
452-
let data = fileHandle.availableData
453-
if data.isEmpty { return }
454-
stderrData.append(contentsOf: data)
455-
callbackQueue.async { delegate.handleOutput(data: data) }
456-
}
451+
if data.isEmpty { return }
452+
stderrData.append(contentsOf: data)
453+
callbackQueue.async { delegate.handleOutput(data: data) }
454+
}
455+
stderrPipe.fileHandleForReading.readabilityHandler = { fileHandle in
456+
// Read and pass on any available free-form text output from the plugin.
457+
// We need the lock since we could run concurrently with the termination handler.
458+
stderrLock.withLock { stderrHandler(fileHandle.availableData) }
457459
}
458460
process.standardError = stderrPipe
459461

@@ -473,7 +475,10 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
473475
try? outputHandle.close()
474476

475477
// Read and pass on any remaining free-form text output from the plugin.
476-
stderrPipe.fileHandleForReading.readabilityHandler?(stderrPipe.fileHandleForReading)
478+
// We need the lock since we could run concurrently with the readability handler.
479+
stderrLock.withLock {
480+
try? stderrPipe.fileHandleForReading.readToEnd().map{ stderrHandler($0) }
481+
}
477482

478483
// Read and pass on any remaining messages from the plugin.
479484
stdoutPipe.fileHandleForReading.readabilityHandler?(stdoutPipe.fileHandleForReading)

0 commit comments

Comments
 (0)