Update AsyncProcess, use it in Shell implementation (#77)

These changes make `AsyncProcess` implementation more robust and increase its code coverage. Now we no longer have to rely on `Foundation.Process` directly in `Shell`, as `AsyncProcess` abstracts rough edges away for us and also provides an `async`-friendly API. --------- Co-authored-by: Johannes Weiss <[email protected]> Co-authored-by: Euan Harris <[email protected]>
swiftlang · Jan 25, 2024 · cf9e76a · cf9e76a
1 parent 6e55a2f
commit cf9e76a
Show file tree

Hide file tree

Showing 14 changed files with 972 additions and 302 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -41,8 +41,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-collections.git",
       "state" : {
-        "revision" : "937e904258d22af6e447a0b72c0bc67583ef64a2",
-        "version" : "1.0.4"
+        "revision" : "d029d9d39c87bed85b1c50adee7c41795261a192",
+        "version" : "1.0.6"
       }
     },
     {
@@ -54,6 +54,15 @@
         "version" : "3.1.0"
       }
     },
+    {
+      "identity" : "swift-http-types",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/apple/swift-http-types",
+      "state" : {
+        "revision" : "12358d55a3824bd5fed310b999ea8cf83a9a1a65",
+        "version" : "1.0.3"
+      }
+    },
     {
       "identity" : "swift-log",
       "kind" : "remoteSourceControl",
@@ -68,26 +77,26 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-nio.git",
       "state" : {
-        "revision" : "3db5c4aeee8100d2db6f1eaf3864afdad5dc68fd",
-        "version" : "2.59.0"
+        "revision" : "635b2589494c97e48c62514bc8b37ced762e0a62",
+        "version" : "2.63.0"
       }
     },
     {
       "identity" : "swift-nio-extras",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-nio-extras.git",
       "state" : {
-        "revision" : "0e0d0aab665ff1a0659ce75ac003081f2b1c8997",
-        "version" : "1.19.0"
+        "revision" : "363da63c1966405764f380c627409b2f9d9e710b",
+        "version" : "1.21.0"
       }
     },
     {
       "identity" : "swift-nio-http2",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-nio-http2.git",
       "state" : {
-        "revision" : "860622124b01cc1863b4e65dc449b6b457ca5704",
-        "version" : "1.25.1"
+        "revision" : "0904bf0feb5122b7e5c3f15db7df0eabe623dd87",
+        "version" : "1.30.0"
       }
     },
     {

diff --git a/Package.swift b/Package.swift
@@ -21,10 +21,10 @@ let package = Package(
     .package(url: "https://github.com/apple/swift-argument-parser", from: "1.2.2"),
     .package(url: "https://github.com/apple/swift-async-algorithms.git", exact: "1.0.0-beta.1"),
     .package(url: "https://github.com/apple/swift-atomics.git", from: "1.1.0"),
-    .package(url: "https://github.com/apple/swift-collections.git", from: "1.0.4"),
+    .package(url: "https://github.com/apple/swift-collections.git", from: "1.0.5"),
     .package(url: "https://github.com/apple/swift-crypto.git", from: "3.1.0"),
-    .package(url: "https://github.com/apple/swift-nio.git", from: "2.58.0"),
-    .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.19.0"),
+    .package(url: "https://github.com/apple/swift-nio.git", from: "2.63.0"),
+    .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.20.0"),
     .package(url: "https://github.com/apple/swift-log.git", from: "1.5.3"),
     .package(url: "https://github.com/swift-server/swift-service-lifecycle.git", from: "2.3.0"),
     .package(url: "https://github.com/apple/swift-syntax.git", from: "509.0.2"),

diff --git a/Sources/AsyncProcess/FileContentStream.swift b/Sources/AsyncProcess/FileContentStream.swift
@@ -67,10 +67,13 @@ struct FileContentStream: AsyncSequence {
       guard let blockingPool else {
         throw IOError(errnoValue: EINVAL)
       }
-      let fileHandle = NIOFileHandle(descriptor: dupedFD)
+      let fileHandle = NIOLoopBound(
+        NIOFileHandle(descriptor: dupedFD),
+        eventLoop: eventLoop
+      )
       NonBlockingFileIO(threadPool: blockingPool)
         .readChunked(
-          fileHandle: fileHandle,
+          fileHandle: fileHandle.value,
           byteCount: .max,
           allocator: ByteBufferAllocator(),
           eventLoop: eventLoop,
@@ -81,7 +84,7 @@ struct FileContentStream: AsyncSequence {
           }
         )
         .whenComplete { result in
-          try! fileHandle.close()
+          try! fileHandle.value.close()
           switch result {
           case let .failure(error):
             asyncChannel.fail(error)
@@ -96,20 +99,16 @@ struct FileContentStream: AsyncSequence {
         }
         .withConnectedSocket(dupedFD)
     case S_IFIFO:
-      let deadPipe = Pipe()
       NIOPipeBootstrap(group: eventLoop)
         .channelInitializer { channel in
           channel.pipeline.addHandler(ReadIntoAsyncChannelHandler(sink: asyncChannel))
         }
-        .takingOwnershipOfDescriptors(
-          input: dupedFD,
-          output: dup(deadPipe.fileHandleForWriting.fileDescriptor)
+        .takingOwnershipOfDescriptor(
+          input: dupedFD
         )
         .whenSuccess { channel in
           channel.close(mode: .output, promise: nil)
         }
-      try! deadPipe.fileHandleForReading.close()
-      try! deadPipe.fileHandleForWriting.close()
     case S_IFDIR:
       throw IOError(errnoValue: EISDIR)
     case S_IFBLK, S_IFCHR, S_IFLNK:
@@ -206,25 +205,30 @@ private final class ReadIntoAsyncChannelHandler: ChannelDuplexHandler {
   private func sendOneItem(_ data: ReceivedEvent, context: ChannelHandlerContext) {
     context.eventLoop.assertInEventLoop()
     assert(self.shouldRead == false, "sendOneItem in unexpected state \(self.state)")
-    context.eventLoop.makeFutureWithTask {
+    let eventLoop = context.eventLoop
+    let sink = self.sink
+    let `self` = NIOLoopBound(self, eventLoop: context.eventLoop)
+    let context = NIOLoopBound(context, eventLoop: context.eventLoop)
+    eventLoop.makeFutureWithTask {
+      // note: We're _not_ on an EventLoop thread here
       switch data {
       case let .chunk(data):
-        await self.sink.send(data)
+        await sink.send(data)
       case .finish:
-        self.sink.finish()
+        sink.finish()
       }
     }.map {
-      if let moreToSend = self.state.didSendOne() {
-        self.sendOneItem(moreToSend, context: context)
+      if let moreToSend = self.value.state.didSendOne() {
+        self.value.sendOneItem(moreToSend, context: context.value)
       } else {
-        if self.heldUpRead {
-          context.eventLoop.execute {
-            context.read()
+        if self.value.heldUpRead {
+          eventLoop.execute {
+            context.value.read()
           }
         }
       }
     }.whenFailure { error in
-      self.state.fail(error)
+      self.value.state.fail(error)
     }
   }
 
@@ -268,7 +272,7 @@ extension FileContentStream {
   }
 }
 
-public extension AsyncSequence where Element == ByteBuffer {
+public extension AsyncSequence where Element == ByteBuffer, Self: Sendable {
   func splitIntoLines(
     dropTerminator: Bool = true,
     maximumAllowableBufferSize: Int = 1024 * 1024,

diff --git a/Sources/AsyncProcess/NIOAsyncPipeWriter.swift b/Sources/AsyncProcess/NIOAsyncPipeWriter.swift
@@ -17,22 +17,16 @@ import NIOExtras
 struct NIOAsyncPipeWriter<Chunks: AsyncSequence & Sendable> where Chunks.Element == ByteBuffer {
   static func sinkSequenceInto(
     _ chunks: Chunks,
-    fileDescriptor fd: CInt,
+    takingOwnershipOfFD fd: CInt,
     eventLoop: EventLoop
   ) async throws {
-    // Just so we've got an input.
-    // (workaround for https://github.com/apple/swift-nio/issues/2444)
-    let deadPipe = Pipe()
     let channel = try await NIOPipeBootstrap(group: eventLoop)
       .channelOption(ChannelOptions.allowRemoteHalfClosure, value: true)
       .channelOption(ChannelOptions.autoRead, value: false)
-      .takingOwnershipOfDescriptors(
-        input: dup(deadPipe.fileHandleForReading.fileDescriptor),
-        output: dup(fd)
+      .takingOwnershipOfDescriptor(
+        output: fd
       ).get()
     channel.close(mode: .input, promise: nil)
-    try! deadPipe.fileHandleForReading.close()
-    try! deadPipe.fileHandleForWriting.close()
     defer {
       channel.close(promise: nil)
     }