Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce Sendability in EventLoopFuture and EventLoopPromise methods #2501

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Enforce Sendability in EventLoopFuture and EventLoopPromise methods #2501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 9 additions & 9 deletions Sources/NIOCore/EventLoopFuture+Deprecated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,61 +15,61 @@
extension EventLoopFuture {
@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func flatMap<NewValue>(file: StaticString = #fileID, line: UInt = #line, _ callback: @escaping (Value) -> EventLoopFuture<NewValue>) -> EventLoopFuture<NewValue> {
public func flatMap<NewValue: Sendable>(file: StaticString = #fileID, line: UInt = #line, _ callback: @Sendable @escaping (Value) -> EventLoopFuture<NewValue>) -> EventLoopFuture<NewValue> {
return self.flatMap(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func flatMapThrowing<NewValue>(file: StaticString = #fileID,
line: UInt = #line,
_ callback: @escaping (Value) throws -> NewValue) -> EventLoopFuture<NewValue> {
_ callback: @Sendable @escaping (Value) throws -> NewValue) -> EventLoopFuture<NewValue> {
return self.flatMapThrowing(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func flatMapErrorThrowing(file: StaticString = #fileID, line: UInt = #line, _ callback: @escaping (Error) throws -> Value) -> EventLoopFuture<Value> {
public func flatMapErrorThrowing(file: StaticString = #fileID, line: UInt = #line, _ callback: @Sendable @escaping (Error) throws -> Value) -> EventLoopFuture<Value> where Value: Sendable {
return self.flatMapErrorThrowing(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func map<NewValue>(file: StaticString = #fileID, line: UInt = #line, _ callback: @escaping (Value) -> (NewValue)) -> EventLoopFuture<NewValue> {
public func map<NewValue>(file: StaticString = #fileID, line: UInt = #line, _ callback: @Sendable @escaping (Value) -> (NewValue)) -> EventLoopFuture<NewValue> {
return self.map(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func flatMapError(file: StaticString = #fileID, line: UInt = #line, _ callback: @escaping (Error) -> EventLoopFuture<Value>) -> EventLoopFuture<Value> {
public func flatMapError(file: StaticString = #fileID, line: UInt = #line, _ callback: @Sendable @escaping (Error) -> EventLoopFuture<Value>) -> EventLoopFuture<Value> where Value: Sendable {
return self.flatMapError(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func flatMapResult<NewValue, SomeError: Error>(file: StaticString = #fileID,
line: UInt = #line,
_ body: @escaping (Value) -> Result<NewValue, SomeError>) -> EventLoopFuture<NewValue> {
_ body: @Sendable @escaping (Value) -> Result<NewValue, SomeError>) -> EventLoopFuture<NewValue> {
return self.flatMapResult(body)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func recover(file: StaticString = #fileID, line: UInt = #line, _ callback: @escaping (Error) -> Value) -> EventLoopFuture<Value> {
public func recover(file: StaticString = #fileID, line: UInt = #line, _ callback: @Sendable @escaping (Error) -> Value) -> EventLoopFuture<Value> {
return self.recover(callback)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func and<OtherValue>(_ other: EventLoopFuture<OtherValue>,
public func and<OtherValue: Sendable>(_ other: EventLoopFuture<OtherValue>,
file: StaticString = #fileID,
line: UInt = #line) -> EventLoopFuture<(Value, OtherValue)> {
return self.and(other)
}

@inlinable
@available(*, deprecated, message: "Please don't pass file:line:, there's no point.")
public func and<OtherValue>(value: OtherValue,
public func and<OtherValue: Sendable>(value: OtherValue,
file: StaticString = #fileID,
line: UInt = #line) -> EventLoopFuture<(Value, OtherValue)> {
return self.and(value: value)
Expand Down
48 changes: 18 additions & 30 deletions Sources/NIOCore/EventLoopFuture+WithEventLoop.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,12 @@ extension EventLoopFuture {
/// - returns: A future that will receive the eventual value.
@inlinable
@preconcurrency
public func flatMapWithEventLoop<NewValue>(_ callback: @escaping @Sendable (Value, EventLoop) -> EventLoopFuture<NewValue>) -> EventLoopFuture<NewValue> {
let next = EventLoopPromise<NewValue>.makeUnleakablePromise(eventLoop: self.eventLoop)
self._whenComplete { [eventLoop = self.eventLoop] in
switch self._value! {
case .success(let t):
let futureU = callback(t, eventLoop)
if futureU.eventLoop.inEventLoop {
return futureU._addCallback {
next._setValue(value: futureU._value!)
}
} else {
futureU.cascade(to: next)
return CallbackList()
}
case .failure(let error):
return next._setValue(value: .failure(error))
}
}
return next.futureResult
public func flatMapWithEventLoop<NewValue: Sendable>(_ callback: @escaping @Sendable (Value, EventLoop) -> EventLoopFuture<NewValue>) -> EventLoopFuture<NewValue> {
// Is this the same thing and still fast?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this one. Probably the previous implementation was faster but I wasn't really sure here

let eventLoop = self.eventLoop
return self.flatMap {
callback($0, eventLoop)
}.hop(to: self.eventLoop)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can delete the .hop(to:). self.flatMap will return a future on self.eventLoop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the previous implementation probably also allocates less, depends on the optimiser a bit. Why not just revert?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Well this method became a lot easier then :D

}

/// When the current `EventLoopFuture<Value>` is in an error state, run the provided callback, which
Expand All @@ -75,20 +62,22 @@ extension EventLoopFuture {
/// - returns: A future that will receive the recovered value.
@inlinable
@preconcurrency
public func flatMapErrorWithEventLoop(_ callback: @escaping @Sendable (Error, EventLoop) -> EventLoopFuture<Value>) -> EventLoopFuture<Value> {
public func flatMapErrorWithEventLoop(_ callback: @escaping @Sendable (Error, EventLoop) -> EventLoopFuture<Value>) -> EventLoopFuture<Value> where Value: Sendable {
let next = EventLoopPromise<Value>.makeUnleakablePromise(eventLoop: self.eventLoop)
let unsafeSelf = UnsafeTransfer(self)
let unsafeNext = UnsafeTransfer(next)
self._whenComplete { [eventLoop = self.eventLoop] in
switch self._value! {
switch unsafeSelf.wrappedValue._value! {
case .success(let t):
return next._setValue(value: .success(t))
return unsafeNext.wrappedValue._setValue(value: .success(t))
case .failure(let e):
let t = callback(e, eventLoop)
if t.eventLoop.inEventLoop {
return t._addCallback {
next._setValue(value: t._value!)
unsafeNext.wrappedValue._setValue(value: t._value!)
}
} else {
t.cascade(to: next)
t.cascade(to: unsafeNext.wrappedValue)
return CallbackList()
}
}
Expand All @@ -113,16 +102,15 @@ extension EventLoopFuture {
/// - with: A function that will be used to fold the values of two `EventLoopFuture`s and return a new value wrapped in an `EventLoopFuture`.
/// - returns: A new `EventLoopFuture` with the folded value whose callbacks run on `self.eventLoop`.
@inlinable
@preconcurrency
public func foldWithEventLoop<OtherValue>(
_ futures: [EventLoopFuture<OtherValue>],
with combiningFunction: @escaping @Sendable (Value, OtherValue, EventLoop) -> EventLoopFuture<Value>
) -> EventLoopFuture<Value> {
func fold0(eventLoop: EventLoop) -> EventLoopFuture<Value> {
) -> EventLoopFuture<Value> where Value: Sendable, OtherValue: Sendable { // This is a breaking change
let fold0: @Sendable (EventLoop) -> EventLoopFuture<Value> = { (eventLoop: EventLoop) in
let body = futures.reduce(self) { (f1: EventLoopFuture<Value>, f2: EventLoopFuture<OtherValue>) -> EventLoopFuture<Value> in
let newFuture = f1.and(f2).flatMap { (args: (Value, OtherValue)) -> EventLoopFuture<Value> in
let (f1Value, f2Value) = args
self.eventLoop.assertInEventLoop()
eventLoop.assertInEventLoop()
return combiningFunction(f1Value, f2Value, eventLoop)
}
assert(newFuture.eventLoop === self.eventLoop)
Expand All @@ -132,11 +120,11 @@ extension EventLoopFuture {
}

if self.eventLoop.inEventLoop {
return fold0(eventLoop: self.eventLoop)
return fold0(self.eventLoop)
} else {
let promise = self.eventLoop.makePromise(of: Value.self)
self.eventLoop.execute { [eventLoop = self.eventLoop] in
fold0(eventLoop: eventLoop).cascade(to: promise)
fold0(eventLoop).cascade(to: promise)
}
return promise.futureResult
}
Expand Down