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

Simplified task cancel #2163

Closed
Thaina opened this issue Jan 22, 2019 · 12 comments

Comments

Projects
None yet
5 participants
@Thaina
Copy link

commented Jan 22, 2019

I think the current flow to create async/await task that could be cancelled is hard and involve the confusing CancellationToken or TaskCompletionSource just to be cancel the task to make sure it will be discontinued

I think it should be as easy as throw; expression

async Task<string> DoSomething()
{
    var text = await GetAsync();
    if(text.StartsWith("error"))
        throw new Exception("error"); // normal throw for exception

    if(text == "cancel")
        throw; // empty throw to cancel; same behaviour as TaskCompletionSource.TrySetCancel;

    return text;
}
@yaakov-h

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

throw new TaskCanceledException();

?

@Thaina

This comment has been minimized.

Copy link
Author

commented Jan 22, 2019

@yaakov-h

try
{
    var source = new TaskCompletionSource<object>();
    source.TrySetCanceled();
    await source.Task;
}
catch(Exception e)
{
    // should not come here
}
@yaakov-h

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

image

😕

Are you saying that it does that now, or that you want it to do that?

@Thaina

This comment has been minimized.

Copy link
Author

commented Jan 22, 2019

@yaakov-h That's weird. Maybe I did something wrong but I have test it that this code was not throw any exception

But if yours is the correct behavior then that means we have no way to just cancel the task continuation without throwing exception? Unlike the callback function that we have an option to just not call the callback and it will not be continued

If it is then I would like to request this feature to allow this behavior

@yaakov-h

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

then that means we have no way to just cancel the task continuation without throwing exception

Environment.FailFast? 😄

@Thaina

This comment has been minimized.

Copy link
Author

commented Jan 22, 2019

@yaakov-h So you would like to close whole process to just discontinue one task?

Dude we could have many parallel task spawned from UI to loading something. Sometimes we just want to silently cancel some of them. It should just close without exception

As I said, at the time when we use callback function for asynchronous, we could just not call the callback and the whole task chain will silently end. I just want to imitate that feature in async/await system

@HaloFour

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

@Thaina

That's the definition of a cancelled task:

The task acknowledged cancellation by throwing an OperationCanceledException with its own CancellationToken while the token was in signaled state, or the task's CancellationToken was already signaled before the task started executing.

https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskstatus?view=netframework-4.7.2

Either way, this would be a BCL concern. C# doesn't know about Task or CancellationToken.

@svick

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

@Thaina

First of all, you should always call the callback. What if it does something important, like cleanup, or retrying the operation? With proper cancellation, the callback has a chance to respond to what happened. If you drop it, then the callback doesn't have that.

Second, if you really want to do this with async-await, there is a simple way: await Task.Delay(-1). Task.Delay(-1) will return a Task that can never complete, which effectively means that anything that depends on it will silently end.

For example:

using System;
using System.Threading;
using System.Threading.Tasks;

static class Program
{
    static void Main()
    {
        M();
        
        while (true)
        {
            GC.Collect();
            Thread.Sleep(100);
        }
    }

    static async Task M()
    {
        using (new C())
        {
            await DoSomething();
        }
    }

    static async Task DoSomething()
    {
        await Task.Delay(-1);
    }
}

class C : IDisposable
{
    public void Dispose() => Console.WriteLine("Disposed");
    
    ~C() => Console.WriteLine("Finalized");
}

If you run this code (in Release mode), Dispose will never be called, but the finalizer will. That seems to be exactly what you're asking for.

@jnm2

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

new TaskCompletionSource<object>().Task; (a never task) is something I use in unit tests sometimes.

@Thaina

This comment has been minimized.

Copy link
Author

commented Jan 22, 2019

@svick Surely normally we would want return a task and continue the next task properly. But there were some situation we would like to really cancel it and don't want anything catch it

I'm not sure but unlike the callback function that just not get called and go out of stack. The task that will never complete make me feel like the task chain will kept the continuation chain waiting in the memory for that never finish task. Like a infinite loop of the spawned thread that will run in memory, wasted resource indefinitely. So I want to have it properly cancel. Ideally it would called finally for cleanup but not throw any exception because it not an error

@svick

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

@Thaina

The task that will never complete make me feel like the task chain will kept the continuation chain waiting in the memory for that never finish task.

You might feel that way, but it's not what actually happens. That's the point of the finalizer in my code: to show that the continuation chain does get garbage collected.

Ideally it would called finally for cleanup but not throw any exception because it not an error

I think the idea of cancellation being different from throwing an exception does have some merit (which is why Task.IsCanceled is different from Task.IsFaulted). But doing that would effectively obsolete all of the existing code that uses cancellation and it would be a pretty significant change to the .Net execution model. That means it would need a really good justification, and I don't see that happening.

@Thaina

This comment has been minimized.

Copy link
Author

commented Jan 23, 2019

That is sadly but acceptable that it need to change the behavior of existing task system so it most likely be breaking change and I too would not want that

@Thaina Thaina closed this Jan 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.