Runtime: Add `Task.WithTimeout(TimeSpan)` method

[AArnott]

A very popular StackOverflow Q&A asks how to await a Task with a timeout. A simple Task.WithTimeout(TimeSpan) method would alleviate a lot of time developers spend looking for how to properly do this, and avoid doing it wrong (e.g. leaving timers for finalization).

See how we offer a WithTimeout extension method from vs-threading.

[benaadams]

Similar code is already in corefx; but only used for running tests rather than exposed as api

Task TimeoutAfter(this Task task, int millisecondsTimeout)
Task<TResult> TimeoutAfter<TResult>(this Task<TResult> task, int millisecondsTimeout)

https://github.com/dotnet/corefx/blob/407c09d5840384fbd24bbd5b8869ef13be71de63/src/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs#L28-L43

[tarekgh]

CC @stephentoub

[eiriktsarpalis]

I need to use this for production code in https://github.com/dotnet/runtime/pull/42585, so might as well champion it for API review. Will come up with a proposal shortly.

[eiriktsarpalis]

namespace System.Threading.Tasks
{
    public static class TaskTimeoutExtensions
    {
        // throws TaskCanceledException
        public static Task WithCancellation(this Task task, CancellationToken cancellationToken) => throw null;
        public static Task<TResult> WithCancellation<TResult>(this Task<TResult> task, CancellationToken cancellationToken) => throw null;

        // throws TimeoutException
        public static Task WithTimeout(this Task task, TimeSpan timeout) => throw null;
        public static Task<TResult> WithTimeout<TResult>(this Task<TResult> task, TimeSpan timeout) => throw null;

        public static Task WithTimeout(this Task task, int millisecondsTimeout) => throw null;
        public static Task<TResult> WithTimeout<TResult>(this Task<TResult> task, int millisecondsTimeout) => throw null;
    }
}

I haven't included ValueTask overloads, since it should be straightforward to convert them to task instances.

[pentp]

The comment about throwing TaskCanceledException means that the Task will be marked as canceled, not faulted, right?

[stephentoub]

I think there are several questions that need to be answered before considering a concrete API.

One is key use cases. One of the reasons we added ConfigureAwait as we did was the idea that we might in the future add additional overloads for things exactly like cancellation and timeouts, e.g.
C# await task.ConfigureAwait(false, cancellationToken);
There are pros and cons to this, in particular on the plus side is we don't actually need to allocate a Task and are free to do whatever we can internally to make this as efficient as possible (and it arguably makes the semantics clearer, that it's specifically about the await operation rather than about the task itself). The primary con is it's not as composable: if you get back a Task, you can use it as you would any other task, in many other situations than you could the awaitable struct returned from ConfigureAwait.

Regarding ValueTask, it's a middle ground: it gives us a bit more flexibility in how we might optimize the implementation, at the expense of usability; if we expect the overwhelming use case to be await task.WithWhatever, then ValueTask could be the right choice instead of (not in addition to) Task. If we expect it to be something else, then Task is the right answer.

There's also the question of having both timeouts and cancellation tokens that you want to use together, i.e. should these really be separately named methods, one for cancellation and one for timeout, or should a single API (maybe with overloads) let you utilize both at the same time, ala Task.Delay(int, cancellationToken).

[eiriktsarpalis]

I like the ConfigureAwait overload approach, since it more clearly communicates that it is not the underlying task that is being cancelled, but it only concerns this particular awaiter.

[GSPP]

What is the policy with respect to specifying the timeout as int millisecondsTimeout or as a TimeSpan? It seems to me that if .NET could be done "greenfield" there only should be TimeSpan and never an int. But there's an argument for keeping consistency with existing APIs.

Has it been decided on how new APIs with a timeout should be designed?

My opinion: Something like await Task.Delay(100) is an anti-pattern. All code should use the TimeSpan form. New APIs should only use TimeSpan because getting rid of that unfortunate timeout style is more important than keeping consistency.

So maybe the int millisecondsTimeout overloads should be removed from this proposal.

[stephentoub]

I like the ConfigureAwait overload approach

Me, too. I would, though, like to hear from folks if there are compelling use cases for the WithTimeout/Cancellation methods where the result isn't immediately awaited. If there are meaningful such situations, that could move us in that direction, or possibly we could do both approaches (they're not mutually exclusive).

[stephentoub]

