[Bugfix] Deadlock (#2)

.github/workflows/ci.yml

@@ -30,3 +30,12 @@ jobs:
 
       - name: build for release
         run: swift build -c release
+
+      - name: test 2
+        run: swift test
+
+      - name: test 3
+        run: swift test
+
+      - name: test 4
+        run: swift test

Sources/Sh/Data/PipeBuffer.swift

@@ -0,0 +1,40 @@
+import Foundation
+
+class PipeBuffer {
+  enum StreamID: String {
+    case stdOut, stdErr
+  }
+
+  let pipe = Pipe()
+  private var buffer: Data = .init()
+  private let queue: DispatchQueue
+
+  convenience init(id: StreamID) {
+    self.init(label: id.rawValue)
+  }
+
+  init(label: String) {
+    self.queue = DispatchQueue(label: label)
+    pipe.fileHandleForReading.readabilityHandler = { handler in
+      let nextData = handler.availableData
+      self.buffer.append(nextData)
+    }
+  }
+
+  func append(_ more: Data) {
+    queue.async {
+      self.buffer.append(contentsOf: more)
+    }
+  }
+
+  func yieldValue(block: @escaping (Data) -> Void) {
+    queue.sync {
+      let value = self.buffer
+      block(value)
+    }
+  }
+
+  var unsafeValue: Data {
+    buffer
+  }
+}

Sources/Sh/Errors.swift

@@ -1,14 +1,11 @@
 import Foundation
 
 public enum Errors: Error, LocalizedError {
-  case unexpectedNilDataError
   case errorWithLogInfo(String, underlyingError: Error)
   case openingLogError(Error, underlyingError: Error)
 
   public var errorDescription: String? {
     switch self {
-    case .unexpectedNilDataError:
-      return "Expected data, but there wasn't any"
     case .errorWithLogInfo(let logInfo, underlyingError: let underlyingError):
       return """
         An error occurred: \(underlyingError.localizedDescription)

Sources/Sh/Process/Process.runRedirectingAllOutput.swift

@@ -42,6 +42,8 @@ extension Process {
       self.redirectAllOutputToTerminal()
     case .file(path: let path):
       try self.redirectAllOutputToFile(path: path)
+    case .split(let out, err: let err):
+      try self.redirectAllOutputToFiles(out: out, err: err)
     case .null:
       self.redirectAllOutputToNullDevice()
     }
@@ -57,8 +59,7 @@ extension Process {
     self.standardError = FileHandle.nullDevice
   }
 
-  private func redirectAllOutputToFile(path: String) throws {
-
+  private func createFile(atPath path: String) throws -> FileHandle {
     guard FileManager.default.createFile(atPath: path, contents: Data()) else {
       struct CouldNotCreateFile: Error {
         let path: String
@@ -72,8 +73,17 @@ extension Process {
       }
       throw CouldNotOpenFileForWriting(path: path)
     }
-
+    return fileHandle
+  }
+
+  private func redirectAllOutputToFile(path: String) throws {
+    let fileHandle = try self.createFile(atPath: path)
     self.standardError = fileHandle
     self.standardOutput = fileHandle
   }
+
+  private func redirectAllOutputToFiles(out: String, err: String) throws {
+    self.standardOutput = try self.createFile(atPath: out)
+    self.standardError = try self.createFile(atPath: err)
+  }
 }

Sources/Sh/Process/Process.runReturningAllOutput.swift

@@ -2,44 +2,52 @@ import Foundation
 
 extension Process {
 
-  public typealias AllOutput = (stdOut: Data?, stdErr: Data?, terminationError: TerminationError?)
+  public typealias AllOutput = (stdOut: Data,
+                                stdErr: Data,
+                                terminationError: TerminationError?)
 
   public func runReturningAllOutput() throws -> AllOutput {
-    let stdOut = Pipe()
-    let stdErr = Pipe()
-    self.standardOutput = stdOut
-    self.standardError = stdErr
+
+    let stdOut = PipeBuffer(id: .stdOut)
+    self.standardOutput = stdOut.pipe
+
+    let stdErr = PipeBuffer(id: .stdErr)
+    self.standardError = stdErr.pipe
 
     try self.run()
     self.waitUntilExit()
 
-    let stdOutData = try stdOut.fileHandleForReading.readToEnd()
-    let stdErrData = try stdErr.fileHandleForReading.readToEnd()
-    return (stdOut: stdOutData, stdErr: stdErrData, terminationError: terminationError)
+    return (stdOut: stdOut.unsafeValue,
+            stdErr: stdErr.unsafeValue,
+            terminationError: terminationError)
   }
 
   public func runReturningAllOutput() async throws -> AllOutput {
-    try await withCheckedThrowingContinuation  { (continuation: CheckedContinuation<AllOutput, Error>) in
-      let stdOut = Pipe()
-      let stdErr = Pipe()
-      self.standardOutput = stdOut
-      self.standardError = stdErr
+
+    let stdOut = PipeBuffer(id: .stdOut)
+    self.standardOutput = stdOut.pipe
+
+    let stdErr = PipeBuffer(id: .stdErr)
+    self.standardError = stdErr.pipe
+
+    return try await withCheckedThrowingContinuation  { (continuation: CheckedContinuation<AllOutput, Error>) in
 
       self.terminationHandler = { process in
+        let maybeTerminationError = process.terminationError
 
-        do {
-          let stdOutData = try stdOut.fileHandleForReading.readToEnd()
-          let stdErrData = try stdErr.fileHandleForReading.readToEnd()
-          continuation.resume(with: .success((stdOutData, stdErrData, process.terminationError)))
-        } catch {
-          continuation.resume(with: .failure(error))
+        stdErr.yieldValue { stdErrData in
+          stdOut.yieldValue { stdOutData in
+            continuation.resume(returning: (stdOutData,
+                                            stdErrData,
+                                            maybeTerminationError))
+          }
         }
       }
 
       do {
         try self.run()
       } catch {
-        continuation.resume(with: .failure(error))
+        continuation.resume(throwing: error)
       }
     }
   }

Sources/Sh/Process/Process.runReturningData.swift

@@ -2,39 +2,36 @@ import Foundation
 
 extension Process {
 
-  public func runReturningData() throws -> Data? {
-    
-    let stdOut = Pipe()
-    self.standardOutput = stdOut
+  public func runReturningData() throws -> Data {
+
+    let stdOut = PipeBuffer(id: .stdOut)
+    self.standardOutput = stdOut.pipe
     self.standardError = FileHandle.standardError
 
     try self.run()
+
     self.waitUntilExit()
 
     if let terminationError = terminationError {
       throw terminationError
     } else {
-      return try stdOut.fileHandleForReading.readToEnd()
+      return stdOut.unsafeValue
     }
   }
-
-  public func runReturningData() async throws -> Data? {
 
-    try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Data?, Error>) in
-      let stdOut = Pipe()
-      self.standardOutput = stdOut
-      self.standardError = FileHandle.standardError
-
+  public func runReturningData() async throws -> Data {   
+    self.standardError = FileHandle.standardError
+
+    let stdOut = PipeBuffer(id: .stdOut)
+    self.standardOutput = stdOut.pipe
+
+    return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Data, Error>) in
       self.terminationHandler = { process in
-
         if let terminationError = process.terminationError {
           continuation.resume(throwing: terminationError)
         } else {
-          do {
-            let stdOutData = try stdOut.fileHandleForReading.readToEnd()
-            continuation.resume(with: .success(stdOutData))
-          } catch {
-            continuation.resume(with: .failure(error))
+          stdOut.yieldValue { data in
+            continuation.resume(returning: data)
           }
         }
       }

Sources/Sh/Process/Process.runReturningTrimmedString.swift

@@ -3,10 +3,10 @@ import Foundation
 public extension Process {
 
   func runReturningTrimmedString() throws -> String? {
-    try runReturningData()?.asTrimmedString()
+    try runReturningData().asTrimmedString()
   }
 
   func runReturningTrimmedString() async throws -> String? {
-    try await runReturningData()?.asTrimmedString()
+    try await runReturningData().asTrimmedString()
   }
 }

Sources/Sh/Sink.swift

@@ -2,5 +2,6 @@
 public enum Sink {
   case terminal /// Redirect output to the terminal
   case file(_ path: String) /// Redirect output to the file at the given path, creating if necessary.
+  case split(_ out: String, err: String) /// Redirect output and error streams to the files at the given paths, creating if necessary.
   case null /// The null device, also known as `/dev/null`
 }

Sources/Sh/sh.swift

@@ -89,14 +89,15 @@ public func sh(_ sink: Sink,
   switch sink {
   case .terminal:
     announce("Running `\(cmd)`")
-
     try shq(sink, cmd, environment: environment, workingDirectory: workingDirectory)
 
   case .null:
     announce("Running `\(cmd)`, discarding output")
-
     try shq(sink, cmd, environment: environment, workingDirectory: workingDirectory)
 
+  case .split(let out, let err):
+    announce("Running `\(cmd)`, output to `\(out.blue)`, error to `\(err.blue)`")
+    try shq(sink, cmd, environment: environment, workingDirectory: workingDirectory)
 
   case .file(let path):
     announce("Running `\(cmd)`, logging to `\(path.blue)`")
@@ -131,6 +132,8 @@ public func sh(_ sink: Sink,
     await announce("Running `\(cmd)`")
   case .file(let path):
     await announce("Running `\(cmd)`, logging to `\(path.blue)`")
+  case .split(let out, let err):
+    await announce("Running `\(cmd)`, output to `\(out.blue)`, error to `\(err.blue)`")
   case .null:
     await announce("Running `\(cmd)`, discarding output")
   }

Sources/Sh/shq.swift

@@ -25,15 +25,16 @@ public func shq(_ cmd: String,
                 environment: [String: String] = [:],
                 workingDirectory: String? = nil) throws -> String?  {
   return try Process(cmd: cmd, environment: environment, workingDirectory: workingDirectory)
-    .runReturningData()?
+    .runReturningData()
     .asTrimmedString()
 }
 
+/// async version of the method with the same signature
 public func shq(_ cmd: String,
                 environment: [String: String] = [:],
                 workingDirectory: String? = nil) async throws -> String?  {
   return try await Process(cmd: cmd, environment: environment, workingDirectory: workingDirectory)
-    .runReturningData()?
+    .runReturningData()
     .asTrimmedString()
 }
 
@@ -45,15 +46,9 @@ public func shq<D: Decodable>(_ type: D.Type,
                               _ cmd: String,
                              environment: [String: String] = [:],
                              workingDirectory: String? = nil) throws -> D {
-  let decoded = try Process(cmd: cmd, environment: environment, workingDirectory: workingDirectory)
-    .runReturningData()?
+  try Process(cmd: cmd, environment: environment, workingDirectory: workingDirectory)
+    .runReturningData()
     .asJSON(decoding: type, using: jsonDecoder)
-
-  if let decoded = decoded {
-    return decoded
-  } else {
-    throw Errors.unexpectedNilDataError
-  }
 }
 
 /// Asynchronously, run a shell command, and parse the output as JSON
@@ -63,15 +58,11 @@ public func shq<D: Decodable>(_ type: D.Type,
                               _ cmd: String,
                              environment: [String: String] = [:],
                              workingDirectory: String? = nil) async throws -> D {
-  let decoded = try await Process(cmd: cmd, environment: environment, workingDirectory: workingDirectory)
-    .runReturningData()?
-    .asJSON(decoding: type, using: jsonDecoder)
-
-  if let decoded = decoded {
-    return decoded
-  } else {
-    throw Errors.unexpectedNilDataError
-  }
+  try await Process(cmd: cmd,
+                    environment: environment,
+                    workingDirectory: workingDirectory)
+  .runReturningData()
+  .asJSON(decoding: type, using: jsonDecoder)
 }
 
 /// Run a shell command, sending output to the terminal or a file.

Tests/ShTests/LogFileTests.swift

@@ -1,10 +1,20 @@
 import XCTest
+import Foundation
 @testable import Sh
 
 final class LogFileTests: XCTestCase {
-
+  
   func testSimple() throws {
+    try sh(.file("/tmp/sh-test.log"), #"echo "simple""#)
+    XCTAssertEqual(try String(contentsOfFile: "/tmp/sh-test.log"), "simple\n")
+  }
+
+  func testSimpleAsync() async throws {
+    try await sh(.file("/tmp/sh-test.log"), #"echo "simple""#)
+    XCTAssertEqual(try String(contentsOfFile: "/tmp/sh-test.log"), "simple\n")
+  }
 
+  func testError() throws {
     do {
       try sh(.file("/tmp/sh-test.log"), #"echo "simple" > /unknown/path/name"#)
       XCTFail("Expected the above to throw an `Errors.errorWithLogInfo`")
@@ -20,6 +30,7 @@ final class LogFileTests: XCTestCase {
       XCTFail("Expected the above to throw an `Errors.errorWithLogInfo`, instead got an \(error)")
     }
   }
+
 
   func testUnwritableLogfile() throws {
     do {