-
Notifications
You must be signed in to change notification settings - Fork 178
Add race, timeout, and deadline async functions #343
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.
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
base: main
Are you sure you want to change the base?
Conversation
There is a discussion already going on here: |
//===----------------------------------------------------------------------===// | ||
|
||
/// Returns the value or throws an error, from the first completed or failed operation. | ||
public func race(_ operations: (@Sendable () async throws -> Void)...) async throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All those closure's can be sending
since we move them into a child task. Also do we really need these overloads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding Void
overloads. If you do this:
let u: () async throws -> Void = {
return try await race {
let _ = 2
}
}
You get:
Cannot convert value of type '()?' to closure result type 'Void'
Interestingly, if you don't use the return
keyword and rely on inference, this compiles 😄
} | ||
|
||
/// Returns the value or throws an error, from the first completed or failed operation. | ||
public func race<T: Sendable>(_ operations: [@Sendable () async throws -> T]) async throws -> T? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using an array here we might be better off making this generic by doing some Sequence<sending () asynchronous throws -> T>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot use sending
for the closure - this gives an error: sending' may only be used on parameters and results
.
The same for the comment above regarding the variadic parameter.
If I do this:
_ operations: sending some Sequence<() async throws -> T>
then the compiler gives a warning at:
group.addTask { try await operation() }
saying:
Passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure; this is an error in the Swift 6 language mode
Sending a sequence of closures doesn't imply closures as sending, which makes total sense, this information would need to be passed through generics. Would this require to have @Sending
to be added to the language?
} | ||
|
||
/// Returns the value or throws an error, from the first completed or failed operation. | ||
public func race<T: Sendable>(_ operations: (@Sendable () async throws -> T?)...) async throws -> T? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need optional overloads here at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do need them. Otherwise, if operations
return an optional type, you end up with double optional T??
due to the race function returning the result of try await group.next()
, which is an optional. So these overloads are here to flatten this conveniently.
If `customError` is `nil`, then `CancellationError` is thrown. | ||
*/ | ||
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) | ||
public func withTimeout<Success: Sendable>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's the shape of how this method should look like. Ideally we end up with something closer to this:
nonisolated(nonsending) public func withTimeout<T: Sendable, Clock: _Concurrency.Clock>(
in timeout: Clock.Duration,
clock: Clock,
body: nonisolated(nonsending) () async throws -> T
) async throws -> T
Importantly, here the body closure is both non-Sendable
and non-escaping
. This allows to easily compose it with surrounding context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nonisolated(nonsending)
is a Swift 6.2 feature. This package supports Swift 5.8. Do we want these methods to be 6.2+ only or define a different API for previous Swift versions?
This PR adds simple race, timeout, and deadline async functions.
I believe these are commonly implemented by anyone who uses async-await and would be a welcomed addition.