3

I'm using CancellationTokenSource for timeout purposes most of the time. To avoid forgetting dispose, when I'm done with the CancellationTokenSource, I'm using a using statement. But before the end of the using statement, I'm always doing a CancellationTokenSource.Cancel().

Is it necessary to cancel CancellationTokenSource before disposal if the cancellation hasn't been used?

Here is an example of code where I'm doing this.

using (CancellationTokenSource TokenSource = new CancellationTokenSource(nTimeout * 1000))
{
  for (int i = 0; i < nLoop; i++)
  {
    if (TokenSource.Token.IsCancellationRequested)
    {
      bSuccess = false;
      break;
    }

    await Task.Delay(cDelay);
    // do some work
  }

  TokenSource.Cancel();
}
Jerome
  • 366
  • 5
  • 17

3 Answers3

3

After a CancellationTokenSource has been disposed tokens based on this source may throw ObjectDisposedException so you should not use CancellationTokenSource.Token after the source has been disposed. Fortunately, I don't see this happening in your code.

When you cancel a CancellationTokenSource it changes state and notifies callbacks that have been registered for the token. However, when your code is about to dispose the CancellationTokenSource you are already done using the token and there is no need to cancel it.

So in your case it is not necessary to cancel the CancellationTokenSource before disposing it. However, your use case is somewhat special. When you have a background task you should wait for the task to complete before disposing the source (as stated in my initial paragraph):

using (var cts = new CancellationTokenSource()) {
    var task = Task.Run(() => DoSomething(cts.Token));
    // Cancel cts or let it cancel itself based on a timeout.
    // Then wait for the task to end.
    await task;
}
Martin Liversage
  • 104,481
  • 22
  • 209
  • 256
  • 1
    Thanks Martin. What happen if I dispose the CancellationTokenSource without cancelling it before? Is there any impact on the memory or on any other thing? – Jerome Jan 31 '18 at 10:14
  • I ran a few test: I see no impact when cancelling the CancellationTokenSource before disposing it! Hence, I don't see the necessity of doing it systematically. – Jerome Jan 31 '18 at 11:01
  • @Jerome: Based on your comments and the edited question I have updated my answer. – Martin Liversage Jan 31 '18 at 12:22
  • The answer is clear regarding the CancellationTokenSource. Now, you've opened the hidden part of the question: what about the timeout related pending task? Actually I'm using one dedicated task for the timeout (let's say Task_A) and I'm using another task to run some work (working with HTML and DOM objects from the web), let's say Task_B. As I'm running the code `Task.WhenAny(Task_A, Task_B)`, when one of them is completing, what shall I do on the other one to do things properly? – Jerome Jan 31 '18 at 13:30
  • You are using `Task.WhenAny()` so I assume that you only care about the task that completes first. The other task should then be cancelled and awaited. However, you may not want to await the other task if doesn't cancel immediately. In that case I suggest that you cancel the task and dispose the source and then let the task continue until either it discovers that it is cancelled or it ends on its own. The risk of `ObjectDisposedException` is quite small because apparently the task does not integrate deeply with the cancellation token. Retrieving `IsCancellationRequested` does not throw. – Martin Liversage Jan 31 '18 at 14:02
  • Well Martin, you guess it right! If the non-timeout task is finishing first, I just don't care about the pending timeout. And if the timeout is finishing first, I would tell the same. But my concern is to do it the right way. If the timeout is finishing first, I need to cancel properly the task which is doing some stuff like the `NavigateAsync` in [link]https://stackoverflow.com/a/22250054/7991036 from Stephen Cleary. – Jerome Jan 31 '18 at 14:34
  • If you only use the cancellation token to provide a timeout while using a web browser control what is wrong with the answer you linked to which doesn't use a cancellation token? Anyway, I'm not sure if there is a new question in your latest comment but if there is I suggest that you post it as a new question on this site. I believe that I have answered the question in your title (the answer is no) but I'm not sure that this title really covers the problem you are concerned about. – Martin Liversage Jan 31 '18 at 14:48
  • Many thanks Martin, I have learnt a lot from your answer! I guess there's nothing wrong with the solution from the link I provided... It was just to avoid any mistake from me taking code from other (good) answers. – Jerome Jan 31 '18 at 17:06
2

For the question "Is it necessary to cancel CancellationTokenSource before disposal?" the answer is no, it's not necessary. But canceling it or not determines the final state of the the orphaned token.

As a general rule, an orphaned token should not be used after disposing it's source. But if you're curious what happens if you try to use it anyway, it should work to some extent. For example check this test:

[Theory]
[InlineData(false)]
[InlineData(true)]
public void Test48538934(bool canceledBeforeDispose)
{
    var cts = new CancellationTokenSource();
    var ct = cts.Token;

    Assert.False(cts.IsCancellationRequested);
    Assert.False(ct.IsCancellationRequested);

    if (canceledBeforeDispose)
        cts.Cancel();

    Assert.Equal(canceledBeforeDispose, cts.IsCancellationRequested);
    Assert.Equal(canceledBeforeDispose, ct.IsCancellationRequested);

    cts.Dispose();

    Assert.Throws<ObjectDisposedException>(() => cts.Token);

    Assert.Equal(canceledBeforeDispose, cts.IsCancellationRequested);
    Assert.Equal(canceledBeforeDispose, ct.IsCancellationRequested);
}

So in case that a token is accidentally lost somewhere in the asynchronous calls pipeline, canceling the source before disposing it might be desirable.

Sinus32
  • 21
  • 3
0

If you want your operations to be notified about cancellation, then yes you should explicitly call Cancel before Dispose. Usually this is a good thing.

MS also make this explicit in their documentation:

Note that calling Dispose does not communicate a request for cancellation to consumers of the associated Token. You can communicate a request for cancellation by calling methods such as Cancel or CancelAfter.

nawfal
  • 70,104
  • 56
  • 326
  • 368