Skip to content

Commit f15e03d

Browse files
committed
Refactor captureOutput() to minimize force unwrap
1 parent 00a96c9 commit f15e03d

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

Sources/Subprocess/IO/AsyncIO.swift

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,14 @@ extension AsyncIO {
352352
)
353353
var readLength: Int = 0
354354
let signalStream = self.registerFileDescriptor(fileDescriptor, for: .read)
355+
/// Outer loop: every iteration signals we are ready to read more data
355356
for try await _ in signalStream {
356-
// Every iteration signals we are ready to read more data
357+
/// Inner loop: repeatedly call `.read()` and read more data until:
358+
/// 1. We reached EOF (read length is 0), in which case return the result
359+
/// 2. We read `maxLength` bytes, in which case return the result
360+
/// 3. `read()` returns -1 and sets `errno` to `EAGAIN` or `EWOULDBLOCK`. In
361+
/// this case we `break` out of the inner loop and wait `.read()` to be
362+
/// ready by `await`ing the next signal in the outer loop.
357363
while true {
358364
let bytesRead = resultBuffer.withUnsafeMutableBufferPointer { bufferPointer in
359365
// Get a pointer to the memory at the specified offset
@@ -417,7 +423,13 @@ extension AsyncIO {
417423
let fileDescriptor = diskIO.fileDescriptor
418424
let signalStream = self.registerFileDescriptor(fileDescriptor, for: .write)
419425
var writtenLength: Int = 0
426+
/// Outer loop: every iteration signals we are ready to read more data
420427
for try await _ in signalStream {
428+
/// Inner loop: repeatedly call `.write()` and write more data until:
429+
/// 1. We've written bytes.count bytes.
430+
/// 3. `.write()` returns -1 and sets `errno` to `EAGAIN` or `EWOULDBLOCK`. In
431+
/// this case we `break` out of the inner loop and wait `.write()` to be
432+
/// ready by `await`ing the next signal in the outer loop.
421433
while true {
422434
let written = bytes.withUnsafeBytes { ptr in
423435
let remainingLength = ptr.count - writtenLength
@@ -454,7 +466,13 @@ extension AsyncIO {
454466
let fileDescriptor = diskIO.fileDescriptor
455467
let signalStream = self.registerFileDescriptor(fileDescriptor, for: .write)
456468
var writtenLength: Int = 0
469+
/// Outer loop: every iteration signals we are ready to read more data
457470
for try await _ in signalStream {
471+
/// Inner loop: repeatedly call `.write()` and write more data until:
472+
/// 1. We've written bytes.count bytes.
473+
/// 3. `.write()` returns -1 and sets `errno` to `EAGAIN` or `EWOULDBLOCK`. In
474+
/// this case we `break` out of the inner loop and wait `.write()` to be
475+
/// ready by `await`ing the next signal in the outer loop.
458476
while true {
459477
let written = span.withUnsafeBytes { ptr in
460478
let remainingLength = ptr.count - writtenLength

Sources/Subprocess/IO/Output.swift

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,10 @@ public struct BytesOutput: OutputProtocol {
140140
public let maxSize: Int
141141

142142
internal func captureOutput(
143-
from diskIO: consuming TrackedPlatformDiskIO?
143+
from diskIO: consuming TrackedPlatformDiskIO
144144
) async throws -> [UInt8] {
145-
let result = try await AsyncIO.shared.read(from: diskIO!, upTo: self.maxSize)
146-
try diskIO?.safelyClose()
145+
let result = try await AsyncIO.shared.read(from: diskIO, upTo: self.maxSize)
146+
try diskIO.safelyClose()
147147
#if canImport(Darwin)
148148
return result?.array() ?? []
149149
#else
@@ -255,17 +255,26 @@ extension OutputProtocol {
255255
internal func captureOutput(
256256
from diskIO: consuming TrackedPlatformDiskIO?
257257
) async throws -> OutputType {
258-
if let bytesOutput = self as? BytesOutput {
259-
return try await bytesOutput.captureOutput(from: diskIO) as! Self.OutputType
260-
}
261-
262258
if OutputType.self == Void.self {
263259
return () as! OutputType
264260
}
261+
// `diskIO` is only `nil` for any types that conform to `OutputProtocol`
262+
// and have `Void` as ``OutputType` (i.e. `DiscardedOutput`). Since we
263+
// made sure `OutputType` is not `Void` on the line above, `diskIO`
264+
// must not be nil; otherwise, this is a programmer error.
265+
guard var diskIO else {
266+
fatalError(
267+
"Internal Inconsistency Error: diskIO must not be nil when OutputType is not Void"
268+
)
269+
}
270+
271+
if let bytesOutput = self as? BytesOutput {
272+
return try await bytesOutput.captureOutput(from: diskIO) as! Self.OutputType
273+
}
265274
// Force unwrap is safe here because only `OutputType.self == Void` would
266275
// have nil `TrackedPlatformDiskIO`
267-
let result = try await AsyncIO.shared.read(from: diskIO!, upTo: self.maxSize)
268-
try diskIO?.safelyClose()
276+
let result = try await AsyncIO.shared.read(from: diskIO, upTo: self.maxSize)
277+
try diskIO.safelyClose()
269278
#if canImport(Darwin)
270279
return try self.output(from: result ?? .empty)
271280
#else

0 commit comments

Comments
 (0)