Skip to content

Commit 97c3f28

Browse files
authored
Fix flakiness in testWithConfiguredStreamSocket (#3399)
Motivation: This test occasionally fails in our ongoing continuous CI solution. Flaky tests are bad, and we should aim not to have them, so let's fix it. My diagnosis of the most likely race here (which leads to `alreadyClosed`) being thrown _somewhere_ from this code is that the client channel got closed unexpectedly. This would happen if an unusual series of events occurs: 1. The client connection succeeds and the channel starts up 2. The server close then goes through and the server channel is closed _before_ it actually accepts the connection. 3. This causes the client connection to be reset. 4. The client channel observes this and handles that close. 5. We then close the client from the outside. Modifications: Add a ConditionLock we can use to wait for a connection to be established. Make the use of this ConditionLock resilient to weird test behaviour by always using timeouts for blocking operations. Result: Less flaky tests.
1 parent a3ed8e1 commit 97c3f28

File tree

1 file changed

+20
-2
lines changed

1 file changed

+20
-2
lines changed

Tests/NIOPosixTests/SocketChannelTest.swift

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,13 +373,22 @@ final class SocketChannelTest: XCTestCase {
373373
}
374374

375375
public func testWithConfiguredStreamSocket() throws {
376+
let didAccept = ConditionLock<Int>(value: 0)
376377
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
377378
defer { XCTAssertNoThrow(try group.syncShutdownGracefully()) }
378379

379380
let serverSock = try Socket(protocolFamily: .inet, type: .stream)
380381
try serverSock.bind(to: SocketAddress(ipAddress: "127.0.0.1", port: 0))
381382
let serverChannelFuture = try serverSock.withUnsafeHandle {
382-
ServerBootstrap(group: group).withBoundSocket(dup($0))
383+
ServerBootstrap(group: group)
384+
.childChannelInitializer { channel in
385+
channel.eventLoop.makeCompletedFuture {
386+
let acquiredLock = didAccept.lock(whenValue: 0, timeoutSeconds: 1)
387+
XCTAssertTrue(acquiredLock)
388+
didAccept.unlock(withValue: 1)
389+
}
390+
}
391+
.withBoundSocket(dup($0))
383392
}
384393
try serverSock.close()
385394
let serverChannel = try serverChannelFuture.wait()
@@ -391,10 +400,19 @@ final class SocketChannelTest: XCTestCase {
391400
ClientBootstrap(group: group).withConnectedSocket(dup($0))
392401
}
393402
try clientSock.close()
394-
let clientChannel = try clientChannelFuture.wait()
395403

404+
// At this point we need to wait not just for the client connection to be created
405+
// but also for the server connection to come in. Otherwise we risk a race where the
406+
// client connection has come up but the server connection hasn't yet, leading to the
407+
// server channel close below causing the server to never accept the inbound channel
408+
// and leading to an unexpected error on close.
409+
let clientChannel = try clientChannelFuture.wait()
396410
XCTAssertEqual(true, clientChannel.isActive)
397411

412+
let acquiredLock = didAccept.lock(whenValue: 1, timeoutSeconds: 1)
413+
XCTAssertTrue(acquiredLock)
414+
didAccept.unlock()
415+
398416
try serverChannel.close().wait()
399417
try clientChannel.close().wait()
400418
}

0 commit comments

Comments
 (0)