Skip to content

Commit

Permalink
add AcceptHandler after serverChannelInitializer runs (#1393)
Browse files Browse the repository at this point in the history
Motivation:

For compatibility with 2.13.1 and earlier, we're adding the
AcceptHandler after the serverChannelInitializer runs. If that's the
best solution is discussed in #1392.

Modifications:

- add AcceptHandler after the serverChannelInitializer
- give the AcceptHandler the name AcceptHandler

Result:

- better compatibility
  • Loading branch information
weissi committed Feb 13, 2020
1 parent 589bc7c commit 16ab4d6
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
7 changes: 4 additions & 3 deletions Sources/NIO/Bootstrap.swift
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,11 @@ public final class ServerBootstrap {

return eventLoop.submit {
serverChannelOptions.applyAllChannelOptions(to: serverChannel).flatMap {
serverChannel.pipeline.addHandler(AcceptHandler(childChannelInitializer: childChannelInit,
childChannelOptions: childChannelOptions))
}.flatMap {
serverChannelInit(serverChannel)
}.flatMap {
serverChannel.pipeline.addHandler(AcceptHandler(childChannelInitializer: childChannelInit,
childChannelOptions: childChannelOptions),
name: "AcceptHandler")
}.flatMap {
register(eventLoop, serverChannel)
}.map {
Expand Down
1 change: 1 addition & 0 deletions Tests/NIOTests/BootstrapTest+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extension BootstrapTest {
("testPreConnectedSocketSetsChannelOptionsBeforeChannelInitializer", testPreConnectedSocketSetsChannelOptionsBeforeChannelInitializer),
("testDatagramBootstrapSetsChannelOptionsBeforeChannelInitializer", testDatagramBootstrapSetsChannelOptionsBeforeChannelInitializer),
("testPipeBootstrapSetsChannelOptionsBeforeChannelInitializer", testPipeBootstrapSetsChannelOptionsBeforeChannelInitializer),
("testServerBootstrapAddsAcceptHandlerAfterServerChannelInitialiser", testServerBootstrapAddsAcceptHandlerAfterServerChannelInitialiser),
]
}
}
Expand Down
37 changes: 37 additions & 0 deletions Tests/NIOTests/BootstrapTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,43 @@ class BootstrapTest: XCTestCase {
return []
})
}

func testServerBootstrapAddsAcceptHandlerAfterServerChannelInitialiser() {
// It's unclear if this is the right solution, see https://github.com/apple/swift-nio/issues/1392
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}

struct FoundHandlerThatWasNotSupposedToBeThereError: Error {}

var maybeServer: Channel? = nil
XCTAssertNoThrow(maybeServer = try ServerBootstrap(group: group)
.serverChannelInitializer { channel in
// Here, we test that we can't find the AcceptHandler
return channel.pipeline.context(name: "AcceptHandler").flatMap { context -> EventLoopFuture<Void> in
XCTFail("unexpectedly found \(context)")
return channel.eventLoop.makeFailedFuture(FoundHandlerThatWasNotSupposedToBeThereError())
}.flatMapError { error -> EventLoopFuture<Void> in
XCTAssertEqual(.notFound, error as? ChannelPipelineError)
if case .some(.notFound) = error as? ChannelPipelineError {
return channel.eventLoop.makeSucceededFuture(())
}
return channel.eventLoop.makeFailedFuture(error)
}
}
.bind(host: "127.0.0.1", port: 0)
.wait())

guard let server = maybeServer else {
XCTFail("couldn't bootstrap server")
return
}

// But now, it should be there.
XCTAssertNoThrow(_ = try server.pipeline.context(name: "AcceptHandler").wait())
XCTAssertNoThrow(try server.close().wait())
}
}

private final class MakeSureAutoReadIsOffInChannelInitializer: ChannelInboundHandler {
Expand Down

0 comments on commit 16ab4d6

Please sign in to comment.