Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the HappyEyeballsResolver and core Bootstraps under strict concurrency #3062

Merged
merged 7 commits into from
Jan 23, 2025
Merged

Fix the HappyEyeballsResolver and core Bootstraps under strict concurrency #3062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ccbb13f
Fix the HappyEyeballsResolver and core Bootstraps
Lukasa Dec 17, 2024
1381acc
Fix HE tests
Lukasa Dec 17, 2024
42ff33d
More warnings
Lukasa Jan 15, 2025
5656491
Formatting
Lukasa Jan 15, 2025
8468014
Merge branch 'main' into cb-strict-concurrency-happy-eyeballs-resolver
Lukasa Jan 21, 2025
7bf23be
Merge branch 'main' into cb-strict-concurrency-happy-eyeballs-resolver
Lukasa Jan 22, 2025
a340641
Lift baselines
Lukasa Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Benchmarks/Thresholds/5.10/NIOPosixBenchmarks.TCPEcho.p90.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"mallocCountTotal" : 107
"mallocCountTotal" : 108
}
2 changes: 1 addition & 1 deletion Benchmarks/Thresholds/5.9/NIOPosixBenchmarks.TCPEcho.p90.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"mallocCountTotal" : 109
"mallocCountTotal" : 110
}
2 changes: 1 addition & 1 deletion Benchmarks/Thresholds/6.0/NIOPosixBenchmarks.TCPEcho.p90.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"mallocCountTotal" : 107
"mallocCountTotal" : 108
}
2 changes: 1 addition & 1 deletion Benchmarks/Thresholds/nightly-6.1/NIOPosixBenchmarks.TCPEcho.p90.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"mallocCountTotal" : 107
"mallocCountTotal" : 108
}
2 changes: 1 addition & 1 deletion Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEcho.p90.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"mallocCountTotal" : 107
"mallocCountTotal" : 108
}
166 changes: 153 additions & 13 deletions Sources/NIOPosix/Bootstrap.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol {
@usableFromInline
internal var _channelOptions: ChannelOptions.Storage
private var connectTimeout: TimeAmount = TimeAmount.seconds(10)
private var resolver: Optional<Resolver>
private var resolver: Optional<Resolver & Sendable>
private var bindTarget: Optional<SocketAddress>
private var enableMPTCP: Bool

Expand Down Expand Up @@ -924,7 +924,8 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol {
///
/// - Parameters:
/// - resolver: The resolver that will be used during the connection attempt.
public func resolver(_ resolver: Resolver?) -> Self {
@preconcurrency
public func resolver(_ resolver: (Resolver & Sendable)?) -> Self {
self.resolver = resolver
return self
}
Expand Down Expand Up @@ -967,11 +968,23 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol {
func makeSocketChannel(
eventLoop: EventLoop,
protocolFamily: NIOBSDSocket.ProtocolFamily
) throws -> SocketChannel {
try Self.makeSocketChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTCP: self.enableMPTCP
)
}

static func makeSocketChannel(
eventLoop: EventLoop,
protocolFamily: NIOBSDSocket.ProtocolFamily,
enableMPTCP: Bool
) throws -> SocketChannel {
try SocketChannel(
eventLoop: eventLoop as! SelectableEventLoop,
protocolFamily: protocolFamily,
enableMPTCP: self.enableMPTCP
enableMPTCP: enableMPTCP
)
}

Expand All @@ -992,14 +1005,26 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol {
aiSocktype: .stream,
aiProtocol: .tcp
)
let enableMPTCP = self.enableMPTCP
let channelInitializer = self.channelInitializer
let channelOptions = self._channelOptions
let bindTarget = self.bindTarget

let connector = HappyEyeballsConnector(
resolver: resolver,
loop: loop,
host: host,
port: port,
connectTimeout: self.connectTimeout
) { eventLoop, protocolFamily in
self.initializeAndRegisterNewChannel(eventLoop: eventLoop, protocolFamily: protocolFamily) {
Self.initializeAndRegisterNewChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTCP: enableMPTCP,
channelInitializer: channelInitializer,
channelOptions: channelOptions,
bindTarget: bindTarget
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically I think this is our new allocation.

$0.eventLoop.makeSucceededFuture(())
}
}
Expand Down Expand Up @@ -1148,24 +1173,67 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol {
eventLoop: EventLoop,
protocolFamily: NIOBSDSocket.ProtocolFamily,
_ body: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
) -> EventLoopFuture<Channel> {
Self.initializeAndRegisterNewChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTCP: self.enableMPTCP,
channelInitializer: self.channelInitializer,
channelOptions: self._channelOptions,
bindTarget: self.bindTarget,
body
)
}

private static func initializeAndRegisterNewChannel(
eventLoop: EventLoop,
protocolFamily: NIOBSDSocket.ProtocolFamily,
enableMPTCP: Bool,
channelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<Void>,
channelOptions: ChannelOptions.Storage,
bindTarget: SocketAddress?,
_ body: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
) -> EventLoopFuture<Channel> {
let channel: SocketChannel
do {
channel = try self.makeSocketChannel(eventLoop: eventLoop, protocolFamily: protocolFamily)
channel = try Self.makeSocketChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTCP: enableMPTCP
)
} catch {
return eventLoop.makeFailedFuture(error)
}
return self.initializeAndRegisterChannel(channel, body)
return Self.initializeAndRegisterChannel(
channel,
channelInitializer: channelInitializer,
channelOptions: channelOptions,
bindTarget: bindTarget,
body
)
}

private func initializeAndRegisterChannel(
_ channel: SocketChannel,
_ body: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
) -> EventLoopFuture<Channel> {
let channelInitializer = self.channelInitializer
let channelOptions = self._channelOptions
Self.initializeAndRegisterChannel(
channel,
channelInitializer: self.channelInitializer,
channelOptions: self._channelOptions,
bindTarget: self.bindTarget,
body
)
}

private static func initializeAndRegisterChannel(
_ channel: SocketChannel,
channelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<Void>,
channelOptions: ChannelOptions.Storage,
bindTarget: SocketAddress?,
_ body: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
) -> EventLoopFuture<Channel> {
let eventLoop = channel.eventLoop
let bindTarget = self.bindTarget

@inline(__always)
@Sendable
Expand Down Expand Up @@ -1352,16 +1420,25 @@ extension ClientBootstrap {
aiProtocol: .tcp
)

let enableMPTCP = self.enableMPTCP
let bootstrapChannelInitializer = self.channelInitializer
let channelOptions = self._channelOptions
let bindTarget = self.bindTarget

let connector = HappyEyeballsConnector<PostRegistrationTransformationResult>(
resolver: resolver,
loop: eventLoop,
host: host,
port: port,
connectTimeout: self.connectTimeout
) { eventLoop, protocolFamily in
self.initializeAndRegisterNewChannel(
Self.initializeAndRegisterNewChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTPCP: enableMPTCP,
bootstrapChannelInitializer: bootstrapChannelInitializer,
channelOptions: channelOptions,
bindTarget: bindTarget,
channelInitializer: channelInitializer,
postRegisterTransformation: postRegisterTransformation
) {
Expand Down Expand Up @@ -1426,6 +1503,46 @@ extension ClientBootstrap {
).map { (channel, $0) }
}

@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
private static func initializeAndRegisterNewChannel<
ChannelInitializerResult: Sendable,
PostRegistrationTransformationResult: Sendable
>(
eventLoop: EventLoop,
protocolFamily: NIOBSDSocket.ProtocolFamily,
enableMPTPCP: Bool,
bootstrapChannelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<Void>,
channelOptions: ChannelOptions.Storage,
bindTarget: SocketAddress?,
channelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<ChannelInitializerResult>,
postRegisterTransformation: @escaping @Sendable (ChannelInitializerResult, EventLoop) -> EventLoopFuture<
PostRegistrationTransformationResult
>,
_ body: @escaping @Sendable (Channel) -> EventLoopFuture<Void>
) -> EventLoopFuture<(Channel, PostRegistrationTransformationResult)> {
let channel: SocketChannel
do {
channel = try Self.makeSocketChannel(
eventLoop: eventLoop,
protocolFamily: protocolFamily,
enableMPTCP: enableMPTPCP
)
} catch {
return eventLoop.makeFailedFuture(error)
}
return Self.initializeAndRegisterChannel(
channel: channel,
bootstrapChannelInitializer: bootstrapChannelInitializer,
channelOptions: channelOptions,
bindTarget: bindTarget,
channelInitializer: channelInitializer,
registration: { channel in
channel.registerAndDoSynchronously(body)
},
postRegisterTransformation: postRegisterTransformation
).map { (channel, $0) }
}

@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
private func initializeAndRegisterChannel<
ChannelInitializerResult: Sendable,
Expand All @@ -1438,16 +1555,39 @@ extension ClientBootstrap {
PostRegistrationTransformationResult
>
) -> EventLoopFuture<PostRegistrationTransformationResult> {
let bootstrapChannelInitializer = self.channelInitializer
Self.initializeAndRegisterChannel(
channel: channel,
bootstrapChannelInitializer: self.channelInitializer,
channelOptions: self._channelOptions,
bindTarget: self.bindTarget,
channelInitializer: channelInitializer,
registration: registration,
postRegisterTransformation: postRegisterTransformation
)
}

@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
private static func initializeAndRegisterChannel<
ChannelInitializerResult: Sendable,
PostRegistrationTransformationResult: Sendable
>(
channel: SocketChannel,
bootstrapChannelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<Void>,
channelOptions: ChannelOptions.Storage,
bindTarget: SocketAddress?,
channelInitializer: @escaping @Sendable (Channel) -> EventLoopFuture<ChannelInitializerResult>,
registration: @escaping @Sendable (SocketChannel) -> EventLoopFuture<Void>,
postRegisterTransformation: @escaping @Sendable (ChannelInitializerResult, EventLoop) -> EventLoopFuture<
PostRegistrationTransformationResult
>
) -> EventLoopFuture<PostRegistrationTransformationResult> {
let channelInitializer = { @Sendable channel in
bootstrapChannelInitializer(channel).hop(to: channel.eventLoop)
.assumeIsolated()
.flatMap { channelInitializer(channel) }
.nonisolated()
}
let channelOptions = self._channelOptions
let eventLoop = channel.eventLoop
let bindTarget = self.bindTarget

@inline(__always)
@Sendable
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOPosix/GetaddrinfoResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import struct WinSDK.SOCKADDR_IN6
// A thread-specific variable where we store the offload queue if we're on an `SelectableEventLoop`.
let offloadQueueTSV = ThreadSpecificVariable<DispatchQueue>()

internal class GetaddrinfoResolver: Resolver {
internal final class GetaddrinfoResolver: Resolver, Sendable {
private let loop: EventLoop
private let v4Future: EventLoopPromise<[SocketAddress]>
private let v6Future: EventLoopPromise<[SocketAddress]>
Expand Down
Loading
Loading