-
Notifications
You must be signed in to change notification settings - Fork 1k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
[Proposal]: Cancel Async Functions Without Throw #4565
Comments
What if the caller forget to call |
That would just compile to 'await (Task)t', as compared to from the OP: 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 child cancels with the flag, but the handling of this would have to be entirely on the parent. |
Exactly right @AartBluestoke. If the awaiter doesn't implement |
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:
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 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 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! await token.CancelIfRequested(); // Instead of CancelAsyncIfCancellationRequested |
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.
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
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).
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.
The existing async method builders should work exactly as they do currently, simply adding a
See the previous dicussion for the answer to that. TLDR: yes it's possible, but not as desirable.
I don't disagree about the verbosity, but |
I think a new keyword is warranted for something whose purpose is to stop executing the method in the middle. |
await token.ReturnCancled()? |
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! |
I prefer |
We have the method:
Why not add an override to this with an:
Then you could add options like existing continue on captured context and maybe a return value for cancelation:
|
@andreas-synnerdahl that would be a request for dotnet/runtime. Thanks! |
@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 [Edit] Also, this should be supported for all custom async task-like types, not just Task and ValueTask. |
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 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 |
Related to dotnet/roslyn#65863 |
Perhaps [Edit] Although, maybe not if dotnet/roslyn#65863 will be added with |
Consider
|
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
[Edit] Actually, there is a behavior change of losing the async stacktrace... Perhaps a more sensical keyword could be introduced that conveys the intention. |
I think a nice pattern would be
Perhaps you could write |
@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. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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 anIsCanceled
property 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, whereawaiter
is the awaiter whoseIsCanceled
property 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 awaiter
is for custom task-like types to be able to accept custom cancelations.Example state-machine outputs:
CancellationToken and Task extensions:
Simple Example:
Equivalent C#:
Complex Example:
Equivalent C#:
Drawbacks
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
GetException
method 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
The text was updated successfully, but these errors were encountered: