Skip to content

Commit

Permalink
Enforce Sendability in EventLoopFuture and EventLoopPromise methods
Browse files Browse the repository at this point in the history 
# Motivation
With #2496 we fixed a Sendability checking hole by removing the unconditional conformance of `EventLoopFuture/Promise` to `Sendable`. This was the correct thing to do; however, it has the fallout that a couple of methods are now rightly complaining that their values are send across isolation domains.

# Modification
This PR requires values on some `ELF/P` methods to be `Sendable` when we might potentially transfer the values across isolation domains/ELs. We have to be overly aggressive here because we do not know that some `ELF` method are staying on the same EL. For example `flatMap` gets a new `ELF` from the closure provided to it. If the `ELF` is on the same EL we do not need to hop; however, we can not guarantee this right now from a type level so we have to stay on the safe side and actually require the `NewValue` to be `Sendable`.

# Result
This PR makes us more correct from a Sendability perspective but produces warnings for some safe patterns that are currently in use.
  • Loading branch information
@FranzBusch
FranzBusch committed Aug 11, 2023
1 parent 8c2654c commit 05ec0a9
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 125 deletions.
18 changes: 9 additions & 9 deletions Sources/NIOCore/EventLoopFuture+Deprecated.swift
View file
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
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?
let eventLoop = self.eventLoop
return self.flatMap {
callback($0, eventLoop)
}.hop(to: self.eventLoop)
}

/// 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
Loading

0 comments on commit 05ec0a9

Please sign in to comment.