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.

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

Developers can have access to more options when configuring async awaitables #47525

Closed
Tracked by #44314
eiriktsarpalis opened this issue Jan 27, 2021 · 75 comments · Fixed by #48842
Closed
Tracked by #44314

Developers can have access to more options when configuring async awaitables #47525

eiriktsarpalis opened this issue Jan 27, 2021 · 75 comments · Fixed by #48842
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks Bottom Up Work Not part of a theme, epic, or user story Cost:M Work that requires one engineer up to 2 weeks Priority:3 Work that is nice to have Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 27, 2021

Background and Motivation

While it is currently possible to configure task awaitables via the [Value]Task.ConfigureAwait methods, the current API only exposes one setting: continueOnCapturedContext. We have identified a number of other await settings that could be useful to users:

  • Accepting a cancellation token.
  • Awaiting on failing tasks without throwing an exception.
  • Forcing asynchronous execution of continuations.
  • Allowing for future extensions in configurability.

This proposal combines the feature requests from #27723 and #22144.

Usage examples

At the core of the proposal is introducing the following struct in System.Threading.Tasks:

public readonly struct AwaitBehavior
{
    public CancellationToken CancellationToken { get; init; } // = default;
    public TimeSpan Timeout { get; init; } // = Timeout.InfiniteTimeSpan;

    public bool ContinueOnCapturedContext { get; init; } // = true;
    public bool ForceAsync { get; init; } // = false;
    public bool SuppressExceptions { get; init; } // = false;
}

and exposing Task/ValueTask ConfigureAwait overloads that accept it like so:

var result = await task.ConfigureAwait(new AwaitBehavior { ContinueOnCapturedContext = false }); // equivalent to task.ConfigureAwait(false);

Suppressing exceptions

ValueTask<string> failedTask = ValueTask.FromException<string>(new Exception());
string result = await failedTask.ConfigureAwait(new AwaitBehavior { SuppressExceptions = true });
Assert.Equal(null, result);

Cancellation

await task.ConfigureAwait(new AwaitBehavior { CancellationToken = new CancellationToken(true) }); // throws TaskCanceledException

Timeouts

await task.ConfigureAwait(new AwaitBehavior { Timeout = TimeSpan.FromSeconds(1) }); // throws TimeoutException

Note that timeouts are declarative on awaitables; a timer will only be allocated whenever an awaiter instance is created, so we should expect the following behaviour:

var task = Task.Delay(10_000);
var awaitableWithTimeout = task.ConfigureAwait(new AwaitBehavior { Timeout = TimeSpan.FromSeconds(1) });
try { await awaitableWithTimeout ; } catch { } // throws TimeoutException
await task;
await awaitableWithTimeout; // does not throw

Finally, it should be possible to combine all the above settings:

string result = await task.ConfigureAwait(
    new AwaitBehavior 
    { 
        ContinueOnCapturedContext = false,
        SuppressExceptions = true,
        Timeout = TimeSpan.FromSeconds(5), 
        CancellationToken = new CancellationToken(true),
    });

Proposed API

namespace System.Threading.Tasks
{
    public readonly struct AwaitBehavior
    {
        public CancellationToken CancellationToken { get; init; } // = default;
        public TimeSpan Timeout { get; init; } // = Timeout.InfiniteTimeSpan;

        public bool ContinueOnCapturedContext { get; init; } // = true;
        public bool ForceAsync { get; init; } // = false;
        public bool SuppressExceptions { get; init; } // = false;
    }

    public partial class Task
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
    }

    public partial class Task<TResult>
    {
        public new System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
    }

    public partial struct ValueTask
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
    }

    public partial struct ValueTask<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult> ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
    }
}

namespace System.Runtime.CompilerServices
{
    public readonly struct ConfiguredCancelableTaskAwaitable
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable.ConfiguredCancelableTaskAwaiter GetAwaiter() { throw null; }
        public readonly struct ConfiguredCancelableTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public void GetResult() { }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly struct ConfiguredCancelableTaskAwaitable<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult>.ConfiguredCancelableTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredCancelableTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public TResult GetResult() { throw null; }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly partial struct ConfiguredCancelableValueTaskAwaitable
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable.ConfiguredCancelableValueTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredCancelableValueTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public void GetResult() { }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly partial struct ConfiguredCancelableValueTaskAwaitable<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult>.ConfiguredCancelableValueTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredCancelableValueTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public TResult GetResult() { throw null; }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }
}

Implementation

See this commit for a prototype implementation.

