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.98k stars 652 forks source link

Happy Eyeballs should not initialise multiple channels. #508

Open Lukasa opened 6 years ago

Lukasa commented 6 years ago

Related to #506.

A surprising fact of the way ClientBootstrap behaves is that it may call the channelInitializer multiple times. This is because it creates multiple channels and races them to establish the connection.

We made this decision because connect operates on SocketAddress objects, which are specific IP/port pairs. There may be multiple of these during one connection to a hostname, and so to allow users to intercept and modify connection attempts we necessarily had to create a bunch of channels.

This behaviour is pretty gross: it's a layering violation and leads to nasty bugs when users don't plan for this behaviour.

We should enhance connect to work on abstract endpoints rather than specifically on SocketAddress objects. Once we do that, we can push the happy eyeballs logic down into the event loop itself, which will allow us to configure the channel only once and then just let it use the correct socket. Sadly, doing this is a 2.0 change.

weissi commented 5 years ago

yeah, with HTTP/2, getting the multiplexer is also pretty annoying. Something like this works but is cumbersome:

bootstrap.connect(...).flatMap { channel in
    return channel.pipeline.context(handlerType: HTTP2StreamMultiplexer.self).map { $0.handler as! HTTP2StreamMultiplexer }.flatMap { multiplexer in
        // here we've got `channel` and `multiplexer`
        // [...]
    }
}
Lukasa commented 5 years ago

In general this is just an annoyance that we need to address in v3.

weissi commented 5 years ago

In general this is just an annoyance that we need to address in v3.

Agreed. However we could offer something nicer on the pipeline to get you the handler out rather than the context from which you then need to force cast your handler out :). But that's a separate issue, filed as #966 .