huoyaoyuan
commented
3 years ago
await OtherFuncAsync().CancelAsyncIfCanceled()
What if the caller forget to call CancelAsyncIfCanceled?
Open timcassell opened 3 years ago
huoyaoyuan
commented
3 years ago
await OtherFuncAsync().CancelAsyncIfCanceled()
What if the caller forget to call CancelAsyncIfCanceled?
AartBluestoke
commented
3 years ago
await OtherFuncAsync().CancelAsyncIfCanceled()What if the caller forget to call
CancelAsyncIfCanceled?
That would just compile to 'await (Task)t', as compared to await OtherFuncAsync().CancelAsyncIfCanceled() which would compile to 'await (CancelableTask)t'
from the OP: "Both the awaiter and the builder must have those methods in order for the compiler to emit the fast-tracked cancelation instructions"
The new async state machine would still have to set the Exception object to the relevant throwable, along with setting 'isCanceled' to fast-path if the parent supported it.
I would imagine that every step which doesn't have that check would be one the old behaviour of returning by thrown exception at each layer. The new code has to return a Task capable of being sensibly handled by the old state machine, as you can't guarantee the parent has been recompiled since the introduction of this feature.
The child cancels with the flag, but the handling of this would have to be entirely on the parent.
timcassell
commented
3 years ago Exactly right @AartBluestoke. If the awaiter doesn't implement IsCanceled, then its GetResult should throw an OperationCanceledException, and if the builder doesn't implement SetCanceled, the awaiter's GetResult should still throw an OperationCanceledException. Which is why in my extension example I had public void GetResult() => _token.ThrowIfCancellationRequested();.
bjbr-dev
commented
3 years ago I really like the idea of this, just curious what would happen to the stack trace? Is it something you should care about for a cancelled event? How can you distinguish between a cancellation due to a timeout (over the network) or a "user requested" cancellation due to clicking a button? I guess its important to know whether your caller cancelled or whether something you called cancelled.
I supposed you could do something like this:
var task = OtherFuncAsync().CancelAsyncIfCanceled(cancellationToken);
await task;
if (cancellationToken. IsCancellationRequested) {
... gracefully handle cancellation by caller
}
else if (task.IsCancelled) {
... gracefully handle cancellation by dependency
}
else {
... we werent cancelled!
}but that seems pretty verbose. Admittedly it's much more common for code to just be "pass through" and not care why it was cancelled, but this sort of code is needed at the "top level" and therefore worth keeping terse.
I wonder if the task should return a tuple, in order to stop us having to allocate the Task as a local variable and await it seperately:
var (task, isCancelled) = await OtherFuncAsync().CancelAsyncIfCanceled(cancellationToken);
if (isCancelled) {
} else {
}Can this style of cancellation be mixed with OperationCanceledException?
I.e. User method, A, supports IsCancelled, and call method B
Library method, B, does not support IsCancelled, and calls method C
Library method C, supports IsCancelled and calls SetCancelled
Because B does not support it, it throws the exception. Would method A bubble the exception or could it "catch it" and turn it into an IsCancelled task?
If the async machine doesnt catch it, would a developer be expected to catch OperationCancelledException AND check for the IsCancelled property to deal with legacy / non legacy?
I'm wondering if it would be more user friendly to somehow invoke the catch clause with an OperationCancelledException but not actually throw it. Would that be possible / still fast?
Finally, I appreciate that naming things is hard but the names seem extremely verbose if we're supposed to use it everywhere! How about
await token.CancelIfRequested(); // Instead of CancelAsyncIfCancellationRequestedI really like the idea of this, just curious what would happen to the stack trace? Is it something you should care about for a cancelled event?
Typically you wouldn't care about the stack trace for cancelations, and if you do care about it, you can just opt not to use this feature.
How can you distinguish between a cancellation due to a timeout (over the network) or a "user requested" cancellation due to clicking a button? I guess its important to know whether your caller cancelled or whether something you called cancelled.
That's already a concern with cancellations today. That needs to be handled at an individual level by the programmer.
[Edit] Also, it's bad practice to cancel an operation without the caller requesting it. The operation in that case should throw a TimeoutException rather than an OperationCancelledException.
I supposed you could do something like this:
var task = OtherFuncAsync().CancelAsyncIfCanceled(cancellationToken); await task; if (cancellationToken. IsCancellationRequested) { ... gracefully handle cancellation by caller } else if (task.IsCancelled) { ... gracefully handle cancellation by dependency } else { ... we werent cancelled! }but that seems pretty verbose. Admittedly it's much more common for code to just be "pass through" and not care why it was cancelled, but this sort of code is needed at the "top level" and therefore worth keeping terse.
I wonder if the task should return a tuple, in order to stop us having to allocate the Task as a local variable and await it seperately:
var (task, isCancelled) = await OtherFuncAsync().CancelAsyncIfCanceled(cancellationToken); if (isCancelled) { } else { }
You're getting into territory that is outside the scope of this feature request. Suppressing exceptions and returning a tuple like that is already possible today with custom awaiters (see how UniTask does exactly that).
Can this style of cancellation be mixed with OperationCanceledException?
I.e. User method, A, supports
IsCancelled, and call method B Library method, B, does not supportIsCancelled, and calls method C Library method C, supports IsCancelled and callsSetCancelledBecause B does not support it, it throws the exception. Would method A bubble the exception or could it "catch it" and turn it into an
IsCancelledtask?
Yes. One method would throw, then the next would take the fast path or vice-versa. The only difference is stack traces would be lost.
If the async machine doesnt catch it, would a developer be expected to catch OperationCancelledException AND check for the IsCancelled property to deal with legacy / non legacy?
The existing async method builders should work exactly as they do currently, simply adding a SetCancelled as an extra optimization. And because awaiters can optionally support IsCancelled, yes, builders should be able to handle both. So they can have SetCancelled and check for OperationCancelledException inside SetException as they do currently.
I'm wondering if it would be more user friendly to somehow invoke the catch clause with an OperationCancelledException but not actually throw it. Would that be possible / still fast?
See the previous dicussion for the answer to that. TLDR: yes it's possible, but not as desirable.
Finally, I appreciate that naming things is hard but the names seem extremely verbose if we're supposed to use it everywhere! How about
await token.CancelIfRequested(); // Instead of CancelAsyncIfCancellationRequested
I don't disagree about the verbosity, but CancelAsyncIfCancellationRequested was meant to mirror ThrowIfCancellationRequested and CancelAsync was to give an extremely clear intent of canceling the async method. It's propagating the cancelation up, not triggering a cancelation. CancelIfRequested is not clear in its intent to me.
erik-kallen
commented
3 years ago I think a new keyword is warranted for something whose purpose is to stop executing the method in the middle.
AartBluestoke
commented
3 years ago await token.ReturnCancled()?
ronnygunawan
commented
3 years ago I think a new keyword is warranted for something whose purpose is to stop executing the method in the middle.
public class M
{
async Task<int> FuncAsync(CancellationToken token)
{
await Task.Yield();
if (token.IsCancellationRequested) cancel;
return 42;
}
}@AartBluestoke That does not look like something that is expected to stop executing the method and return normally.
@ronnygunawan Yes, something like that!
ronnygunawan
commented
3 years ago I prefer Task.CurrentTask.Cancel() or Task.Cancel()
andreas-synnerdahl
commented
2 years ago We have the method: ConfigureAwait(Boolean) for
TaskTask<TResult>ValueTaskValueTask<TResult>Why not add an override to this with an:
ConfigureAwait(Action<IAwaitConfigurationBuilder> configureDelegate)ConfigureAwait(Action<IAwaitConfigurationBuilder<TResult>> configureDelegate)Then you could add options like existing continue on captured context and maybe a return value for cancelation:
public async Task<string> DoCurlAsync()
{
using (var httpClient = new HttpClient())
using (var httpResponse = await httpClient.GetAsync("https://www.bynder.com").ConfigureAwait(false))
{
return await httpResponse
.Content
.ReadAsStringAsync()
.ConfigureAwait(o => o
.ContinueOnCapturedContext(true)
.CancelationReturnValue(string.Empty));
}
}
CyrusNajmabadi
commented
2 years ago @andreas-synnerdahl that would be a request for dotnet/runtime. Thanks!
timcassell
commented
2 years ago @andreas-synnerdahl I thought of ways to resolve this in the async method builder, but it's not possible to do that and also support finally clauses (which is what using translates to), because the C# compiler creates the state machine that handles the finally clauses. This feature requires language compiler support to adjust the compiled state machine.
[Edit] Also, this should be supported for all custom async task-like types, not just Task and ValueTask.
timcassell
commented
2 years ago I was just wondering, should direct cancelations have the same or lower precedence as exceptions?
Consider:
async Task<int> FuncAsync()
{
try
{
throw new InvalidOperationException();
}
finally
{
await new CancellationToken(true).CancelAsyncIfCancellationRequested();
}
}Would the InvalidOperationException be discarded and the task canceled as if an OperationCanceledException were thrown? Or would the cancelation be ignored and the task faulted?
I think it's fairly obvious that the other way around is an easy choice, the exception would discard the cancelation.
After writing this out and thinking about it some more, I think that cancelations and exceptions should have the same precedence for consistency's sake. If the builder doesn't implement the SetCanceled method, thus causing the awaiter to throw, that would discard the InvalidOperationException and cancel the task. So the direct cancelation should retain the same behavior.
sharwell
commented
1 year ago Related to dotnet/roslyn#65863
timcassell
commented
1 year ago Perhaps TResult GetResult(out bool isCanceled); void GetResult(out bool isCanceled); overload methods could be used, rather than bool IsCanceled property.
[Edit] Although, maybe not if https://github.com/dotnet/roslyn/issues/65863 will be added with TResult GetResult(out Exception? exception);, in which case we'd still need to use the property to not clash (unless we also use TResult GetResult(out Exception? exception, out bool isCanceled);, but that's probably overcomplicating it).
RenderMichael
commented
1 year ago I think a new keyword is warranted for something whose purpose is to stop executing the method in the middle.
Consider await break.
await makes it clear this is related to the async-ness of the method.break is conceptually similar in loops and switch statements.timcassell
commented
1 year ago I think a new keyword is warranted for something whose purpose is to stop executing the method in the middle.
Consider
await break.
- It uses existing keywords.
awaitmakes it clear this is related to the async-ness of the method.breakis conceptually similar in loops and switch statements.- Cancelling a task like this is not to be taken lightly. Making this feature verbose with a two-part keyword makes that clearer.
At first I thought I liked that, but then I realized it does not convey its intention. The idea is to propagate cancelation efficiently, not trigger cancelation. When I see break, I don't think "stop execution if...", I just think "stop execution".
I proposed new keywords in the original discussion, and they were rightfully shot down. Considering this is purely an optimization, and the behavior is expected to be the same as throw new OperationCanceledException(), the compiler could opt to only generate this code if it's not inside a try-catch block. New keywords should not be necessary just for an optimization.
[Edit] Actually, there is a behavior change of losing the async stacktrace... Perhaps a more sensical keyword could be introduced that conveys the intention. awaitcancelable or something like that (I don't really have any good ideas here).
RenderMichael
commented
1 year ago I think a nice pattern would be await break when (token.IsCancellationRequested);. I know it's a bit verbose, but there are a bunch of upsides
await break today doesn't compile.when is used here similarly to it's other use next to a catch block, taking a boolean as an "argument" and affirming the execution of the catch.
awaitmakes it clear this is related to the async-ness of the method.breakis conceptually similar in loops and switch statements.
Perhaps you could write await break; by itself for unconditional cancellation.
timcassell
commented
1 year ago @RenderMichael Sure, that works to trigger cancelation. But that doesn't do much to propagate cancelation, which is the main point of this proposal. That would be extremely verbose to await a task with throws disabled just to check its canceled state and break. And that won't even work with ValueTasks and other awaitables that can't have their state checked after they are awaited.
Cancel Async Functions Without Throw
Summary
Cancelation of async functions is currently only possible by throwing an
OperationCanceledException. This proposal allows canceling async functions directly via awaiters implementing anIsCanceledproperty and AsyncMethodBuilders implementing avoid SetCanceled<TAwaiter>(ref TAwaiter)method. Finally blocks are allowed to run after an awaiter is canceled, just like if an exception were thrown.Previous Discussion
Motivation
Performance! Benchmarks show up to 1000x performance improvement when canceling directly instead of throwing an exception. Source
Detailed design
Any awaiter can implement the
bool IsCanceled { get; }property to enable fast-tracked async cancelations. The compiler could use duck-typing to check for the property (like it does withGetResult), or a new interface (like it does withI(Critical)NotifyCompletion).AsyncMethodBuilders will need to add a new method
void SetCanceled<TAwaiter>(ref TAwaiter awaiter)to enable fast-tracked cancelations for the async return type, whereawaiteris the awaiter whoseIsCanceledproperty returnedtrue. Both the awaiter and the builder must have those methods in order for the compiler to emit the fast-tracked cancelation instructions, otherwise it falls back to the existing implementation.Tasks won't be using the
ref TAwaiter awaiter, it will just set the task to canceled. The purpose of theref TAwaiter awaiteris for custom task-like types to be able to accept custom cancelations.Example state-machine outputs:
CancellationToken and Task extensions:
```cs public static class CancellationTokenExtensions { public readonly struct CancelAsyncAwaitable : INotifyCompletion { private readonly CancellationToken _token; public CancelAsyncAwaitable(CancellationToken token) => _token = token; public CancelAsyncAwaitable GetAwaiter() => this; public bool IsCompleted => true; public bool IsCanceled => _token.IsCancellationRequested; public void GetResult() => _token.ThrowIfCancellationRequested(); void INotifyCompletion.OnCompleted(Action continuation) => throw new NotImplementedException(); } public static CancelAsyncAwaitable CancelAsyncIfCancellationRequested(this CancellationToken token) { return new CancelAsyncAwaitable(token); } } public static class TaskExtensions { public readonly struct CancelAsyncAwaitable : ICriticalNotifyCompletion { private readonly Task _task; private readonly TaskAwaiter _awaiter; public CancelAsyncAwaitable(Task task) { _task = task; _awaiter = task.GetAwaiter(); } public CancelAsyncAwaitable GetAwaiter() => this; public bool IsCompleted => _awaiter.IsCompleted; public bool IsCanceled => _task.Status == TaskStatus.Canceled; public void GetResult() => _awaiter.GetResult(); public void OnCompleted(Action continuation) => _awaiter.OnCompleted(continuation); public void UnsafeOnCompleted(Action continuation) => _awaiter.UnsafeOnCompleted(continuation); } public static CancelAsyncAwaitable CancelAsyncIfCanceled(this Task task) { return new CancelAsyncAwaitable(task); } } ```Simple Example:
Equivalent C#:
```cs public class M { [StructLayout(LayoutKind.Auto)] [CompilerGenerated] private struct _d__0 : IAsyncStateMachine { public int _1__state; public AsyncTaskMethodBuilderComplex Example:
Equivalent C#:
```cs public class M { [StructLayout(LayoutKind.Auto)] [CompilerGenerated] private struct _d__0 : IAsyncStateMachine { public int _1__state; public AsyncTaskMethodBuilderDrawbacks
Unlike exceptions, direct cancelations cannot be caught. Therefore, existing types should use the existing functionality by default, and expose new APIs for performance.
More work for the compiler.
Alternatives
New keywords - undesirable and unlikely to get implemented for such a niche feature.
Awaiters have a
GetExceptionmethod and the state-machine checks the return value for null - Still requires allocating an exception object and brings up the question of how the async stack traces are preserved.Unresolved questions
Should a new keyword be added to catch these direct cancelations? (I think no)
Design meetings