Unbreak CI (swift-server#800)

Lukasa · web-flow · commit 126518507b86 · 2025-01-14T16:34:21.000Z
# Motivation

The NIO 2.78 release introduced a bunch of new warnings. These warnings
cause us a bunch of trouble, so we should fix them.

# Modifications

Mostly use a bunch of assumeIsolated() and syncOperations.

# Result 

CI passes again.

Note that Swift 6 has _many_ more warnings than this, but we expect more
to come and we aren't using warnings-as-errors on that mode at the
moment. We'll be cleaning that up soon.
diff --git a/Package.swift b/Package.swift
@@ -21,7 +21,7 @@ let package = Package(
         .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"])
     ],
     dependencies: [
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.71.0"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.78.0"),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.27.1"),
         .package(url: "https://github.com/apple/swift-nio-http2.git", from: "1.19.0"),
         .package(url: "https://github.com/apple/swift-nio-extras.git", from: "1.13.0"),
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift
@@ -249,15 +249,15 @@ extension HTTPConnectionPool.ConnectionFactory {
 
             // The proxyEstablishedFuture is set as soon as the HTTP1ProxyConnectHandler is in a
             // pipeline. It is created in HTTP1ProxyConnectHandler's handlerAdded method.
-            return proxyHandler.proxyEstablishedFuture!.flatMap {
-                channel.pipeline.removeHandler(proxyHandler).flatMap {
-                    channel.pipeline.removeHandler(decoder).flatMap {
-                        channel.pipeline.removeHandler(encoder)
-                    }
-                }
+            return proxyHandler.proxyEstablishedFuture!.assumeIsolated().flatMap {
+                channel.pipeline.syncOperations.removeHandler(proxyHandler).assumeIsolated().flatMap {
+                    channel.pipeline.syncOperations.removeHandler(decoder).assumeIsolated().flatMap {
+                        channel.pipeline.syncOperations.removeHandler(encoder)
+                    }.nonisolated()
+                }.nonisolated()
             }.flatMap {
                 self.setupTLSInProxyConnectionIfNeeded(channel, deadline: deadline, logger: logger)
-            }
+            }.nonisolated()
         }
     }
 
@@ -291,13 +291,13 @@ extension HTTPConnectionPool.ConnectionFactory {
 
             // The socksEstablishedFuture is set as soon as the SOCKSEventsHandler is in a
             // pipeline. It is created in SOCKSEventsHandler's handlerAdded method.
-            return socksEventHandler.socksEstablishedFuture!.flatMap {
-                channel.pipeline.removeHandler(socksEventHandler).flatMap {
-                    channel.pipeline.removeHandler(socksConnectHandler)
-                }
+            return socksEventHandler.socksEstablishedFuture!.assumeIsolated().flatMap {
+                channel.pipeline.syncOperations.removeHandler(socksEventHandler).assumeIsolated().flatMap {
+                    channel.pipeline.syncOperations.removeHandler(socksConnectHandler)
+                }.nonisolated()
             }.flatMap {
                 self.setupTLSInProxyConnectionIfNeeded(channel, deadline: deadline, logger: logger)
-            }
+            }.nonisolated()
         }
     }
 
