From aadf850d5e3abdfdfa06c50de41a6a52f3972878 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Thu, 13 Feb 2025 14:32:06 +0000 Subject: [PATCH 1/5] Add more methods to work with Isolated futures Motivation: Whilst working on the NIOHTTP1 strict concurrency work I encountered a few gaps in the API where I needed isolated analogues of methods on futures. Modifications: * Add `makeSucceededIsolatedFuture` * Add a new internal initializer for `EventLoopFuture` which does not require the value to be Sendable for use in isolated contexts. * Add a variant of `flatMapError` which can handle closures which return isolated futures * Add a `futureResult` computed property which allows us to avoid the otherwise clumsy `future.nonIsolated().futureResult.assumeIsolated()` Result: More ability to work with isolated futures. --- Sources/NIOCore/EventLoop.swift | 16 +++++++ .../EventLoopFuture+AssumeIsolated.swift | 43 +++++++++++++++++++ Sources/NIOCore/EventLoopFuture.swift | 10 +++++ .../EventLoopFutureIsolatedTests.swift | 13 ++++++ Tests/NIOPosixTests/EventLoopFutureTest.swift | 37 ++++++++++++++++ 5 files changed, 119 insertions(+) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index d31640b0ada..ae64d8dc7bf 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -1107,6 +1107,22 @@ extension EventLoop { } } + /// Creates and returns a new isolated `EventLoopFuture` that is already marked as success. Notifications will be done using this `EventLoop` as execution `NIOThread`. + /// + /// - Parameters: + /// - value: the value that is used by the `EventLoopFuture.Isolated`. + /// - Returns: a succeeded `EventLoopFuture.Isolated`. + @inlinable + @available(*, noasync) + public func makeSucceededIsolatedFuture(_ value: Success) -> EventLoopFuture.Isolated { + if Success.self == Void.self { + // The as! will always succeed because we previously checked that Success.self == Void.self. + return self.makeSucceededVoidFuture().assumeIsolated() as! EventLoopFuture.Isolated + } else { + return EventLoopFuture.Isolated(_wrapped: EventLoopFuture(eventLoop: self, isolatedValue: value)) + } + } + /// Creates and returns a new `EventLoopFuture` that is marked as succeeded or failed with the value held by `result`. /// /// - Parameters: diff --git a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift index 7e7dd9f741f..b54225f14c2 100644 --- a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift +++ b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift @@ -389,6 +389,43 @@ extension EventLoopFuture { return next.futureResult.assumeIsolatedUnsafeUnchecked() } + /// When the current `EventLoopFuture.Isolated` is in an error state, run the provided callback, which + /// may recover from the error by returning an `EventLoopFuture.Isolated`. The callback is intended to potentially + /// recover from the error by returning a new `EventLoopFuture.Isolated` that will eventually contain the recovered + /// result. + /// + /// If the callback cannot recover it should return a failed `EventLoopFuture.Isolated`. + /// + /// - Note: The `Value` need not be `Sendable` since the isolation domains of this future and the future returned from the callback + /// must be the same + /// + /// - Parameters: + /// - callback: Function that will receive the error value of this `EventLoopFuture.Isolated` and return + /// a new value lifted into a new `EventLoopFuture.Isolated`. + /// - Returns: A future that will receive the recovered value. + @inlinable + @available(*, noasync) + public func flatMapError( + _ callback: @escaping (Error) -> EventLoopFuture.Isolated + ) -> EventLoopFuture.Isolated { + let next = EventLoopPromise.makeUnleakablePromise(eventLoop: self._wrapped.eventLoop) + let base = self._wrapped + + base._whenCompleteIsolated { + switch base._value! { + case .success(let t): + return next._setValue(value: .success(t)) + case .failure(let e): + let t = callback(e) + t._wrapped.eventLoop.assertInEventLoop() + return t._wrapped._addCallback { + next._setValue(value: t._wrapped._value!) + } + } + } + return next.futureResult.assumeIsolatedUnsafeUnchecked() + } + /// When the current `EventLoopFuture` is fulfilled, run the provided callback, which /// performs a synchronous computation and returns either a new value (of type `NewValue`) or /// an error depending on the `Result` returned by the closure. @@ -643,6 +680,11 @@ extension EventLoopPromise { self._wrapped = _wrapped } + @usableFromInline + var futureResult: EventLoopFuture.Isolated { + self._wrapped.futureResult.assumeIsolated() + } + /// Deliver a successful result to the associated `EventLoopFuture` object. /// /// - Parameters: @@ -708,3 +750,4 @@ extension EventLoopPromise { @available(*, unavailable) extension EventLoopPromise.Isolated: Sendable {} + diff --git a/Sources/NIOCore/EventLoopFuture.swift b/Sources/NIOCore/EventLoopFuture.swift index 482bdb5afdf..d047f23f9ab 100644 --- a/Sources/NIOCore/EventLoopFuture.swift +++ b/Sources/NIOCore/EventLoopFuture.swift @@ -437,6 +437,16 @@ public final class EventLoopFuture { self._callbacks = .init() } + /// A EventLoopFuture that has already succeeded with an isolated (not-necessarily-sendable) value + @inlinable + internal init(eventLoop: EventLoop, isolatedValue value: Value) { + eventLoop.assertInEventLoop() + + self.eventLoop = eventLoop + self._value = .success(value) + self._callbacks = .init() + } + /// A EventLoopFuture that has already failed @inlinable internal init(eventLoop: EventLoop, error: Error) { diff --git a/Tests/NIOPosixTests/EventLoopFutureIsolatedTests.swift b/Tests/NIOPosixTests/EventLoopFutureIsolatedTests.swift index 705d6319700..4b2ec8874f1 100644 --- a/Tests/NIOPosixTests/EventLoopFutureIsolatedTests.swift +++ b/Tests/NIOPosixTests/EventLoopFutureIsolatedTests.swift @@ -288,6 +288,19 @@ final class EventLoopFutureIsolatedTest: XCTestCase { } XCTAssertEqual(r, originalValue.x - 1) } + throwingFuture.map { _ in 5 }.flatMapError { (error: any Error) -> EventLoopFuture.Isolated in + guard let error = error as? TestError, error == .error else { + XCTFail("Invalid passed error: \(error)") + return loop.makeSucceededIsolatedFuture(originalValue.x) + } + return loop.makeSucceededIsolatedFuture(originalValue.x - 2) + }.whenComplete { (result: Result) in + guard case .success(let r) = result else { + XCTFail("Unexpected error") + return + } + XCTAssertEqual(r, originalValue.x - 2) + } // This block handles unwrap. newFuture.map { x -> SuperNotSendable? in diff --git a/Tests/NIOPosixTests/EventLoopFutureTest.swift b/Tests/NIOPosixTests/EventLoopFutureTest.swift index 8ef68fe6378..386c7366e30 100644 --- a/Tests/NIOPosixTests/EventLoopFutureTest.swift +++ b/Tests/NIOPosixTests/EventLoopFutureTest.swift @@ -1110,6 +1110,30 @@ class EventLoopFutureTest: XCTestCase { XCTAssertEqual(try promise.futureResult.wait(), "yay") } + func testFutureFulfilledIfHasNonSendableResult() throws { + let eventLoop = EmbeddedEventLoop() + let f = EventLoopFuture(eventLoop: eventLoop, isolatedValue: NonSendableObject(value: 5)) + XCTAssertTrue(f.isFulfilled) + } + + func testSucceededIsolatedFutureIsCompleted() throws { + let group = EmbeddedEventLoop() + let loop = group.next() + + let value = NonSendableObject(value: 4) + + let future = loop.makeSucceededIsolatedFuture(value) + + future.whenComplete { result in + switch result{ + case .success(let nonSendableStruct): + XCTAssertEqual(nonSendableStruct, value) + case .failure(let error): + XCTFail("\(error)") + } + } + } + func testPromiseCompletedWithFailedFuture() throws { let group = EmbeddedEventLoop() let loop = group.next() @@ -1652,3 +1676,16 @@ class EventLoopFutureTest: XCTestCase { XCTAssertNotEqual(promise3, promise2) } } + +class NonSendableObject: Equatable { + var value: Int + init(value: Int) { + self.value = value + } + + static func == (lhs: NonSendableObject, rhs: NonSendableObject) -> Bool { + lhs.value == rhs.value + } +} +@available(*, unavailable) +extension NonSendableObject: Sendable {} From 9e84f10b05a77bd57b9aabdd7ff9c68e630780de Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Fri, 14 Feb 2025 09:19:23 +0000 Subject: [PATCH 2/5] Update Sources/NIOCore/EventLoop.swift Co-authored-by: Johannes Weiss --- Sources/NIOCore/EventLoop.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index ae64d8dc7bf..efc02fd6559 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -1107,7 +1107,7 @@ extension EventLoop { } } - /// Creates and returns a new isolated `EventLoopFuture` that is already marked as success. Notifications will be done using this `EventLoop` as execution `NIOThread`. + /// Creates and returns a new isolated `EventLoopFuture` that is already marked as success. Notifications will be done using this `EventLoop. /// /// - Parameters: /// - value: the value that is used by the `EventLoopFuture.Isolated`. From 99ffc07bd4f6eb960d143846ffdb418450be4246 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Fri, 14 Feb 2025 10:01:56 +0000 Subject: [PATCH 3/5] make EventLoopFuture.Isolated.futureResult public --- Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift index b54225f14c2..caae2bc5945 100644 --- a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift +++ b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift @@ -680,8 +680,9 @@ extension EventLoopPromise { self._wrapped = _wrapped } - @usableFromInline - var futureResult: EventLoopFuture.Isolated { + /// Returns the `EventLoopFuture.Isolated` which will be notified once the execution of the scheduled task completes. + @inlinable + public var futureResult: EventLoopFuture.Isolated { self._wrapped.futureResult.assumeIsolated() } From 0d5d9801ee2fdd2517f82df9f4da9b944590f4b4 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Fri, 14 Feb 2025 10:17:46 +0000 Subject: [PATCH 4/5] formatting --- Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift | 1 - Tests/NIOPosixTests/EventLoopFutureTest.swift | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift index caae2bc5945..e767b28dafc 100644 --- a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift +++ b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift @@ -751,4 +751,3 @@ extension EventLoopPromise { @available(*, unavailable) extension EventLoopPromise.Isolated: Sendable {} - diff --git a/Tests/NIOPosixTests/EventLoopFutureTest.swift b/Tests/NIOPosixTests/EventLoopFutureTest.swift index 386c7366e30..b5c933a528c 100644 --- a/Tests/NIOPosixTests/EventLoopFutureTest.swift +++ b/Tests/NIOPosixTests/EventLoopFutureTest.swift @@ -1125,7 +1125,7 @@ class EventLoopFutureTest: XCTestCase { let future = loop.makeSucceededIsolatedFuture(value) future.whenComplete { result in - switch result{ + switch result { case .success(let nonSendableStruct): XCTAssertEqual(nonSendableStruct, value) case .failure(let error): From d2abde92a22c3771074e6bab8d87d20df0d80668 Mon Sep 17 00:00:00 2001 From: Rick Newton-Rogers Date: Fri, 14 Feb 2025 13:24:32 +0000 Subject: [PATCH 5/5] noasync --- Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift index e767b28dafc..d71fb823faa 100644 --- a/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift +++ b/Sources/NIOCore/EventLoopFuture+AssumeIsolated.swift @@ -682,6 +682,7 @@ extension EventLoopPromise { /// Returns the `EventLoopFuture.Isolated` which will be notified once the execution of the scheduled task completes. @inlinable + @available(*, noasync) public var futureResult: EventLoopFuture.Isolated { self._wrapped.futureResult.assumeIsolated() }