Skip to content

Commit

Permalink
Prevent crash in Happy Eyeballs Resolver (#3003)
Browse files Browse the repository at this point in the history
Motivation:

In vanishingly rare situations it is possible for the AAAA results to
come in on the same tick as the resolution delay timer completes. In
those cases, depending on the ordering of the tasks, we can get
situations where the resolution delay timer completion causes a crash.

Modifications:

Tolerate receiving the resolution delay timer after resolution
completes.

Result:

Fewer crashes
  • Loading branch information
Lukasa authored Nov 26, 2024
1 parent 49b9d97 commit 16f19c0
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
7 changes: 6 additions & 1 deletion Sources/NIOPosix/HappyEyeballs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,12 @@ internal final class HappyEyeballsConnector<ChannelBuilderResult> {
// notifications, and can also get late scheduled task callbacks. We want to just quietly
// ignore these, as our transition into the complete state should have already sent
// cleanup messages to all of these things.
case (.complete, .resolverACompleted),
//
// We can also get the resolutionDelayElapsed after allResolved, as it's possible that
// callback was already dequeued in the same tick as the cancellation. That's also fine:
// the resolution delay isn't interesting.
case (.allResolved, .resolutionDelayElapsed),
(.complete, .resolverACompleted),
(.complete, .resolverAAAACompleted),
(.complete, .connectSuccess),
(.complete, .connectFailed),
Expand Down
47 changes: 47 additions & 0 deletions Tests/NIOPosixTests/HappyEyeballsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1308,4 +1308,51 @@ public final class HappyEyeballsTest: XCTestCase {

XCTAssertNoThrow(try client.close().wait())
}

func testResolutionTimeoutAndResolutionInSameTick() throws {
var channels: [Channel] = []
let (eyeballer, resolver, loop) = buildEyeballer(host: "example.com", port: 80) {
let channelFuture = defaultChannelBuilder(loop: $0, family: $1)
channelFuture.whenSuccess { channel in
try! channel.pipeline.addHandler(ConnectionDelayer(), name: CONNECT_DELAYER, position: .first).wait()
channels.append(channel)
}
return channelFuture
}
let targetFuture = eyeballer.resolveAndConnect().flatMapThrowing { (channel) -> String? in
let target = channel.connectTarget()
_ = try (channel as! EmbeddedChannel).finish()
return target
}
loop.run()

// Then, queue a task to resolve the v6 promise after 50ms.
// Why 50ms? This is the same time as the resolution delay.
let promise = resolver.v6Promise
loop.scheduleTask(in: .milliseconds(50)) {
promise.fail(DummyError())
}

// Kick off the IPv4 resolution. This triggers the timer for the resolution delay.
resolver.v4Promise.succeed(SINGLE_IPv4_RESULT)
loop.run()

// Advance time 50ms.
loop.advanceTime(by: .milliseconds(50))

// Then complete the connection future.
XCTAssertEqual(channels.count, 1)
channels.first!.succeedConnection()

// Should be done.
let target = try targetFuture.wait()
XCTAssertEqual(target!, "10.0.0.1")

// We should have had queries for AAAA and A.
let expectedQueries: [DummyResolver.Event] = [
.aaaa(host: "example.com", port: 80),
.a(host: "example.com", port: 80),
]
XCTAssertEqual(resolver.events, expectedQueries)
}
}

0 comments on commit 16f19c0

Please sign in to comment.