Don't close when waiting unless we were asked to. (#149)

Motivation:

When we're waiting for connectivity, the user might tell us that they
aren't interested in waiting. In that case we take advantage of the
signal and close early.

However, the code as-written had a bug: we didn't care whether the user
told us they _didn't_ want to wait, or they _did_: we just closed
because they had an opinion! That's no good! We should only close if
they don't want to wait.

Modifications:

- Only close if the user doesn't want to wait.
- Add tests

Result:

We won't close if users don't want us to.
This commit is contained in:
Cory Benfield 2022-06-07 14:36:37 +01:00 committed by GitHub
parent e90ac29391
commit 7d09200afa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 1 deletions

View File

@ -313,7 +313,7 @@ extension NIOTSConnectionChannel: Channel {
let newValue = value as! Bool
self.options.waitForActivity = newValue
if let state = self.nwConnection?.state, case .waiting(let err) = state {
if let state = self.nwConnection?.state, case .waiting(let err) = state, !newValue {
// We're in waiting now, so we should drop the connection.
self.close0(error: err, mode: .all, promise: nil)
}

View File

@ -86,6 +86,19 @@ final class DisableWaitingAfterConnect: ChannelOutboundHandler {
}
}
@available(OSX 10.14, iOS 12.0, tvOS 12.0, *)
final class EnableWaitingAfterWaiting: ChannelInboundHandler {
typealias InboundIn = Any
typealias InboundOut = Any
func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) {
if event is NIOTSNetworkEvents.WaitingForConnectivity {
// Note that this is the default value and is set to _true_.
try! context.channel.syncOptions!.setOption(NIOTSChannelOptions.waitForActivity, value: true)
}
}
}
final class PromiseOnActiveHandler: ChannelInboundHandler {
typealias InboundIn = Any
@ -578,6 +591,36 @@ class NIOTSConnectionChannelTests: XCTestCase {
}
}
func testSettingWaitForConnectivityDoesntCloseImmediately() throws {
enum Reason {
case timedClose
case autoClose
}
// We want a single loop for all this to enforce serialization.
let chosenLoop = self.group.next()
let closedPromise = chosenLoop.makePromise(of: Reason.self)
_ = NIOTSConnectionBootstrap(group: chosenLoop)
.channelInitializer { channel in
try! channel.pipeline.syncOperations.addHandler(EnableWaitingAfterWaiting())
channel.eventLoop.scheduleTask(in: .milliseconds(500)) {
channel.close(promise: nil)
closedPromise.succeed(.timedClose)
}
channel.closeFuture.whenComplete { _ in
closedPromise.succeed(.autoClose)
}
return channel.eventLoop.makeSucceededVoidFuture()
}.connect(to: try SocketAddress(unixDomainSocketPath: "/this/path/definitely/doesnt/exist"))
let result = try closedPromise.futureResult.wait()
XCTAssertEqual(result, .timedClose)
}
func testCanObserveValueOfDisableWaiting() throws {
let listener = try NIOTSListenerBootstrap(group: self.group)
.bind(host: "localhost", port: 0).wait()