Complete NIOCore strict concurrency (#2959)

Motivation:

With our earlier big refactors, NIOCore is now currently strict
concurrency clean. Let's lock in the win by adopting the relevant Swift
settings and fixing up the tests.

Modifications:

- Adopt our strict concurrency settings in NIOCore and NIOCoreTests
- Clean up the tests

Result:

One step closer to a world without warnings.

Package.swift

@@ -86,7 +86,8 @@ let package = Package(
                 "_NIODataStructures",
                 swiftCollections,
                 swiftAtomics,
-            ]
+            ],
+            swiftSettings: strictConcurrencySettings
         ),
         .target(
             name: "_NIODataStructures",
@@ -414,11 +415,13 @@ let package = Package(
         .testTarget(
             name: "NIOCoreTests",
             dependencies: [
+                "NIOConcurrencyHelpers",
                 "NIOCore",
                 "NIOEmbedded",
                 "NIOFoundationCompat",
                 swiftAtomics,
-            ]
+            ],
+            swiftSettings: strictConcurrencySettings
         ),
         .testTarget(
             name: "NIOEmbeddedTests",

Sources/NIOCore/BSDSocketAPI.swift

@@ -121,7 +121,7 @@ let SO_TIMESTAMP = CNIOLinux_SO_TIMESTAMP
 let SO_RCVTIMEO = CNIOLinux_SO_RCVTIMEO
 #endif
 
-public enum NIOBSDSocket {
+public enum NIOBSDSocket: Sendable {
     #if os(Windows)
     public typealias Handle = SOCKET
     #else

Tests/NIOCoreTests/AsyncChannel/AsyncChannelTests.swift

@@ -253,7 +253,8 @@ final class AsyncChannelTests: XCTestCase {
             let strongSentinel: Sentinel? = Sentinel()
             sentinel = strongSentinel!
             try await XCTAsyncAssertNotNil(
-                await channel.pipeline.handler(type: NIOAsyncChannelHandler<Sentinel, Sentinel, Never>.self).map { _ in
+                await channel.pipeline.handler(type: NIOAsyncChannelHandler<Sentinel, Sentinel, Never>.self).map {
+                    _ -> Bool in
                     true
                 }.get()
             )
@@ -428,9 +429,8 @@ private final class CloseRecorder: ChannelOutboundHandler, @unchecked Sendable {
     }
 }
 
-private final class CloseSuppressor: ChannelOutboundHandler, RemovableChannelHandler {
+private final class CloseSuppressor: ChannelOutboundHandler, RemovableChannelHandler, Sendable {
     typealias OutboundIn = Any
-    typealias outbound = Any
 
     func close(context: ChannelHandlerContext, mode: CloseMode, promise: EventLoopPromise<Void>?) {
         // We drop the close here.

Tests/NIOCoreTests/AsyncSequences/NIOAsyncSequenceTests.swift

@@ -519,7 +519,7 @@ final class NIOAsyncSequenceProducerTests: XCTestCase {
             let value = await iterator.next()
             resumed.fulfill()
 
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             return value
         }
 
@@ -562,7 +562,7 @@ final class NIOAsyncSequenceProducerTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task: Task<Int?, Never> = Task {
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             let iterator = sequence.makeAsyncIterator()
             return await iterator.next()
         }

Tests/NIOCoreTests/AsyncSequences/NIOAsyncWriterTests.swift

@@ -69,7 +69,8 @@ final class NIOAsyncWriterTests: XCTestCase {
     override func setUp() {
         super.setUp()
 
-        self.delegate = .init()
+        let delegate = MockAsyncWriterDelegate()
+        self.delegate = delegate
         let newWriter = NIOAsyncWriter.makeWriter(
             elementType: String.self,
             isWritable: true,
@@ -78,7 +79,7 @@ final class NIOAsyncWriterTests: XCTestCase {
         )
         self.writer = newWriter.writer
         self.sink = newWriter.sink
-        self.sink._storage._setDidSuspend { self.delegate.didSuspend() }
+        self.sink._storage._setDidSuspend { delegate.didSuspend() }
     }
 
     override func tearDown() {
@@ -411,7 +412,7 @@ final class NIOAsyncWriterTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task = Task { [writer] in
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             try await writer!.yield("message2")
         }
 
@@ -470,7 +471,7 @@ final class NIOAsyncWriterTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task = Task { [writer] in
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             try await writer!.yield("message1")
         }
 
@@ -491,7 +492,7 @@ final class NIOAsyncWriterTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task = Task { [writer] in
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             try await writer!.yield("message2")
         }
 
@@ -545,7 +546,7 @@ final class NIOAsyncWriterTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task = Task { [writer] in
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             try await writer!.yield("message1")
         }
 

Tests/NIOCoreTests/AsyncSequences/NIOThrowingAsyncSequenceTests.swift

@@ -611,7 +611,7 @@ final class NIOThrowingAsyncSequenceProducerTests: XCTestCase {
             let iterator = sequence.makeAsyncIterator()
             let element = try await iterator.next()
             resumed.fulfill()
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             return element
         }
 
@@ -655,7 +655,7 @@ final class NIOThrowingAsyncSequenceProducerTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task: Task<Int?, Error> = Task {
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             let iterator = sequence.makeAsyncIterator()
             return try await iterator.next()
         }
@@ -686,7 +686,7 @@ final class NIOThrowingAsyncSequenceProducerTests: XCTestCase {
         let cancelled = expectation(description: "task cancelled")
 
         let task: Task<Int?, Error> = Task {
-            await fulfillment(of: [cancelled], timeout: 1)
+            await XCTWaiter().fulfillment(of: [cancelled], timeout: 1)
             let iterator = sequence.makeAsyncIterator()
             return try await iterator.next()
         }
@@ -879,12 +879,13 @@ final class NIOThrowingAsyncSequenceProducerTests: XCTestCase {
 
     func testIteratorThrows_whenCancelled() async {
         _ = self.source.yield(contentsOf: Array(1...100))
+        guard let sequence = self.sequence else {
+            return XCTFail("Expected to have an AsyncSequence")
+        }
+
         await withThrowingTaskGroup(of: Void.self) { group in
             group.addTask {
                 var itemsYieldedCounter = 0
-                guard let sequence = self.sequence else {
-                    return XCTFail("Expected to have an AsyncSequence")
-                }
 
                 do {
                     for try await next in sequence {

Tests/NIOCoreTests/ByteBufferTest.swift

@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import Atomics
 import NIOFoundationCompat
 import XCTest
 import _NIOBase64
@@ -2440,8 +2441,8 @@ class ByteBufferTest: XCTestCase {
     }
 
     func testReserveCapacityLargerUniquelyReferencedCallsRealloc() throws {
-        testReserveCapacityLarger_reallocCount = 0
-        testReserveCapacityLarger_mallocCount = 0
+        testReserveCapacityLarger_reallocCount.store(0, ordering: .sequentiallyConsistent)
+        testReserveCapacityLarger_mallocCount.store(0, ordering: .sequentiallyConsistent)
 
         let alloc = ByteBufferAllocator(
             hookedMalloc: testReserveCapacityLarger_mallocHook,
@@ -2453,17 +2454,17 @@ class ByteBufferTest: XCTestCase {
 
         let oldCapacity = buf.capacity
 
-        XCTAssertEqual(testReserveCapacityLarger_mallocCount, 1)
-        XCTAssertEqual(testReserveCapacityLarger_reallocCount, 0)
+        XCTAssertEqual(testReserveCapacityLarger_mallocCount.load(ordering: .sequentiallyConsistent), 1)
+        XCTAssertEqual(testReserveCapacityLarger_reallocCount.load(ordering: .sequentiallyConsistent), 0)
         buf.reserveCapacity(32)
-        XCTAssertEqual(testReserveCapacityLarger_mallocCount, 1)
-        XCTAssertEqual(testReserveCapacityLarger_reallocCount, 1)
+        XCTAssertEqual(testReserveCapacityLarger_mallocCount.load(ordering: .sequentiallyConsistent), 1)
+        XCTAssertEqual(testReserveCapacityLarger_reallocCount.load(ordering: .sequentiallyConsistent), 1)
         XCTAssertNotEqual(buf.capacity, oldCapacity)
     }
 
     func testReserveCapacityLargerMultipleReferenceCallsMalloc() throws {
-        testReserveCapacityLarger_reallocCount = 0
-        testReserveCapacityLarger_mallocCount = 0
+        testReserveCapacityLarger_reallocCount.store(0, ordering: .sequentiallyConsistent)
+        testReserveCapacityLarger_mallocCount.store(0, ordering: .sequentiallyConsistent)
 
         let alloc = ByteBufferAllocator(
             hookedMalloc: testReserveCapacityLarger_mallocHook,
@@ -2480,11 +2481,11 @@ class ByteBufferTest: XCTestCase {
                 UInt(bitPattern: $0.baseAddress!)
             }
 
-            XCTAssertEqual(testReserveCapacityLarger_mallocCount, 1)
-            XCTAssertEqual(testReserveCapacityLarger_reallocCount, 0)
+            XCTAssertEqual(testReserveCapacityLarger_mallocCount.load(ordering: .sequentiallyConsistent), 1)
+            XCTAssertEqual(testReserveCapacityLarger_reallocCount.load(ordering: .sequentiallyConsistent), 0)
             buf.reserveCapacity(32)
-            XCTAssertEqual(testReserveCapacityLarger_mallocCount, 2)
-            XCTAssertEqual(testReserveCapacityLarger_reallocCount, 0)
+            XCTAssertEqual(testReserveCapacityLarger_mallocCount.load(ordering: .sequentiallyConsistent), 2)
+            XCTAssertEqual(testReserveCapacityLarger_reallocCount.load(ordering: .sequentiallyConsistent), 0)
 
             let newPtrVal = buf.withVeryUnsafeBytes {
                 UInt(bitPattern: $0.baseAddress!)
@@ -3354,7 +3355,15 @@ private enum AllocationExpectationState: Int {
     case freeDone
 }
 
-private var testAllocationOfReallyBigByteBuffer_state = AllocationExpectationState.begin
+private let _testAllocationOfReallyBigByteBuffer_state = ManagedAtomic<Int>(AllocationExpectationState.begin.rawValue)
+private var testAllocationOfReallyBigByteBuffer_state: AllocationExpectationState {
+    get {
+        .init(rawValue: _testAllocationOfReallyBigByteBuffer_state.load(ordering: .acquiring))!
+    }
+    set {
+        _testAllocationOfReallyBigByteBuffer_state.store(newValue.rawValue, ordering: .releasing)
+    }
+}
 private func testAllocationOfReallyBigByteBuffer_freeHook(_ ptr: UnsafeMutableRawPointer?) {
     precondition(AllocationExpectationState.reallocDone == testAllocationOfReallyBigByteBuffer_state)
     testAllocationOfReallyBigByteBuffer_state = .freeDone
@@ -3387,22 +3396,22 @@ private func testAllocationOfReallyBigByteBuffer_memcpyHook(
     // not actually doing any copies
 }
 
-private var testReserveCapacityLarger_reallocCount = 0
-private var testReserveCapacityLarger_mallocCount = 0
+private let testReserveCapacityLarger_reallocCount = ManagedAtomic(0)
+private let testReserveCapacityLarger_mallocCount = ManagedAtomic(0)
 private func testReserveCapacityLarger_freeHook(_ ptr: UnsafeMutableRawPointer) {
     free(ptr)
 }
 
 private func testReserveCapacityLarger_mallocHook(_ size: Int) -> UnsafeMutableRawPointer? {
-    testReserveCapacityLarger_mallocCount += 1
+    testReserveCapacityLarger_mallocCount.wrappingIncrement(ordering: .sequentiallyConsistent)
     return malloc(size)
 }
 
 private func testReserveCapacityLarger_reallocHook(
     _ ptr: UnsafeMutableRawPointer?,
     _ count: Int
 ) -> UnsafeMutableRawPointer? {
-    testReserveCapacityLarger_reallocCount += 1
+    testReserveCapacityLarger_reallocCount.wrappingIncrement(ordering: .sequentiallyConsistent)
     return realloc(ptr, count)
 }
 

Tests/NIOCoreTests/ChannelOptionStorageTest.swift

@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import NIOConcurrencyHelpers
 import NIOCore
 import NIOEmbedded
 import XCTest
@@ -110,8 +111,16 @@ class ChannelOptionStorageTest: XCTestCase {
     }
 }
 
-class OptionsCollectingChannel: Channel {
-    var allOptions: [(Any, Any)] = []
+final class OptionsCollectingChannel: Channel {
+    private let _allOptions = NIOLockedValueBox<[(any Sendable, any Sendable)]>([])
+    var allOptions: [(any Sendable, any Sendable)] {
+        get {
+            self._allOptions.withLockedValue { $0 }
+        }
+        set {
+            self._allOptions.withLockedValue { $0 = newValue }
+        }
+    }
 
     var allocator: ByteBufferAllocator { fatalError() }
 

Tests/NIOCoreTests/DispatchQueue+WithFutureTest.swift

@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import Atomics
 import Dispatch
 import NIOCore
 import NIOEmbedded
@@ -30,20 +31,20 @@ class DispatchQueueWithFutureTest: XCTestCase {
         }
         let eventLoop = group.next()
         let sem = DispatchSemaphore(value: 0)
-        var nonBlockingRan = false
+        let nonBlockingRan = ManagedAtomic(false)
         let futureResult: EventLoopFuture<String> = DispatchQueue.global().asyncWithFuture(eventLoop: eventLoop) {
             () -> String in
             sem.wait()  // Block in callback
             return "hello"
         }
         futureResult.whenSuccess { value in
             XCTAssertEqual(value, "hello")
-            XCTAssertTrue(nonBlockingRan)
+            XCTAssertTrue(nonBlockingRan.load(ordering: .sequentiallyConsistent))
         }
 
         let p2 = eventLoop.makePromise(of: Bool.self)
         p2.futureResult.whenSuccess { _ in
-            nonBlockingRan = true
+            nonBlockingRan.store(true, ordering: .sequentiallyConsistent)
         }
         p2.succeed(true)
 
@@ -57,20 +58,20 @@ class DispatchQueueWithFutureTest: XCTestCase {
         }
         let eventLoop = group.next()
         let sem = DispatchSemaphore(value: 0)
-        var nonBlockingRan = false
+        let nonBlockingRan = ManagedAtomic(false)
         let futureResult: EventLoopFuture<String> = DispatchQueue.global().asyncWithFuture(eventLoop: eventLoop) {
             () -> String in
             sem.wait()  // Block in callback
             throw DispatchQueueTestError.example
         }
         futureResult.whenFailure { err in
             XCTAssertEqual(err as! DispatchQueueTestError, DispatchQueueTestError.example)
-            XCTAssertTrue(nonBlockingRan)
+            XCTAssertTrue(nonBlockingRan.load(ordering: .sequentiallyConsistent))
         }
 
         let p2 = eventLoop.makePromise(of: Bool.self)
         p2.futureResult.whenSuccess { _ in
-            nonBlockingRan = true
+            nonBlockingRan.store(true, ordering: .sequentiallyConsistent)
         }
         p2.succeed(true)
 

Tests/NIOCoreTests/NIOCloseOnErrorHandlerTest.swift

@@ -16,7 +16,7 @@ import NIOCore
 import NIOEmbedded
 import XCTest
 
-final class DummyFailingHandler1: ChannelInboundHandler {
+final class DummyFailingHandler1: ChannelInboundHandler, Sendable {
     typealias InboundIn = NIOAny
 
     struct DummyError1: Error {}

Tests/NIOCoreTests/XCTest+Extensions.swift

@@ -57,7 +57,7 @@ func assertNoThrowWithValue<T>(
 
 func withTemporaryFile<T>(content: String? = nil, _ body: (NIOCore.NIOFileHandle, String) throws -> T) throws -> T {
     let temporaryFilePath = "\(temporaryDirectory)/nio_\(UUID())"
-    FileManager.default.createFile(atPath: temporaryFilePath, contents: content?.data(using: .utf8))
+    _ = FileManager.default.createFile(atPath: temporaryFilePath, contents: content?.data(using: .utf8))
     defer {
         XCTAssertNoThrow(try FileManager.default.removeItem(atPath: temporaryFilePath))
     }