Fix RunningProcess potentially hanging

Author: rockbruno

Sources/SourceKitBazelBSP/SharedUtils/Shell/RunningProcess.swift

@@ -36,65 +36,64 @@ extension Process: CommandLineProcess {
     }
 }
 
-public struct RunningProcess: Sendable {
+public final class RunningProcess: Sendable {
     let cmd: String
     let stdout: Pipe
     let stderr: Pipe
     let wrappedProcess: CommandLineProcess
 
+    private nonisolated(unsafe) var stdoutData: Data = Data()
+    private nonisolated(unsafe) var stderrData: Data = Data()
+    private let group = DispatchGroup()
+
     public init(cmd: String, stdout: Pipe, stderr: Pipe, wrappedProcess: CommandLineProcess) {
         self.cmd = cmd
         self.stdout = stdout
         self.stderr = stderr
         self.wrappedProcess = wrappedProcess
     }
 
-    public func result<T: DataConvertible>() throws -> T {
-        let (stdoutData, stderrString): (T, String) = self.outputs()
-
-        guard wrappedProcess.terminationStatus == 0 else {
-            logger.debug("Command failed: \(cmd)")
-            throw ShellCommandRunnerError.failed(cmd, stderrString)
-        }
-
-        return stdoutData
-    }
-
-    public func outputs<T: DataConvertible>() -> (T, String) {
-        nonisolated(unsafe) var stdoutData: Data = Data()
-        nonisolated(unsafe) var stderrData: Data = Data()
-        let group = DispatchGroup()
-
+    public func attachPipes() {
         // We need to read the pipes continuously to avoid hitting buffer limits.
         // See https://github.com/spotify/sourcekit-bazel-bsp/pull/65
         group.enter()
-        stdout.fileHandleForReading.readabilityHandler = { stdoutFileHandle in
+        stdout.fileHandleForReading.readabilityHandler = { [weak self] stdoutFileHandle in
             let tmpstdoutData = stdoutFileHandle.availableData
             if tmpstdoutData.isEmpty {  // EOF
-                stdout.fileHandleForReading.readabilityHandler = nil
-                group.leave()
+                self?.stdout.fileHandleForReading.readabilityHandler = nil
+                self?.group.leave()
             } else {
-                stdoutData.append(tmpstdoutData)
+                self?.stdoutData.append(tmpstdoutData)
             }
         }
-
         group.enter()
-        stderr.fileHandleForReading.readabilityHandler = { stderrFileHandle in
+        stderr.fileHandleForReading.readabilityHandler = { [weak self] stderrFileHandle in
             let tmpstderrData = stderrFileHandle.availableData
             if tmpstderrData.isEmpty {  // EOF
-                stderr.fileHandleForReading.readabilityHandler = nil
-                group.leave()
+                self?.stderr.fileHandleForReading.readabilityHandler = nil
+                self?.group.leave()
             } else {
-                stderrData.append(tmpstderrData)
+                self?.stderrData.append(tmpstderrData)
             }
         }
+    }
+
+    public func result<T: DataConvertible>() throws -> T {
+        let (stdoutData, stderrString): (T, String) = self.outputs()
+
+        guard wrappedProcess.terminationStatus == 0 else {
+            logger.debug("Command failed: \(self.cmd)")
+            throw ShellCommandRunnerError.failed(cmd, stderrString)
+        }
 
-        wrappedProcess.waitUntilExit()
-        group.wait()
+        return stdoutData
+    }
 
+    public func outputs<T: DataConvertible>() -> (T, String) {
+        group.wait()
+        wrappedProcess.waitUntilExit()
         let stdoutResult = T.convert(from: stdoutData)
         let stderrResult = String(data: stderrData, encoding: .utf8) ?? "(no stderr)"
-
         return (stdoutResult, stderrResult)
     }
 

Sources/SourceKitBazelBSP/SharedUtils/Shell/ShellCommandRunner.swift

@@ -46,16 +46,18 @@ struct ShellCommandRunner: CommandRunner {
         process.arguments = ["-c", cmd]
         process.standardInput = nil
 
-        logger.info("Running shell: \(cmd)")
-        try process.run()
-
         let runningProcess = RunningProcess(
             cmd: cmd,
             stdout: stdout,
             stderr: stderr,
             wrappedProcess: process
         )
 
+        runningProcess.attachPipes()
+
+        logger.info("Running shell: \(cmd)")
+        try process.run()
+
         return runningProcess
     }
 }

Tests/SourceKitBazelBSPTests/Fakes/CommandRunnerFake.swift

@@ -54,10 +54,13 @@ final class CommandRunnerFake: CommandRunner, @unchecked Sendable {
         guard let response = responses[cacheKey] else {
             throw CommandRunnerFakeError.unregisteredCommand(cmd, cwd)
         }
+        let procFake = CommandLineProcessFake()
+        let runningProc = RunningProcess(cmd: cmd, stdout: stdout, stderr: stderr, wrappedProcess: procFake)
+        runningProc.attachPipes()
         stdout.fileHandleForWriting.write(response)
         stdout.fileHandleForWriting.closeFile()
         stderr.fileHandleForWriting.closeFile()
-        return RunningProcess(cmd: cmd, stdout: stdout, stderr: stderr, wrappedProcess: CommandLineProcessFake())
+        return runningProc
     }
 
     private func key(for command: String, cwd: String?) -> String { return command + "|" + (cwd ?? "nil") }