Unless I am not understanding some fundamental part of how Clock is designed to work, this sounds like either a design flaw with the pitch or possibly Clock needs to be improved. Are there examples in the Swift stdlib with a concrete requirement of ContinuousClock?
Edit: Perhaps I’m misreading something - would this proposed API just work as-is with a SuspendingClock?
Just thought I would put this here: I am using GitHub - swhitty/swift-timeout: Swift Concurrency timeout handler · GitHub successfully today with some interesting extensions on the concept:
TimeoutError is thrown.Clock (not to be confused with any Clock)withThrowingTimeoutthat passes in a TimeoutController.try await withThrowingTimeout(after: .now + .seconds(2.0)) { timeout in
try await foo()
timeout.cancelExpiration()
try await bar()
timeout.expire(after: .now + .seconds(0.5))
try await baz()
}
AsyncTimeoutSequence which allows you to set a timeout on the next()of an AsyncSequence like:for try await val in someAsyncSequence.timeout(duration: .seconds(2.0)) {
...
}
I think there are two constraints/requirements at play here that make it hard to come up with a single solution that fulfills both:
The problem here is that supporting generic clocks means that propagating the deadline becomes almost impossible since clocks can have different duration and instant types. An approach could be to constrain the duration of the clocks for deadline to Swift.Duration; however, this means that a lot of "test" clocks won't work since they often have their own definition for duration and instant.
In my initial pitch, I decided not to include propagation of the deadline at all since deadlines result in the task being cancelled. Cancellation is already propagated down the call stack, and everything is expected to respect cancellation. The motivating examples for propagation that I have heard are to communicate the deadline across processes such as XPC or network requests. This would allow the other process to enforce the deadline locally. Since task cancellation is already expected to cancel any such requests, this would strictly speaking be an optimization. However, that optimization might still be worth solving.
To allow for both options of local timeouts with any clock and propagation of timeouts with absolute time, could you combine the two approaches? Have withTimeout accept a Clock and suspend according to that clock’s semantics (possibly with special case handling for ContinuousClock? @ktoso mentioned some implementation optimisations that I assume are only possible for this clock). But also set the task local deadline only if the clock is ContinuousClock (where the deadline is an absolute point in time that would make sense when propagated to another system)
I think that is an OK tradeoff; the test clocks would just have to use Swift.Duration - which can be incremented in similar quanta to a non Swift.Duration and the comparison to run things is just sorted by a comparison to the now and the proposed Instant. It 100% isn't impossible (since I have a test clock for testing these APIs drafted already; however making that fully robust for production might be a tall order for supporting generality).
I agree. If people have existing test clocks for other purposes, it would seem unreasonable for them to expect them to work in other / new situations.
Building a test / mock clock that supports this api seems an entirely reasonable constraint.
Swift Concurrency’s core design strongly signals an intent to support tasks actors and executors with custom clocks. Introducing an API that only works if the task only ever executes on actors that use ContinuousClock seems like a significant retreat from that design goal.
Since this API is implemented atop cancellation, which any task is free to ignore and/or suppress from propagating to its child tasks, I believe it is reasonable to allow this API to fail in certain scenarios for the sake of extending compatibility to other Clock types.
The specific changes I would recommend for this proposal are:
withAutomaticCancellation()This makes it more obvious that the Task may successfully complete even if it observes deadline expiry.
Compelling reasons for this have already been given by people who are actively using custom Clocks in testing.
I am unconvinced that withAutomaticCancellation() must always await the original Task before returning. The cases where it’s needed for program correctness are cases where the user must already avoid wrapping their work in Task.detached { }.
I believe withAutomaticCancellation() should return immediately after canceling the task if the deadline expires. The return value will contain the wrapped Task, so that the user can choose to await its value if necessary for program correctness. But in cases where the work can continue executing in a detached state, the application can simply ignore the Task and avoid the penalty of synchronizing on it.
For the name, I would suggest withTaskCancellationAfter(deadline:tolerance:operation:) and a convenience method withTaskCancellationAfter(timeout:tolerance:operation:). In my opinion, this fits nicely with other with-style APIs , i.e., withTaskCancellationHandler and withTaskCancellationShield.
Task100% against this. In the previous thread I already explained why I see the cancellation as a must.
The ugly truth about the APIs that are “optional” is that most of the users will never do the “optional” thing. This is the same situation as when you make something “required” (like closing the file descriptor): people will forget it. APIs should try to prevent bugs “by design” by doing the “right thing” by default. In this situation, cancelling the child Task is the correct (and I would argue the ONLY) choice.
Judging by the feedback in this thread, the proposal authors may come to the conclusion that the community favors the with[Automatic]Cancellation naming. I just want to say that I'm perfectly fine with withDeadline.
I feel like there is an overlap between the people who want the rename and those who suggest Tash.detached (or similar “Task outlives the withDeadline call” solution). For me, from the very beginning, it was natural that withDeadline will have to cancel the child Task before returning, which may be the reason why I see withDeadline as a suitable name. Cancellation is implied, because this is how it should work in cooperative multitasking.
Btw. If you want the child Task to outlive the withDeadline call then you should not use the with[Resource] naming convention.
Btw. Any official stance on the timeouts that are not connected to any Clock? (Mainly about the DeadlineError containing the instant.)
I agree that we shouldn't break structured concurrency by returning a detached task. That's a non-starter IMO, the user, if they really want to detach a task, already has an explicit API for it in Task.detached.
There is no way to combine Task.detached with this proposed API in a way that achieves the goal that I believe most users will want: freeing up the awaiting task when the deadline expires.
I don't think that is within the scope of this proposal. However, I have been convinced to change the proposal to be generic on the clock (with the restriction that the clock must have a duration type of Swift.Duration).
If the actual deadline-firing mechanism is offloaded to a requirement of a new Clock-refining protocol, could this method be constrained to any Clock that implements that refined protocol?
protocol AlarmClock: Clock {
// Can’t quite decide which attribute applies to body…
func alarm(at: Instant, body: @concurrent (clock: Self) -> Void)
}
// using name and semantics as pitched:
func withDeadline<C: AlarmClock>(on clock: C = ContinuousClock(), at: C.Instant, tolerance: C.Instant.Duration? = nil, body: nonisolated(nonsending) () async -> Void ) { }
Related, is the fact that the pitch takes a Duration instead of an Instant deadline a typo?
A new protocol is not sufficient to create a minimum deadline on its own (unless there was an additional method to do that calculation, which I am not sure would be ideal to expose since it would lead to inaccuracies in the wrong place). Furthermore the attribution of that body closure kinda runs rough shot along some spaces that I dont think we even have ways to express yet.
I will be updating the proposal this week to reflect the adjustment to the clocks; there is an overload that uses duration but the primary funnel method should be listed as an Instant (it seems to be in the pitch).
This is the new signature that I will be updating the proposal to be:
func withDeadline<C: Clock, Return, Failure: Error>(
_ expiration: C.Instant,
tolerance: C.Instant.Duration? = nil,
body: nonisolated(nonsending) () async throws(Failure) -> Return
) async throws(DeadlineError<Failure>) -> Return where C.Instant.Duration == Swift.Duration
I didn’t think I was entering unexplored territory… for context, here’s how I was envisioning it being used:
struct SteppingClock: AlarmClock {
struct Instant: Hashable, Equatable {
var tick: Int
func duration(to other: Instant) -> Int { other.tick - tick }
func advanced(by duration: Int) -> Instant { Instant(tick: tick + duration) }
static func==(lhs: Instant, rhs: Instant) -> Bool { lhs.tick == rhs.tick }
}
private var alarms: [Instant: [(SteppingClock) -> Void]]
public private(set) var now: Instant
func alarm(at: Instant, body: @concurrent (clock: SteppingClock) -> Void) {
// body must be responsible for switching to the appropriate executor.
// obv. this won’t compile as a nonmutating func; imagine thread-safe, out of line storage for alarms.
var newAlarms = alarms[at] ?? []
newAlarms.append(body)
alarms[at] = newAlarms
}
mutating func step() {
now = now.advanced(by: 1)
for alarm in alarms[now] ?? [] {
alarm(self)
}
}
}