From 38306728466480deca038e71dd6832c9f29c3d07 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Tue, 8 Apr 2025 17:27:04 +0100 Subject: [PATCH] Strict concurrency for NIOPosixTests, EL and ELF --- Tests/NIOPosixTests/EventLoopFutureTest.swift | 103 +++++++++--------- Tests/NIOPosixTests/EventLoopTest.swift | 75 ++++++++----- 2 files changed, 103 insertions(+), 75 deletions(-) diff --git a/Tests/NIOPosixTests/EventLoopFutureTest.swift b/Tests/NIOPosixTests/EventLoopFutureTest.swift index b5c933a528c..edd424eee67 100644 --- a/Tests/NIOPosixTests/EventLoopFutureTest.swift +++ b/Tests/NIOPosixTests/EventLoopFutureTest.swift @@ -851,8 +851,8 @@ class EventLoopFutureTest: XCTestCase { let promises = (0..<5).map { _ in group.next().makePromise(of: Int.self) } let futures = promises.map { $0.futureResult } - var succeeded = false - var completedPromises = false + let succeeded = NIOLockedValueBox(false) + let completedPromises = NIOLockedValueBox(false) let mainFuture: EventLoopFuture<[Int]> @@ -864,13 +864,13 @@ class EventLoopFutureTest: XCTestCase { } mainFuture.whenSuccess { _ in - XCTAssertTrue(completedPromises) - XCTAssertFalse(succeeded) - succeeded = true + XCTAssertTrue(completedPromises.withLockedValue { $0 }) + XCTAssertFalse(succeeded.withLockedValue { $0 }) + succeeded.withLockedValue { $0 = true } } // Should be false, as none of the promises have completed yet - XCTAssertFalse(succeeded) + XCTAssertFalse(succeeded.withLockedValue { $0 }) // complete the first four promises for (index, promise) in promises.dropLast().enumerated() { @@ -878,10 +878,10 @@ class EventLoopFutureTest: XCTestCase { } // Should still be false, as one promise hasn't completed yet - XCTAssertFalse(succeeded) + XCTAssertFalse(succeeded.withLockedValue { $0 }) // Complete the last promise - completedPromises = true + completedPromises.withLockedValue { $0 = true } promises.last!.succeed(4) let results = try assertNoThrowWithValue(mainFuture.wait()) @@ -903,8 +903,8 @@ class EventLoopFutureTest: XCTestCase { let promises = expected.map { _ in group.next().makePromise(of: Int.self) } let futures = promises.map { $0.futureResult } - var succeeded = false - var completedPromises = false + let succeeded = NIOLockedValueBox(false) + let completedPromises = NIOLockedValueBox(false) let mainFuture: EventLoopFuture<[Int]> @@ -916,14 +916,14 @@ class EventLoopFutureTest: XCTestCase { } mainFuture.whenSuccess { _ in - XCTAssertTrue(completedPromises) - XCTAssertFalse(succeeded) - succeeded = true + XCTAssertTrue(completedPromises.withLockedValue { $0 }) + XCTAssertFalse(succeeded.withLockedValue { $0 }) + succeeded.withLockedValue { $0 = true } } for index in expected.reversed() { if index == 0 { - completedPromises = true + completedPromises.withLockedValue { $0 = true } } promises[index].succeed(index) } @@ -1009,8 +1009,8 @@ class EventLoopFutureTest: XCTestCase { let promises = (0..<5).map { _ in group.next().makePromise(of: Int.self) } let futures = promises.map { $0.futureResult } - var succeeded = false - var completedPromises = false + let succeeded = NIOLockedValueBox(false) + let completedPromises = NIOLockedValueBox(false) let mainFuture: EventLoopFuture<[Result]> @@ -1022,13 +1022,13 @@ class EventLoopFutureTest: XCTestCase { } mainFuture.whenSuccess { _ in - XCTAssertTrue(completedPromises) - XCTAssertFalse(succeeded) - succeeded = true + XCTAssertTrue(completedPromises.withLockedValue { $0 }) + XCTAssertFalse(succeeded.withLockedValue { $0 }) + succeeded.withLockedValue { $0 = true } } // Should be false, as none of the promises have completed yet - XCTAssertFalse(succeeded) + XCTAssertFalse(succeeded.withLockedValue { $0 }) // complete the first four promises for (index, promise) in promises.dropLast().enumerated() { @@ -1036,10 +1036,10 @@ class EventLoopFutureTest: XCTestCase { } // Should still be false, as one promise hasn't completed yet - XCTAssertFalse(succeeded) + XCTAssertFalse(succeeded.withLockedValue { $0 }) // Complete the last promise - completedPromises = true + completedPromises.withLockedValue { $0 = true } promises.last!.succeed(4) let results = try assertNoThrowWithValue(mainFuture.wait().map { try $0.get() }) @@ -1051,28 +1051,31 @@ class EventLoopFutureTest: XCTestCase { } struct DatabaseError: Error {} - struct Database { - let query: () -> EventLoopFuture<[String]> + final class Database: Sendable { + private let query: @Sendable () -> EventLoopFuture<[String]> + private let _closed = NIOLockedValueBox(false) - var closed = false + var closed: Bool { + self._closed.withLockedValue { $0 } + } - init(query: @escaping () -> EventLoopFuture<[String]>) { + init(query: @escaping @Sendable () -> EventLoopFuture<[String]>) { self.query = query } func runQuery() -> EventLoopFuture<[String]> { - query() + self.query() } - mutating func close() { - self.closed = true + func close() { + self._closed.withLockedValue { $0 = true } } } func testAlways() throws { let group = EmbeddedEventLoop() let loop = group.next() - var db = Database { loop.makeSucceededFuture(["Item 1", "Item 2", "Item 3"]) } + let db = Database { loop.makeSucceededFuture(["Item 1", "Item 2", "Item 3"]) } XCTAssertFalse(db.closed) let _ = try assertNoThrowWithValue( @@ -1087,7 +1090,7 @@ class EventLoopFutureTest: XCTestCase { func testAlwaysWithFailingPromise() throws { let group = EmbeddedEventLoop() let loop = group.next() - var db = Database { loop.makeFailedFuture(DatabaseError()) } + let db = Database { loop.makeFailedFuture(DatabaseError()) } XCTAssertFalse(db.closed) let _ = try XCTAssertThrowsError( @@ -1173,26 +1176,26 @@ class EventLoopFutureTest: XCTestCase { func testAndAllCompleteWithZeroFutures() { let eventLoop = EmbeddedEventLoop() - let done = DispatchWorkItem {} + let done = DispatchSemaphore(value: 0) EventLoopFuture.andAllComplete([], on: eventLoop).whenComplete { (result: Result) in _ = result.mapError { error -> Error in XCTFail("unexpected error \(error)") return error } - done.perform() + done.signal() } done.wait() } func testAndAllSucceedWithZeroFutures() { let eventLoop = EmbeddedEventLoop() - let done = DispatchWorkItem {} + let done = DispatchSemaphore(value: 0) EventLoopFuture.andAllSucceed([], on: eventLoop).whenComplete { result in _ = result.mapError { error -> Error in XCTFail("unexpected error \(error)") return error } - done.perform() + done.signal() } done.wait() } @@ -1325,12 +1328,12 @@ class EventLoopFutureTest: XCTestCase { } let exitPromise: EventLoopPromise = elg1.next().makePromise() - var callNumber = 0 + let callNumber = NIOLockedValueBox(0) _ = elg1.next().scheduleRepeatedAsyncTask(initialDelay: .nanoseconds(0), delay: .nanoseconds(0)) { task in struct Dummy: Error {} - callNumber += 1 - switch callNumber { + callNumber.withLockedValue { $0 += 1 } + switch callNumber.withLockedValue({ $0 }) { case 1: return elg2.next().makeSucceededFuture(()) case 2: @@ -1460,18 +1463,18 @@ class EventLoopFutureTest: XCTestCase { func testWhenFailureBlocking() { let eventLoop = EmbeddedEventLoop() let sem = DispatchSemaphore(value: 0) - var nonBlockingRan = false + let nonBlockingRan = NIOLockedValueBox(false) let p = eventLoop.makePromise(of: String.self) p.futureResult.whenFailureBlocking(onto: DispatchQueue.global()) { err in sem.wait() // Block in callback XCTAssertEqual(err as! EventLoopFutureTestError, EventLoopFutureTestError.example) - XCTAssertTrue(nonBlockingRan) + XCTAssertTrue(nonBlockingRan.withLockedValue { $0 }) } p.fail(EventLoopFutureTestError.example) let p2 = eventLoop.makePromise(of: Bool.self) p2.futureResult.whenSuccess { _ in - nonBlockingRan = true + nonBlockingRan.withLockedValue { $0 = true } } p2.succeed(true) @@ -1481,17 +1484,17 @@ class EventLoopFutureTest: XCTestCase { func testWhenCompleteBlockingSuccess() { let eventLoop = EmbeddedEventLoop() let sem = DispatchSemaphore(value: 0) - var nonBlockingRan = false + let nonBlockingRan = NIOLockedValueBox(false) let p = eventLoop.makePromise(of: String.self) p.futureResult.whenCompleteBlocking(onto: DispatchQueue.global()) { _ in sem.wait() // Block in callback - XCTAssertTrue(nonBlockingRan) + XCTAssertTrue(nonBlockingRan.withLockedValue { $0 }) } p.succeed("hello") let p2 = eventLoop.makePromise(of: Bool.self) p2.futureResult.whenSuccess { _ in - nonBlockingRan = true + nonBlockingRan.withLockedValue { $0 = true } } p2.succeed(true) @@ -1501,17 +1504,17 @@ class EventLoopFutureTest: XCTestCase { func testWhenCompleteBlockingFailure() { let eventLoop = EmbeddedEventLoop() let sem = DispatchSemaphore(value: 0) - var nonBlockingRan = false + let nonBlockingRan = NIOLockedValueBox(false) let p = eventLoop.makePromise(of: String.self) p.futureResult.whenCompleteBlocking(onto: DispatchQueue.global()) { _ in sem.wait() // Block in callback - XCTAssertTrue(nonBlockingRan) + XCTAssertTrue(nonBlockingRan.withLockedValue { $0 }) } p.fail(EventLoopFutureTestError.example) let p2 = eventLoop.makePromise(of: Bool.self) p2.futureResult.whenSuccess { _ in - nonBlockingRan = true + nonBlockingRan.withLockedValue { $0 = true } } p2.succeed(true) @@ -1548,11 +1551,11 @@ class EventLoopFutureTest: XCTestCase { let futures = (1...10).map { el.makeSucceededFuture($0) } - var calls = 0 + let calls = NIOLockedValueBox(0) let all = el.makeSucceededFuture(0).foldWithEventLoop(futures) { l, r, el2 in - calls += 1 + calls.withLockedValue { $0 += 1 } XCTAssert(el === el2) - XCTAssertEqual(calls, r) + XCTAssertEqual(calls.withLockedValue { $0 }, r) return el2.makeSucceededFuture(l + r) } diff --git a/Tests/NIOPosixTests/EventLoopTest.swift b/Tests/NIOPosixTests/EventLoopTest.swift index e099be8f1b3..37b44a596ac 100644 --- a/Tests/NIOPosixTests/EventLoopTest.swift +++ b/Tests/NIOPosixTests/EventLoopTest.swift @@ -1585,8 +1585,8 @@ final class EventLoopTest: XCTestCase { var result: Bool? var error: Error? - scheduled.futureResult.whenSuccess { result = $0 } - scheduled.futureResult.whenFailure { error = $0 } + scheduled.futureResult.assumeIsolated().whenSuccess { result = $0 } + scheduled.futureResult.assumeIsolated().whenFailure { error = $0 } scheduled.cancel() @@ -1801,22 +1801,24 @@ final class EventLoopTest: XCTestCase { } let eventLoop = group.next() - var stop = false // no additional synchronisation needed, since only one thread is used - var reExecuteTask: (() -> Void)! - reExecuteTask = { - if !stop { - eventLoop.execute(reExecuteTask) + let stop = try eventLoop.submit { NIOLoopBoundBox(false, eventLoop: eventLoop) }.wait() + + @Sendable + func reExecuteTask() { + if !stop.value { + eventLoop.execute { reExecuteTask() } } } + eventLoop.execute { // SelectableEventLoop runs batches of up to 4096. // Submit significantly over that for good measure. for _ in (0..<10000) { - eventLoop.execute(reExecuteTask) + eventLoop.assumeIsolated().execute(reExecuteTask) } } let stopTask = eventLoop.scheduleTask(in: .microseconds(10)) { - stop = true + stop.value = true } try stopTask.futureResult.wait() } @@ -1852,12 +1854,20 @@ final class EventLoopTest: XCTestCase { } let eventLoop = group.next() - struct Counter { - var submitCount: Int = 0 - var scheduleCount: Int = 0 + struct Counter: Sendable { + private var _submitCount = NIOLockedValueBox(0) + var submitCount: Int { + get { self._submitCount.withLockedValue { $0 } } + nonmutating set { self._submitCount.withLockedValue { $0 = newValue } } + } + private var _scheduleCount = NIOLockedValueBox(0) + var scheduleCount: Int { + get { self._scheduleCount.withLockedValue { $0 } } + nonmutating set { self._scheduleCount.withLockedValue { $0 = newValue } } + } } - var achieved = Counter() + let achieved = Counter() var immediateTasks = [EventLoopFuture]() var scheduledTasks = [Scheduled]() for _ in (0..<100_000) { @@ -1894,12 +1904,20 @@ final class EventLoopTest: XCTestCase { } let eventLoop = group.next() - struct Counter { - var submitCount: Int = 0 - var scheduleCount: Int = 0 + struct Counter: Sendable { + private var _submitCount = NIOLockedValueBox(0) + var submitCount: Int { + get { self._submitCount.withLockedValue { $0 } } + nonmutating set { self._submitCount.withLockedValue { $0 = newValue } } + } + private var _scheduleCount = NIOLockedValueBox(0) + var scheduleCount: Int { + get { self._scheduleCount.withLockedValue { $0 } } + nonmutating set { self._scheduleCount.withLockedValue { $0 = newValue } } + } } - var achieved = Counter() + let achieved = Counter() let (immediateTasks, scheduledTasks) = try eventLoop.submit { var immediateTasks = [EventLoopFuture]() var scheduledTasks = [Scheduled]() @@ -1969,7 +1987,7 @@ final class EventLoopTest: XCTestCase { } } -private class EventLoopWithPreSucceededFuture: EventLoop { +private final class EventLoopWithPreSucceededFuture: EventLoop { var inEventLoop: Bool { true } @@ -2004,27 +2022,34 @@ private class EventLoopWithPreSucceededFuture: EventLoop { preconditionFailure("not implemented") } - var _succeededVoidFuture: EventLoopFuture? + // We'd need to use an IUO here in order to use a loop-bound here (self needs to be initialized + // to create the loop-bound box). That'd require the use of unchecked Sendable. A locked value + // box is fine, it's only tests. + private let _succeededVoidFuture: NIOLockedValueBox?> + func makeSucceededVoidFuture() -> EventLoopFuture { - guard self.inEventLoop, let voidFuture = self._succeededVoidFuture else { + guard self.inEventLoop, let voidFuture = self._succeededVoidFuture.withLockedValue({ $0 }) else { return self.makeSucceededFuture(()) } return voidFuture } init() { - self._succeededVoidFuture = EventLoopFuture(eventLoop: self, value: ()) + self._succeededVoidFuture = NIOLockedValueBox(nil) + self._succeededVoidFuture.withLockedValue { + $0 = EventLoopFuture(eventLoop: self, value: ()) + } } - func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void) { - self._succeededVoidFuture = nil + func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping @Sendable (Error?) -> Void) { + self._succeededVoidFuture.withLockedValue { $0 = nil } queue.async { callback(nil) } } } -private class EventLoopWithoutPreSucceededFuture: EventLoop { +private final class EventLoopWithoutPreSucceededFuture: EventLoop { var inEventLoop: Bool { true } @@ -2059,7 +2084,7 @@ private class EventLoopWithoutPreSucceededFuture: EventLoop { preconditionFailure("not implemented") } - func shutdownGracefully(queue: DispatchQueue, _ callback: @escaping (Error?) -> Void) { + func shutdownGracefully(queue: DispatchQueue, _ callback: @Sendable @escaping (Error?) -> Void) { queue.async { callback(nil) }