apple / swift-nio

Event-driven network application framework for high performance protocol servers & clients, non-blocking.
https://swiftpackageindex.com/apple/swift-nio/documentation
Apache License 2.0
7.99k stars 651 forks source link

`ClientBootstrap.connect` that returns a NIOAsyncChannel hits precondition if connect fails #2637

Open adam-fowler opened 9 months ago

adam-fowler commented 9 months ago

If ClientBootstrap.connect returns a NIOAsyncChannel and the connect fails after the NIOAsyncChannel has been created, it will hit a precondition instead of throwing an error.

Expected behavior

ClientBootstrap.connect throws the error that caused the connect to fail

Actual behavior

We hit the precondition

NIOCore/NIOAsyncWriter.swift:176: Fatal error: Deinited NIOAsyncWriter without calling finish()

Steps to reproduce

  1. Setup a server to bind to address 127.0.0.1
  2. Connect to server with client using localhost instead. (I assume this is due to localhost resolving to both IPv4 and IPv6)

If possible, minimal yet complete reproducer code (or URL to code)

let result = try await ClientBootstrap(group: MultiThreadedEventLoopGroup.singleton)
    .connect(host: "localhost", port: 8080) { channel in
        channel.eventLoop.makeCompletedFuture {
            try channel.pipeline.syncOperations.addHTTPClientHandlers()
            try channel.pipeline.syncOperations.addHandlers(HTTP1ToHTTPClientCodec())
            return try NIOAsyncChannel<HTTPResponsePart, HTTPRequestPart>(
                wrappingChannelSynchronously: channel,
                configuration: .init()
            )
        }
    }
try await result.executeThenClose { _, _ in
}

SwiftNIO version/commit hash

swift-nio 2.63.0

System & version information

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5) Target: x86_64-apple-macosx14.0

FranzBusch commented 9 months ago

Thanks for filing this. We are aware of this problem and @tayloraswift has also reported this in the forums. The problem here is that we tried to model the NIOAsyncChannel has having ownership of the underlying channel so that users have a safe way to interact with it while still making sure the underlying resource (socket,fds,etc.) are cleaned up. However, our current APIs are not enough here since as you reported in the issue there are timing gaps where a NIOAsyncChannel is created and then dropped resulting in our precondition being triggered. There are actually many more of those timing gaps and we have to address them across all the APIs.

My current idea here is that we have to create new bootstrap APIs that provide scope access to the NIOAsyncChannel directly. Those APIs can make sure that the channel is closed after the scope ends. This will allow us to drop the preconditions again.