What is the policy with respect to specifying the timeout as int millisecondsTimeout or as a TimeSpan?

@bartonjs, have we drawn any lines in the sand here?

[eiriktsarpalis]

Me, too. I would, though, like to hear from folks if there are compelling use cases for the WithTimeout/Cancellation methods where the result isn't immediately awaited.

It should be fairly trivial to write such extension methods when an equivalent ConfigureAwait overload is available.

[stephentoub]

It should be fairly trivial to write such extension methods when an equivalent ConfigureAwait overload is available.

That's a good point, it's a one-liner. It'd have a bit more overhead, but unlikely to be prohibitive, especially since it'd almost certainly be less than existing solutions folks use today.

[eiriktsarpalis]

We might still need to expose those extension methods for the benefit of languages like F# whose async implementation doesn't have first-class support for C#-style awaitables.

[pentp]

It would probably still be worth the effort to implement the Task returning extensions without the additional exception throwing involved from async/await.

[stephentoub]

We might still need to expose those extension methods for the benefit of languages like F# whose async implementation doesn't have first-class support for C#-style awaitables.

Have you seen requests for this functionality from folks using F#?

It would probably still be worth the effort to implement the Task returning extensions without the additional exception throwing involved from async/await.

It's not clear to me whether there's a real need for such methods for anything other than directly awaiting. Hence my question about use cases. But if there is, even if these did avoid one throw, it's likely only one of many, so I'm not sure it would make a meaningful difference. I'd be interested in learning otherwise.

[eiriktsarpalis]

Have you seen requests for this functionality from folks using F#?

If you mean WithTimeout-like functionality, then yes, it's pretty common to knock together such methods in "Utils" modules.

[pentp]

Example WithTimeout, WithCancellation.
And a third variation that is used to reduce the number of exceptions thrown - WhenCancelled, related API proposal #37505, example use case:
C# var probeCancellation = cancellation.WhenCancelled(); var probeTask = prober.Probe(SiloAddress, diagnosticProbeNumber); var task = await Task.WhenAny(stopping, probeCancellation, probeTask);
These implementations involve additional exception throwing/catching because they are built using async/await, but it would be possible to implement them more efficiently so that no exceptions are thrown inside the implementation (only in the consuming code if it uses await).

[stephentoub]

Example WithTimeout, WithCancellation

I'm not following. Searching the repo, every use of those I see is directly awaited.

[pentp]

Yes, that's the common case and I was just providing examples for the current situation that's suboptimal. I'm also in favour of the ConfigureAwait approach. But if we're going to implement Task returning variants also, then I would expect them to not throw exceptions internally.

[stephentoub]

But if we're going to implement Task returning variants also, then I would expect them to not throw exceptions internally.

Oh, yes, totally agree. Implementing them in terms of ConfigureAwait is just an easy way to get that functionality if such methods aren't exposed.

[bartonjs]

What is the policy with respect to specifying the timeout as int millisecondsTimeout or as a TimeSpan?

• DO prefer using TimeSpan to represent timeout time.

But there's an argument for keeping consistency with existing APIs.

That's one of the reasons for not choosing TimeSpan, yes :smile:. Especially if it's an overload of an existing method, or would be the only member on a type that uses TimeSpan while everything else uses int.

Has it been decided on how new APIs with a timeout should be designed?

In 4.0? Yep. TimeSpan, unless there's a good reason not to. (Such as the quantum of time from an underlying API call is in minutes, and no one would expect that TimeSpan.FromSeconds(2) is either 0 or 1 minute).

After 4.5? A little more tricky, since there's some asymmetry between synchronous methods (TimeSpan parameter) and asynchronous methods (CancellationToken can already represent this... how do we reconcile? (sad answer: we do a lot of metaphorical beard-scratching and harrumphing and call it a day with no answer)). Since this is effectively bridging the two, it seems like TimeSpan is the clear winner.

@bartonjs, have we drawn any lines in the sand here?

Yep. But, as you can see, the sand is somewhat dry.

[fowl2]

It would be great if both timeouts and deadlines could be supported with the same technique. Deadlines being a core part of gRPC, and others.

[stephentoub]

It would be great if both timeouts and deadlines could be supported with the same technique. Deadlines being a core part of gRPC, and others.

