-1

Pls look at this code, running on .Net Core 2.0:

var src = new CancellationTokenSource(5000);
var token = src.Token;
var responseTask = await Task.Factory.StartNew(async () =>
{
   //Uncomment bellow to reproduce locally
   //await Task.Delay(60000);

   return await BadSDK.OperationThatDoesNotReceiveCancellationToken();//takes around 1 min
}, token);

var response = await responseTask;

My issue here is that the await is always awaiting the very long sdk call, instead of waiting the 5sec.

What am I doing wrong? Where is my understanding wrong?

Edit1: This code behaves as expected:

var src = new CancellationTokenSource(5000);
var token = src.Token;
var responseTask = Task.Factory.StartNew(() =>
{
    var task = BadSDK.OperationThatDoesNotReceiveCancellationToken();
    task.Wait(token);
    cancellationToken.ThrowIfCancellationRequested();
    return task.Result;
}, token);

meaning, after 5 seconds, a exception is thrown

Leonardo
  • 10,737
  • 10
  • 62
  • 155
  • 3
    1. You shouldn’t use `Task.Factory.StartNew`. The recommended way is `Task.Run`. 2. Why are you even wrapping a task in another task? 3. In your first example the cancellation token is only used for task creation, that’s why nothing happens if the cancellation token triggers cancellation after the task was created. In your second example you cancel the outer task, but the inner task will still run. If you have a task that can’t be cancelled, there is no proper way to abort it. – ckuri Apr 16 '20 at 17:54
  • If you have just created the outer task in order to cancel the inner task, then that’s the wrong way. A correct way would be `await Task.WhenAny(BadSDK.Operation…, Task.Delay(5000));` which will complete if your BadSDK task completes or after 5s, whichever comes first. But remember, the BadSDK task will still run in the background, it just doesn’t get awaited anymore after 5s. – ckuri Apr 16 '20 at 18:00
  • @ckuri 1. Sources? I've never read such thing and actually read that `Run` is a wrapper with default parameters on `StartNew`. 2.Because I was hoping to use the cancel token and `OperationThatDoesNotReceiveCancellationToken ` obviously doesn't accept it. 3. If you had actually run the code, you would see that an exception is thrown at `Task.Wait(token)` and not the line below. That is the behaviour I was expecting on the first try. 4. on the real world, the token would come as a param, so I wouldn't know how much time I got before it expires, that's why `WhenAny` technique is not applied here. – Leonardo Apr 16 '20 at 21:15
  • 1. It’s stated in the [documentation of Task.Factory.StartNew](https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskfactory.startnew?view=netframework-4.8#remarks). 3. I never stated where the exception is thrown and it doesn’t matter either for my point that the inner task still runs and this only aborts the outer task. 4. CancellationTokens can be wrapped as tasks via CancellationToken.Register and a TaskCancellationSource. Just replace the Task.Delay which such a CancellationToken wrapper task and you can still use the WhenAny approach. – ckuri Apr 16 '20 at 23:01
  • Something needs to be clarified here. There is nothing wrong with using Task.Factory.StartNew, in fact it is preferred if your task is expected to take an extended period of time, > 100 ms. In such cases you add the TaskCreationOptions.LongRunning flag as a parameter to ensure the thread pool isn't used, instead a new thread is created. In this way the thread pool isn't exhausted by long running threads. The documentation in the link provided only displays a warning indicating that it is easier to use Task.Run because a default configuration would be used... which is the thread pool. – Nick Hanshaw Dec 14 '21 at 14:36

3 Answers3

2

The problem is that the cancellation token pattern expects the task to check the token and exit or throw an error when the token has expired. Well written tasks will periodically check to see if cancellation is canceled and then the task can do any cleanup necessary and return gracefully or throw an error.

As you demonstrated, BadSDK.OperationThatDoesNotReceiveCancellationToken doesn't accept a CancellationToken and thus won't take any action based on the token. It doesn't matter if the token automatically requests cancellation via a timeout, or if the request is issued in some other matter. The simple fact is that BadSDK.OperationThatDoesNotReceiveCancellationToken simply isn't checking it.

In your Edit1, the CancellationToken is passed to Wait which does watch the token and will exit when cancellation is requested. This does not mean however that task was killed or stopped, it only stopped waiting for it. Depending on what you intend, this may or may not do what you want. While it does return after 5 seconds, the task will still continue running. You could even wait on it again. It may be possible to then kill the task but in practice, it can be a very bad thing to do (See Is it possible to abort a Task like aborting a Thread (Thread.Abort method)?)

Tim
  • 606
  • 4
  • 7
1

The StartNew overload you used is a source of endless confusion. Doubly so, since its type is actually StartNew<Task<T>> (a nested task) rather than StartNew<T>.

A token on its own does nothing. Some code somewhere has to check the token, and throw an exception to exit the Task.

Official documentation follows this pattern:

    var tokenSource = new CancellationTokenSource();
    var ct = tokenSource.Token;

    var task = Task.Run(() =>
    {
        while (...)
        {
            if (ct.IsCancellationRequested) 
            {
                // cleanup your resources before throwing

                ct.ThrowIfCancellationRequested();
            }
        }
    }, ct); // Pass same token to Task.Run

But if you're anyway checking the token, and maybe throwing an exception why do you need to pass in the token in the first place, and then use the same token inside the closure?

The reason is that the token you pass in is what's used to move the Task to a cancelled state.

When a task instance observes an OperationCanceledException thrown by user code, it compares the exception's token to its associated token (the one that was passed to the API that created the Task). If they are the same and the token's IsCancellationRequested property returns true, the task interprets this as acknowledging cancellation and transitions to the Canceled state.

P.S.

If you're on .Net Core or 4.5+, Task.Run is preferred to the factory approach.

Asti
  • 12,447
  • 29
  • 38
0

You are passing the task the cancelation token token, but you don't specify what to do with the token within the async method.

You'd probably want to add token.ThrowIfCancellationRequested(); in the method, perhaps conditioned by token.IsCancellationRequested. This way the task will be canceled if src .Cancel() is called.

Jimenemex
  • 3,104
  • 3
  • 24
  • 56