Open Questions

@eiriktsarpalis eiriktsarpalis added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks labels Jan 27, 2021
@eiriktsarpalis eiriktsarpalis added this to the 6.0.0 milestone Jan 27, 2021
@eiriktsarpalis eiriktsarpalis self-assigned this Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

Tagging subscribers to this area: @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

Existing (Value)Task.ConfigureAwait() methods expose just one setting: continueOnCapturedContext. We have identified a number of other awaitable settings that could be useful to users:

  • Accepting a cancellation token.
  • Awaiting on failing tasks without throwing an exception.
  • Forcing asynchronous execution of continuations.
  • Allowing for future extensions in configurability.

This proposal combines the feature requests from #27723 and #22144.

Usage examples

At the core of the proposal is introducing the following enum in System.Threading.Tasks:

[Flags]
public enum AwaitBehavior
{
    Default = 0x0,
    NoCapturedContext = 0x1,
    NoThrow = 0x2,
    ForceAsync = 0x4,
}

and exposing Task/ValueTask ConfigureAwait overloads that accept behavior flags like so:

var result = await task.ConfigureAwait(AwaitBehavior.NoCapturedContext); // equivalent to task.ConfigureAwait(false);

The NoThrow flag can be used as follows:

string result = await ValueTask.FromException<string>(new Exception()).ConfigureAwait(AwaitBehavior.NoThrow);
Assert.Equal(null, result);

Passing cancellation tokens should work like so:

await Task.Delay(10_000).ConfigureAwait(new CancellationToken(true)); // throws OperationCanceledException

Finally, it should be possible to combine all settings:

string result = await task.ConfigureAwait(AwaitBehavior.NoCapturedContext | AwaitBehavior.ForceAsync, cancellationToken);

Proposed API

namespace System.Threading.Tasks
{
    [Flags]
    public enum AwaitBehavior
    {
        Default = 0x0,
        NoCapturedContext = 0x1,
        NoThrow = 0x2,
        ForceAsync = 0x4,
    }

    public partial class Task
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable ConfigureAwait(System.Threading.CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable ConfigureAwait(bool continueOnCapturedContext, System.Threading.CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior, System.Threading.CancellationToken cancellationToken) { throw null; }
    }

    public partial class Task<TResult>
    {
        public new System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(System.Threading.CancellationToken cancellationToken) { throw null; }
        public new System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(bool continueOnCapturedContext, System.Threading.CancellationToken cancellationToken) { throw null; }
        public new System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(ConfigureAwaitBehavior awaitBehavior) { throw null; }
        public new System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult> ConfigureAwait(ConfigureAwaitBehavior awaitBehavior, System.Threading.CancellationToken cancellationToken) { throw null; }
    }

    public partial struct ValueTask
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable ConfigureAwait(CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable ConfigureAwait(bool continueOnCapturedContext, CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable ConfigureAwait(AwaitBehavior awaitBehavior, CancellationToken cancellationToken) { throw null; }
    }

    public partial struct ValueTask<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult> ConfigureAwait(CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult> ConfigureAwait(bool continueOnCapturedContext, CancellationToken cancellationToken) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult> ConfigureAwait(AwaitBehavior awaitBehavior) { throw null; }
        public System.Runtime.CompilerServices.ConfiguredCancelableValueTaskAwaitable<TResult> ConfigureAwait(AwaitBehavior awaitBehavior, CancellationToken cancellationToken) { throw null; }
    }
}

namespace System.Runtime.CompilerServices
{
    public readonly struct ConfiguredCancelableTaskAwaitable
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable.ConfiguredCancelableTaskAwaiter GetAwaiter() { throw null; }
        public readonly struct ConfiguredCancelableTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public void GetResult() { }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly struct ConfiguredCancelableTaskAwaitable<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredCancelableTaskAwaitable<TResult>.ConfiguredCancelableTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredCancelableTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public TResult GetResult() { throw null; }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly partial struct ConfiguredValueTaskAwaitable
    {
        public System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredValueTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public void GetResult() { }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }

    public readonly partial struct ConfiguredValueTaskAwaitable<TResult>
    {
        public System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable<TResult>.ConfiguredValueTaskAwaiter GetAwaiter() { throw null; }
        public readonly partial struct ConfiguredValueTaskAwaiter : System.Runtime.CompilerServices.ICriticalNotifyCompletion, System.Runtime.CompilerServices.INotifyCompletion
        {
            public bool IsCompleted { get { throw null; } }
            public TResult GetResult() { throw null; }
            public void OnCompleted(System.Action continuation) { }
            public void UnsafeOnCompleted(System.Action continuation) { }
        }
    }
}

Implementation

See this commit for a reference implementation.

Open Questions

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

api-suggestion, area-System.Threading.Tasks

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 27, 2021
@eiriktsarpalis eiriktsarpalis added Bottom Up Work Not part of a theme, epic, or user story Priority:3 Work that is nice to have Team:Libraries User Story A single user-facing feature. Can be grouped under an epic. Cost:M Work that requires one engineer up to 2 weeks and removed untriaged New issue has not been triaged by the area owner labels Jan 27, 2021
@huoyaoyuan
Copy link
Member

huoyaoyuan commented Jan 27, 2021

Can there be extension methods for shortcuts of common await options?
Currently await .ConfigureAwait(false) is already significant amount of code even if the task is encapsulated. It can be longer since the enum values are longer than true and false.

Or, do some exploration with dotnet/csharplang#2649 .

@stephentoub
Copy link
Member

Can there be extension methods for shortcuts of common await options?

I would hope the majority of awaits would not use this support. It's important here and there, but if the majority of awaits need these options, I suspect there are bigger issues to be addressed.

@andriysavin
Copy link

I'd be happy to have a shortcut for the ConfigureAwait(false) and ConfigureAwait(AwaitBehavior.NoCapturedContext), as these are used massively in library code. Especially having readability of the former is poor, and the latter is too lengthy (again, for massive usage). While users can easily implement such extension methods themselves, it won't work with the code analyzer. Alternatively, it would be super helpful to have way to globally configure capturing context on awaiting while this being transparent for client code (no task factories etc.). E.g. if I could override GetAwaiter of Task via custom extension method.

@stephentoub
Copy link
Member

Alternatively, it would be super helpful to have way to globally configure capturing context on awaiting while this being transparent for client code (no task factories etc.)

There are many discussions/issues about that over in dotnet/csharplang.

@stephentoub
Copy link
Member

@eiriktsarpalis, I don't see timeouts represented here. Weren't you going to have some overloads that took a TimeSpan? My concern with relying on CancellationToken in this context is we're likely to see devs accidentally write code like:

await task.ConfigureAwait(new CancellationTokenSource(timeout).Token);

and while an analyzer could be written to flag that and suggest it instead be:

using (var cts = new CancellationTokenSource(timeout))
    await task.ConfigureAwait(cts.Token);

it's easier to get it right with the timeout overload. The above also makes the already-completed task case much more expensive. I'd be ok if we just wanted to have one overload per-task-like type that takes a timeout in addition to options and a token.

Or is there a reason not to do this?

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Feb 1, 2021

Or is there a reason not to do this?

No real reason. Should we use TimeSpan, int for milliseconds or perhaps both?

@stephentoub
Copy link
Member

I'd stick with just TimeSpan in the proposal, and call it out as an open issue to discuss in API review in case anyone has a different opinion.

@stephentoub
Copy link
Member

Also, did you consider alternatives for exposing this? e.g. rather than having a bunch of overloads, you could have one with defaults (though I don't know what we'd do for a TimeSpan default given the type), or potentially a struct with the options:

public struct AwaitBehavior
{
    public bool ContinueOnCapturedContext { get; set; } // defaults to true
    public bool SuppressExceptions { get; set; } // defaults to false
    public bool ForceAsynchronous { get; set; } // defaults to false
    public CancellationToken CancellationToken { get; set; } // defaults to default
    public TimeSpan Timeout { get; set; } // defaults to -1
}

etc. Then you'd just have one new overload per task type ConfigureAwait(AwaitBehavior).

Just throwing out other approaches you might consider.

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Feb 1, 2021

@stephentoub seems like a reasonable alternative, should we consider making this a class to accommodate potential future additions?

@eiriktsarpalis
Copy link
Member Author

Here's an example of how timeouts could be implemented over the existing proposal. Each new awaiter must allocate a CancellationTokenSource instance with delay set to the specified timeout. We'd need to find a good way to dispose of it eventually, and probably the best candidate for achieving that is via the GetResult() method. Note that this carries the added risk of leaking CancellationTokenSource instances in scenaria where we are creating awaiters that are not being awaited.

I'm not particularly fond of this approach, and it suggests to me that perhaps awaiters are not the appropriate layer of abstraction for introducing timeouts. Alternatively we could expose this feature as task extension, building on top of cancellable awaiters:

public static async ValueTask<TResult> WithTimeout<TResult>(this Task<TResult> task, TimeSpan timeout)
{ 
    if (timeout == Timeout.InfiniteTimeSpan)
    {
        return await task.ConfigureAwait(false);
    }
 
    using var cts = new CancellationTokenSource(timeout);
    try
    {
        return await task.ConfigureAwait(false, cts.Token);
    }
    catch (OperationCanceledException e)
    {
        throw TimeoutException("blah", e);
    }
}

@eiriktsarpalis
Copy link
Member Author

it suggests to me that perhaps awaiters are not the appropriate layer of abstraction for introducing timeouts.

This is pertinent to @stephentoub's proposal in #47525 (comment), in that it would be expected to support scenaria where both a cancellation token and timeout are specified, which would make the implementation more complicated than needed.

@stephentoub
Copy link
Member

building on top of cancellable awaiters

If we decided to go with a WithTimeout, I don't think we'd build it this way. It'd be better in that case to build it on top of Timer (or TimerQueueTimer) directly, rather than the layer of CancellationTokenSource in the middle, throwing an extra layer of exception, etc.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 26, 2021
@stephentoub
Copy link
Member

stephentoub commented Feb 26, 2021

But the ConfigureAwait path is probably significantly more efficient when implemented using the originally approved API that had awaiter structs under System.Runtime.CompilerServices?

Not really. You still need some shared object that can coordinate between the Task, the CancellationToken, and the Timer (e.g. something you can pass to all of their callbacks), that can hold onto the Task to be able to unregister from it, the CancellationTokenRegistraiton to be able to dispose of it, and the Timer to be able to dispose of it, etc. That might as well be in a Task object itself, at which point you can reuse the exact same optimized awaiter and not increase the size of every state machine that awaiter gets baked into (multiplied by every unique awaiter type used in the method).

@davidfowl
Copy link
Member

Working on some unrelated code I remembered what the biggest use of NoThrow is for me, non-throwing cancellation. Cancellation is used to stop existing async operations from executing and many times the caller would like a non-throwing way to stop things (Cancellation is a use of exceptions for control flow). In that case it would be super useful to have a built in way to ignore that cancellation when purely waiting for the task to end.

@noseratio
Copy link

noseratio commented Feb 28, 2021

(Cancellation is a use of exceptions for control flow). In that case it would be super useful to have a built in way to ignore that cancellation when purely waiting for the task to end.

I use a helper for that in every single project I've worked on, frontend or backend. It's not quite trivial to get done right, so it'd be great to have it built-in.

E.g., there could be an AggregatedException error for Task.WhenAll where (say) the first exception is OperationCanceledException, but the rest aren't. In this case the default error unwrapping behavior of await is undesirable.

Or vice versa, I've seen cases where task.IsCanceled is false and task.IsFaulted is true, while the result of task.Exception.Flatten() are all of OperationCanceledException. For some reason, the IsCanceled status didn't get propagated correctly there.

I think, a proper helper for observing cancellations should be taking care of the corner cases like these. Here's what I use, it returns Task<Task<T>> , where the inner task is successfully completed or canceled:

// this will throw, unless the returned task is either `IsCompletedSuccessfully` or `IsCanceled`
var task = await TestAsync(token).IgnoreCancellations();
if (task.IsCanceled) return;
Console.WriteLine(task.Result);

// ...

public static Task<Task<T>> IgnoreCancellations<T>(this Task<T> @this)
{
    return @this.ContinueWith<Task<T>>(
        continuationFunction: anteTask =>
        {
            if (anteTask.IsFaulted)
            {
                if (anteTask.Exception.Flatten().InnerExceptions.All(e => e is OperationCanceledException))
                {
                    // all cancelled, return a cancelled task
                    return Task<T>.FromCanceled<T>(new CancellationToken(canceled: true));
                }

                // rethrow
                anteTask.GetAwaiter().GetResult();
            }

            return anteTask;
        },
        cancellationToken: CancellationToken.None,
        TaskContinuationOptions.ExecuteSynchronously,
        scheduler: TaskScheduler.Default);
}

@davidnmbond
Copy link

davidnmbond commented Mar 1, 2021

For syntactic sugar, can we please have await AND awaitf keywords (or similar)?

That way, all my code that is like:

var x = await thing.CallAsync(cancellationToken).ConfigureAwait(false);

...can just be:

var x = awaitf thing.CallAsync(cancellationToken);

...or even...

var x = awaif thing.CallAsync(cancellationToken); // f is an unside down t

@davidfowl
Copy link
Member

Looks like syntax salt 😆

@noseratio
Copy link

The idea of syntax sugar is interesting though, given how pervasive ConfigureAwait is. Although it probably should be pitched to the C# compiler team 🙂

@AshleighAdams
Copy link

@davidnmbond Nah, ConfigureAwait is a Task library construct, and await is a language construct.

The best proposal to this I've seen is to allow extending an await operator, so you can provide a ConfiguredTask await(this Task t). I can't remember the exact syntax now, but it was far more flexible and means the compiler doesn't have to have edge cases with a Task.

@SittenSpynne
Copy link

I see some features in proposed APIs I don't get, I'm not going to argue against them. But what I'd like to see is methods that do what they say. I want call sites that look like:

await DoSomethingAsync().WithCapturedContext();
await DoSomethingAsync().WithContext(customContext);
await DoSomethingAsync().WithoutContext();

These methods say what they do. "Await this thing with a captured context", not "await this thing but configure the context false". It helps people read the code like they think.

I've seen an awful lot of newbies completely flummoxed by ConfigureAwait() and why they need it. I was one of them when the TAP whitepaper first came out. I think we should retire the name. It's hard enough to explain why and when they care about capturing the context, it's harder to explain what ConfigureAwait(false) means if they don't have R# or Rider to tell them the parameter is continueOnCapturedContext. There's a lot that could be done to educate people, but it'd be a nice step to choose a name for the method related to what it's doing.

@noseratio
Copy link

noseratio commented Mar 1, 2021

These methods say what they do. "Await this thing with a captured context", not "await this thing but configure the context false". It helps people read the code like they think.

I really like this proposal, and I can heartily relate to the reasons @SittenSpynne provides behind it.

I'd also add ForceAsync, IgnoreErrors, and IngnoreCancellations (all discussed previously in this thread), and make them easily combinable in an fluent way:

await DoSomethingAsync().WithoutContext()
  .ForceAsync()
  .IngnoreCancellations();

I think this concept would play well with another proposed API, await TaskScheduler.Default.SwitchTo().

Such a set of new, custom-awaiter-based APIs wouldn't be a breaking change, and wouldn't make ConfigureAwait obsolete. The new code and the new developers coming to .NET platform would have an option to start using it. Similar to adoption of ValueTask, which has been a very welcoming and useful addition.

@FiniteReality
Copy link

await DoSomethingAsync().WithoutContext()
  .ForceAsync()
  .IngnoreCancellations();

This would mean a huge explosion of APIs (the awaiter returned by ForceAsync() needs WithoutContext() and IgnoreCancellations(), as well as the awaiter returned by WithoutContext(), as well as the awaiter returned by IgnoreCancellations(), and so on so forth for each API), which is what the original API proposal (ConfigureAwait(AwaitBehaviour)) is supposed to prevent.

@noseratio
Copy link

This would mean a huge explosion of APIs

I don't see why they cannot be composable without provisioning for all possible combinations?

Each API could create an awaiter which is responsible for one thing (e.g, make it async), and that can be composed with another awaiter (e.g., ignore errors), in any particular order.

@StephenCleary
Copy link
Contributor

in any particular order.

I think this is the key point here. Can they really be combined in any order? If so, can they be implemented as extension methods today, on ConfiguredTaskAwaitable?

@noseratio
Copy link

noseratio commented Mar 2, 2021

I think this is the key point here. Can they really be combined in any order? If so, can they be implemented as extension methods today, on ConfiguredTaskAwaitable?

I need to do some experimenting, but I believe that's quite achievable. It should be just about implementing the awaiter protocol methods correctly.

The problems with making them extensions of ConfiguredTaskAwaitable is that we won't be able to alter the continuation behavior of ConfiguredTaskAwaiter or TaskAwaiter. It's not only about how it captures the synchronization context, but also about how it decides when to continue synchronously or asynchronously. Which sometimes can be contra-intuitive in my opinion, I raised this concern here.

I think I may have a clear picture of how to implement extensions like RestoreContext, WithoutContext, ForceAsync using Task.ContinueWith, but not using TaskAwaiter/ConfiguredTaskAwaiter. And I don't think it's possible to implement them as extensions of ValueTask, without making changes to .NET internal code (I hope I'm wrong here).

@FiniteReality
Copy link

This would mean a huge explosion of APIs

I don't see why they cannot be composable without provisioning for all possible combinations?

Each API could create an awaiter which is responsible for one thing (e.g, make it async), and that can be composed with another awaiter (e.g., ignore errors), in any particular order.

If you want to make them composable without defining, e.g. a ContinueOnCapturedContextWithoutCatchingExceptionsAwaitable and a ContinueWithoutCatchingExceptionsOnCapturedContextAwaitable, they need to be implemented as classes, or boxed to the heap, which means a lot of allocations at each configured await.

@timcassell
Copy link

If you want to make them composable without defining, e.g. a ContinueOnCapturedContextWithoutCatchingExceptionsAwaitable and a ContinueWithoutCatchingExceptionsOnCapturedContextAwaitable, they need to be implemented as classes, or boxed to the heap, which means a lot of allocations at each configured await.

That's not necessary if the struct contains all the fields for each method, then you just return a copied struct with that field changed.

@FiniteReality
Copy link

That's not necessary if the struct contains all the fields for each method, then you just return a copied struct with that field changed.

But then you're not really satisfying the "responsible for one thing" point you seem to be so keen on. Also, at that point, you could just have a singular API which creates the singular struct with all the fields set anyway, which brings us back to the original API proposal.

@timcassell
Copy link

But then you're not really satisfying the "responsible for one thing" point you seem to be so keen on. Also, at that point, you could just have a singular API which creates the singular struct with all the fields set anyway, which brings us back to the original API proposal.

I think you have me confused with someone else, but yeah, a single API with optional parameters makes sense to me.

@noseratio
Copy link

noseratio commented Mar 2, 2021

But then you're not really satisfying the "responsible for one thing" point you seem to be so keen on.

That was me. Not saying it's 100% possible, but let's see if I can put some PoC code together (edited: here).
I don't think extra allocations is a big issue, even if I come up with an extra Task object per each of those fluent calls. I have lots of trust in .NET GC :) For performance-critical code, there still will be existing ConfigureAwait and/or ValueTasks.

@noseratio
Copy link

noseratio commented Mar 2, 2021

@FiniteReality here's a gist with a quick proof of concept of what I meant above, where each awaiter is responsible for one specific thing:

await Task.Delay(5000).RestoreContext().ForceAsync();

Disclaimer: it's almost untested and not optimized to reduce allocations.

Other ideas for chain-able custom awaiters like that:

  • Then(task => log("Succeeded")) - in before
  • Catch(ex => log(ex))
  • Finally(task => log("Completed"))
  • IgnoreCancellations
  • ResumeOnAnyContext
  • ResumeOnContext(synchronizationContext)
  • ResumeOnThreadPool
  • ResumeOnTaskScheduler(taskScheduler)
  • FireAndForget
  • WaitAsync(cancellationToken)
  • ...

Thinking about extra allocations and the GC burden, this doesn't seem to be a big issue for JavaScript promises in Node.js, the ecosystem that also runs some quite high-performance sites and microservices out there. Here's a good read: https://v8.dev/blog/fast-async

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 9, 2021
@terrajobst
Copy link
Member

terrajobst commented Mar 9, 2021

Video

  • This API looks cleaner and seems easier to evolve. We don't believe this adds significant overhead of the ConfigureAwait approach. In terms of discoverability it will do about the same, because it requires to invoke a method before awaiting the result.
  • The existing Wait API has overloads that take int milliseconds but we consider this a mistake, so we don't replicate them here and just use TimeSpan.
  • @stephentoub please reopen the issue that allows suppression throwing exceptions because we cut this from this issue.
  • Let's cut the Value and ValueTask ones as there is the additional complexity of accidentally touching it twice and they are really just convenience methods that can be mitigated by calling AsTask().WaitAsync(...)
namespace System.Threading.Tasks
{
    public partial class Task
    {
        public Task WaitAsync(CancellationToken cancellationToken);
        public Task WaitAsync(TimeSpan timeout);
        public Task WaitAsync(TimeSpan timeout, CancellationToken cancellationToken);
    }

    public partial class Task<TResult>
    {
        public Task<TResult> WaitAsync(CancellationToken cancellationToken);
        public Task<TResult> WaitAsync(TimeSpan timeout);
        public Task<TResult> WaitAsync(TimeSpan timeout, CancellationToken cancellationToken);
    }
}

@terrajobst terrajobst removed the blocking Marks issues that we want to fast track in order to unblock other important work label Mar 9, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 11, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks Bottom Up Work Not part of a theme, epic, or user story Cost:M Work that requires one engineer up to 2 weeks Priority:3 Work that is nice to have Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

Successfully merging a pull request may close this issue.