diff --git a/Sources/AsyncHTTPClient/FileDownloadDelegate.swift b/Sources/AsyncHTTPClient/FileDownloadDelegate.swift
@@ -167,7 +167,7 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
             }
         } else {
             let fileHandleFuture = io.openFile(
-                path: self.filePath,
+                _deprecatedPath: self.filePath,
                 mode: .write,
                 flags: .allowFileCreation(),
                 eventLoop: task.eventLoop
diff --git a/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift b/Tests/AsyncHTTPClientTests/AsyncAwaitEndToEndTests.swift
@@ -595,7 +595,9 @@ final class AsyncAwaitEndToEndTests: XCTestCase {
             defer { XCTAssertNoThrow(try serverGroup.syncShutdownGracefully()) }
             let server = ServerBootstrap(group: serverGroup)
                 .childChannelInitializer { channel in
-                    channel.pipeline.addHandler(NIOSSLServerHandler(context: sslContext))
+                    channel.eventLoop.makeCompletedFuture {
+                        try channel.pipeline.syncOperations.addHandler(NIOSSLServerHandler(context: sslContext))
+                    }
                 }
             let serverChannel = try await server.bind(host: "localhost", port: 0).get()
             defer { XCTAssertNoThrow(try serverChannel.close().wait()) }
diff --git a/Tests/AsyncHTTPClientTests/EmbeddedChannel+HTTPConvenience.swift b/Tests/AsyncHTTPClientTests/EmbeddedChannel+HTTPConvenience.swift
@@ -87,8 +87,8 @@ extension EmbeddedChannel {
         let decoder = try self.pipeline.syncOperations.handler(type: ByteToMessageHandler<HTTPResponseDecoder>.self)
         let encoder = try self.pipeline.syncOperations.handler(type: HTTPRequestEncoder.self)
 
-        let removeDecoderFuture = self.pipeline.removeHandler(decoder)
-        let removeEncoderFuture = self.pipeline.removeHandler(encoder)
+        let removeDecoderFuture = self.pipeline.syncOperations.removeHandler(decoder)
+        let removeEncoderFuture = self.pipeline.syncOperations.removeHandler(encoder)
 
         self.embeddedEventLoop.run()
 
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift b/Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
@@ -544,12 +544,12 @@ where
         try sync.addHandler(requestDecoder)
         try sync.addHandler(proxySimulator)
 
-        promise.futureResult.flatMap { _ in
-            channel.pipeline.removeHandler(proxySimulator)
+        promise.futureResult.assumeIsolated().flatMap { _ in
+            channel.pipeline.syncOperations.removeHandler(proxySimulator)
         }.flatMap { _ in
-            channel.pipeline.removeHandler(responseEncoder)
+            channel.pipeline.syncOperations.removeHandler(responseEncoder)
         }.flatMap { _ in
-            channel.pipeline.removeHandler(requestDecoder)
+            channel.pipeline.syncOperations.removeHandler(requestDecoder)
         }.whenComplete { result in
             switch result {
             case .failure:
@@ -653,8 +653,8 @@ where
             }
         }
 
+        try channel.pipeline.syncOperations.addHandler(sslHandler)
         try channel.pipeline.syncOperations.addHandler(alpnHandler)
-        try channel.pipeline.syncOperations.addHandler(sslHandler, position: .before(alpnHandler))
     }
 
     func shutdown() throws {
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift
@@ -1600,7 +1600,9 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass {
 
         let server = ServerBootstrap(group: serverGroup)
             .childChannelInitializer { channel in
-                channel.pipeline.addHandler(NIOSSLServerHandler(context: sslContext))
+                channel.eventLoop.makeCompletedFuture {
+                    try channel.pipeline.syncOperations.addHandler(NIOSSLServerHandler(context: sslContext))
+                }
             }
         let serverChannel = try server.bind(host: "localhost", port: 0).wait()
         defer { XCTAssertNoThrow(try serverChannel.close().wait()) }
@@ -1642,7 +1644,9 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass {
 
         let server = ServerBootstrap(group: serverGroup)
             .childChannelInitializer { channel in
-                channel.pipeline.addHandler(NIOSSLServerHandler(context: sslContext))
+                channel.eventLoop.makeCompletedFuture {
+                    try channel.pipeline.syncOperations.addHandler(NIOSSLServerHandler(context: sslContext))
+                }
             }
         let serverChannel = try server.bind(host: "localhost", port: 0).wait()
         defer { XCTAssertNoThrow(try serverChannel.close().wait()) }
diff --git a/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift b/Tests/AsyncHTTPClientTests/SOCKSTestUtils.swift
@@ -59,17 +59,19 @@ class MockSOCKSServer {
             bootstrap = ServerBootstrap(group: elg)
                 .serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
                 .childChannelInitializer { channel in
-                    let handshakeHandler = SOCKSServerHandshakeHandler()
-                    return channel.pipeline.addHandlers([
-                        handshakeHandler,
-                        SOCKSTestHandler(handshakeHandler: handshakeHandler),
-                        TestHTTPServer(
-                            expectedURL: expectedURL,
-                            expectedResponse: expectedResponse,
-                            file: file,
-                            line: line
-                        ),
-                    ])
+                    channel.eventLoop.makeCompletedFuture {
+                        let handshakeHandler = SOCKSServerHandshakeHandler()
+                        try channel.pipeline.syncOperations.addHandlers([
+                            handshakeHandler,
+                            SOCKSTestHandler(handshakeHandler: handshakeHandler),
+                            TestHTTPServer(
+                                expectedURL: expectedURL,
+                                expectedResponse: expectedResponse,
+                                file: file,
+                                line: line
+                            ),
+                        ])
+                    }
                 }
         }
         self.channel = try bootstrap.bind(host: "localhost", port: 0).wait()
@@ -112,15 +114,19 @@ class SOCKSTestHandler: ChannelInboundHandler, RemovableChannelHandler {
                 ),
                 promise: nil
             )
-            context.channel.pipeline.addHandlers(
-                [
-                    ByteToMessageHandler(HTTPRequestDecoder()),
-                    HTTPResponseEncoder(),
-                ],
-                position: .after(self)
-            ).whenSuccess {
-                context.channel.pipeline.removeHandler(self, promise: nil)
-                context.channel.pipeline.removeHandler(self.handshakeHandler, promise: nil)
+
+            do {
+                try context.channel.pipeline.syncOperations.addHandlers(
+                    [
+                        ByteToMessageHandler(HTTPRequestDecoder()),
+                        HTTPResponseEncoder(),
+                    ],
+                    position: .after(self)
+                )
+                context.channel.pipeline.syncOperations.removeHandler(self, promise: nil)
+                context.channel.pipeline.syncOperations.removeHandler(self.handshakeHandler, promise: nil)
+            } catch {
+                context.fireErrorCaught(error)
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {`
`167`	`167`	`}`
`168`	`168`	`} else {`
`169`	`169`	`let fileHandleFuture = io.openFile(`
`170`		`- path: self.filePath,`
	`170`	`+ _deprecatedPath: self.filePath,`
`171`	`171`	`mode: .write,`
`172`	`172`	`flags: .allowFileCreation(),`
`173`	`173`	`eventLoop: task.eventLoop`