Skip to content

Commit

Permalink
testPlatformConnectErrorIsForwardedOnTimeout port reuse (swift-server…
Browse files Browse the repository at this point in the history
…#716)

Motivation:

The above test has been seen to fail with port already in use.
The test assumes that a port can be bound to twice in a row.
It's possible that another process takes the port between the two
binds - this can be stopped by binding a second time before stopping
the first.

Modifications:

Allow port reuse and reorder the test to bind a second time before
releasing the first.

Result:

Test should no longer be flaky.
  • Loading branch information
PeterAdams-A authored Nov 3, 2023
1 parent 4824907 commit c70e085
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
10 changes: 6 additions & 4 deletions Tests/AsyncHTTPClientTests/HTTP2ClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ class HTTP2ClientTests: XCTestCase {
}

func testPlatformConnectErrorIsForwardedOnTimeout() {
let bin = HTTPBin(.http2(compress: false))
let bin = HTTPBin(.http2(compress: false), reusePort: true)
let clientGroup = MultiThreadedEventLoopGroup(numberOfThreads: 2)
let el1 = clientGroup.next()
let el2 = clientGroup.next()
Expand Down Expand Up @@ -404,20 +404,22 @@ class HTTP2ClientTests: XCTestCase {
XCTAssertEqual(.ok, response1?.status)
XCTAssertEqual(response1?.version, .http2)
let serverPort = bin.port
XCTAssertNoThrow(try bin.shutdown())
// client is now in HTTP/2 state and the HTTPBin is closed
// start a new server on the old port which closes all connections immediately

let serverGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer { XCTAssertNoThrow(try serverGroup.syncShutdownGracefully()) }
var maybeServer: Channel?
XCTAssertNoThrow(maybeServer = try ServerBootstrap(group: serverGroup)
.serverChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: 1)
.childChannelInitializer { channel in
channel.close()
}
.childChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
.bind(host: "127.0.0.1", port: serverPort)
.wait())
// shutting down the old server closes all connections immediately
XCTAssertNoThrow(try bin.shutdown())
// client is now in HTTP/2 state and the HTTPBin is closed
guard let server = maybeServer else { return }
defer { XCTAssertNoThrow(try server.close().wait()) }

Expand Down
7 changes: 5 additions & 2 deletions Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ internal final class HTTPBin<RequestHandler: ChannelInboundHandler> where
_ mode: Mode = .http1_1(ssl: false, compress: false),
proxy: Proxy = .none,
bindTarget: BindTarget = .localhostIPv4RandomPort,
reusePort: Bool = false,
handlerFactory: @escaping (Int) -> (RequestHandler)
) {
self.mode = mode
Expand All @@ -460,6 +461,7 @@ internal final class HTTPBin<RequestHandler: ChannelInboundHandler> where

self.serverChannel = try! ServerBootstrap(group: self.group)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: reusePort ? 1 : 0)
.serverChannelInitializer { channel in
channel.pipeline.addHandler(self.activeConnCounterHandler)
}.childChannelInitializer { channel in
Expand Down Expand Up @@ -642,9 +644,10 @@ extension HTTPBin where RequestHandler == HTTPBinHandler {
convenience init(
_ mode: Mode = .http1_1(ssl: false, compress: false),
proxy: Proxy = .none,
bindTarget: BindTarget = .localhostIPv4RandomPort
bindTarget: BindTarget = .localhostIPv4RandomPort,
reusePort: Bool = false
) {
self.init(mode, proxy: proxy, bindTarget: bindTarget) { HTTPBinHandler(connectionID: $0) }
self.init(mode, proxy: proxy, bindTarget: bindTarget, reusePort: reusePort) { HTTPBinHandler(connectionID: $0) }
}
}

Expand Down

0 comments on commit c70e085

Please sign in to comment.