TaskScheduler should be able to cancel pending tasks

[jkotas]

From @JeffCyr on November 22, 2017 18:48

Rationale

After a Task is enqueued with TaskScheduler.QueueTask(Task task), it is currently not possible to cancel or fail the task if the TaskScheduler can't execute the task.

This makes it hard to implement a disposable TaskScheduler because Dispose needs to block until all pending tasks are executed, which is not always possible, or have to leave the pending tasks uncompleted, which is not ideal either.

Proposed API

public abstract class TaskScheduler
{
    //...

    protected bool TrySetCanceled(Task task);

    protected bool TrySetException(Task task, Exception exception);
}

Details

• TrySetCanceled and TrySetException can only be called for a Task assigned to this TaskScheduler. This is similar to TryExecuteTask.
• TrySetCanceled and TrySetException are implemented with best effort. They will return false if the tasks is already running or canceled by a CancellationToken.

Pull Request

A PR with the proposed changes is available on my fork: JeffCyr/coreclr#1

Copied from original issue: dotnet/coreclr#15171

[JeffCyr]

cc @stephentoub , Any thoughts on this?

[stephentoub]

Any thoughts on this?

I'd need to think about it in more depth, but on the surface it seems reasonable.

[kevingosse]

That's something I could have used. We have a custom task scheduler in our codebase, that is bound to the lifetime of a request, and should stop executing new work after the request timeouts. But as mentioned, since there's no way of cancelling the enqueued task, it means that some code could be waiting on them forever.
The issue is aggravated by the fact that the task-creation methods on the TaskFactory can't be overriden. My workaround has been to provide my own custom task factory (that doesn't inherit from TaskFactory), and force the usage of a cancellation token linked to the lifetime of the TaskScheduler. When I need to kill the TaskScheduler, I cancel the token, then immediately execute all enqueued tasks (to transition them to the cancelled state). It works, but it's hardly optimal, and it'll break if the user bypasses my task factory by calling Task.Factory.StartNew to provide their own cancellation token.

[sharwell]

During a review of the expected final state of a Task, @jasonmalinowski suggested that the cancellation of an operation that used CancellationToken.None should transition to a failed state instead of to the canceled state, and I found the argument for this position compelling. Cancellation frequently leads to code flow situations which are difficult to understand, so I'm hesitant to support a change which makes it possible to further complicate a situation which already tends to be problematic.

[ReubenBond]

Canceling a Task which has been enqueued on a TaskScheduler seems to go against this remark from the TaskScheduler docs:

A task scheduler ensures that the work of a task is eventually executed.

My belief is that we should not change this requirement. For example, consider:

semaphoreSlim.WaitAsync().ContinueWith(t =>
{
  // do something under semaphore and release
});

If the continuation task is enqueued onto a TaskScheduler and that task is then canceled and not executed then the semaphore will not be released. Also consider the case where multiple continuations exist for the same antecedent.

If the WaitAsync task itself was canceled, so that the continuation would see a TaskCanceledException, then that behavior would also be error prone.

Please correct me if I'm wrong with any of that.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.