Thinking about deadlines is interesting. But I'd rather see a single helper added somewhere, e.g. along the lines of:
C# public static class Timeout { public static TimeSpan FromDeadline(DateTimeOffset deadline) { TimeSpan timeout = deadline - DateTime.UtcNow; return timeout < TimeSpan.Zero ? TimeSpan.Zero : timeout; } ... }
rather than add yet another set of overloads to every method throughout the system that takes a timeout. That would be a separate proposal.

[benaadams]

I'd go with weaker wording than Deadline as it's going to start drifting between getting it and using it; also the triggering won't be exact (15 ms range?).

[stephentoub]

Let's keep such a proposal separate. I don't think it should impact this.

[eiriktsarpalis]

TIL IAsyncEnumerable already supports this, I suppose we should probably use that API as a template for any Task/ValueTask proposal?

[stephentoub]

TIL IAsyncEnumerable already supports this

Sort of: it has that API, but it does something different.

It's there because the GetAsyncEnumerator interface accepts a CancellationToken, but if you're using the language await foreach support, you're not explicitly calling GetAsyncEnumerator, rather code generated for you is doing so... so this extension method exists to let you pass a token through. And while it could have just been called ConfigureAwait, it doesn't because it's not just about adding ConfigureAwait onto awaits, but rather what arguments are passed to GetAsyncEnumerator.

As such, it's then different from what's being discussed in this issue: it's not the equivalent of using the proposed WithCancellation on each await. If, for example, an iterator ignored its [EnumeratorCancellation] argument, the CancellationToken supplied to the WithCancellation would be a nop. In contrast, the one in this proposal would actually cancel the await even as the underlying operation represented by the task may still be in flight.

[eiriktsarpalis]

Here's what an API for cancellable task awaitables could look like (wlog for the case of Task<T>):

namespace System.Runtime.CompilerServices
{
    public readonly struct ConfiguredCancelableTaskAwaitable<TResult>
    {
        public ConfiguredCancelableTaskAwaitable<TResult>.ConfiguredCancelableTaskAwaiter GetAwaiter();

        public readonly struct ConfiguredCancelableTaskAwaiter : ICriticalNotifyCompletion
        {
            public bool IsCompleted { get; }
            public TResult GetResult();
            public void OnCompleted(Action continuation);
            public void UnsafeOnCompleted(Action continuation);
        }
    }
}

namespace System.Threading.Tasks
{
    public class Task<T>
    {
        public ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(bool continueOnCapturedContext, CancellationToken cancellationToken);
    }
}

@stephentoub suggested we incorporate the AwaitBehavior enum proposal into this design. It should be possible to embed it into the existing ConfiguredTaskAwaitable struct without any breaking changes, so assuming we go ahead with it we should also include the following APIs:

namespace System.Threading.Tasks
{
    [Flags]
    public enum ConfigureAwaitBehavior
    {
        NoCapturedContext = 0x1, // equivalent to ConfigureAwait(false)
        NoThrow = 0x2, // when set, no exceptions will be thrown for Faulted/Canceled
        Asynchronous = 0x4, // force the continuation to be asynchronous
        ...
    }

    public class Task<T>
    {
        public ConfiguredTaskAwaitable<TResult> ConfigureAwait(ConfigureAwaitBehavior configureAwaitBehavior);
        public ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(ConfigureAwaitBehavior configureAwaitBehavior, CancellationToken cancellationToken);
    }
}

If we are in agreement, I can create a new issue containing an API proposal based on the above sketch, and we can probably close this issue and #22144.

[stephentoub]

public ConfiguredTaskAwaitable ConfigureAwait(ConfigureAwaitBehavior configureAwaitBehavior);

If we are in agreement

This should be prototyped. In particular I'm a little concerned about the extra logic this would likely add to ConfiguredTaskAwaiter's GetResult method, which should generally have its fast path inlined and be really lightweight, but with this we could unexpectedly find GetResult is no longer inlined (which would yield a relatively high overhead increase for synchronously completed operations) or that additional branches are inlined into every call site. We may want to make this overload return the ConfiguredCancelableTaskAwaitable instead; it's already going to be meatier in support of cancellation. (This doesn't need to block further progress/review, just that we need to be open to some tweaks like that, and definitely need to do perf testing along the way.)

cc: @benaadams

[eiriktsarpalis]

Good point. Sounds like we need a better name for ConfiguredCancelableTaskAwaitable?