35

Given the following code:

var cts = new CancellationTokenSource();

try 
{
    // get a "hot" task
    var task = new HttpClient().GetAsync("http://www.google.com", cts.Token);

    // request cancellation
    cts.Cancel();

    await task;

    // pass:
    Assert.Fail("expected TaskCanceledException to be thrown");
}
catch (TaskCanceledException ex) 
{
    // pass:
    Assert.IsTrue(cts.Token.IsCancellationRequested,
        "expected cancellation requested on original token");

    // fail:
    Assert.IsTrue(ex.CancellationToken.IsCancellationRequested,
        "expected cancellation requested on token attached to exception");
}

I would expect ex.CancellationToken.IsCancellationRequested to be true inside the catch block, but it is not. Am I misunderstanding something?

Todd Menier
  • 37,557
  • 17
  • 150
  • 173
  • Is the `ex.CancelationToken` instance equal (ReferenceEqual) to cts? Documentation states: "If the token is associated with a canceled operation, the `CancellationToken.IsCancellationRequested` property of the token returns `true`". – Alex Mar 28 '15 at 15:48
  • 3
    @Alex: `CancellationToken` is a struct, so `ReferenceEquals()` will always return false. – Peter Duniho Mar 28 '15 at 15:49
  • @PeterDuniho your comment suggests, that `object.ReferenceEquals` will return `false` for checking structs. Did you imply that, or did you mean that it must logically be `false` as the results of `IsCancellationRequested` differ? –  Mar 28 '15 at 15:55
  • 1
    @AndreasNiedermair: no. I mean that if you have `CancellationToken token = cts.Token;` and evaluate `object.ReferenceEquals(token, token)` (i.e. compare the `CancellationToken` value _to itself_), even that will return `false`, because value types have to be boxed before being passed as an `object` reference, and so the boxed objects will _always_ be different, even if they were obtained from the same value. – Peter Duniho Mar 28 '15 at 15:59
  • Can you post executable code? Write an assert into the catch. I suspect you are somehow misinterpreting things. This should work. – usr Mar 28 '15 at 15:59
  • @Alex As a side-note ... `CancellationToken.IsCancellationRequested` returns the value of the `CancellationTokenSource.IsCancellationRequested` ([reference.microsoft.com](http://referencesource.microsoft.com/#mscorlib/system/threading/CancellationToken.cs,f32abbe8e2f1f6c1,references)) and there's a custom implementation on [`Equals`](http://referencesource.microsoft.com/#mscorlib/system/threading/CancellationToken.cs,b20a9c6ab8670753,references) –  Mar 28 '15 at 16:00
  • 1
    @usr: the code he posted works. I copy/pasted it into a blank console app (in its own method, since `Main()` can't be `async`) and it behaves exactly as reported. – Peter Duniho Mar 28 '15 at 16:00
  • @PeterDuniho copying a struct is always memberwise so there is no potential to overlook copying some bits. It is not even possible to create such behavior. – usr Mar 28 '15 at 16:00
  • @usr: my point is that it might have been copied from some different `CancellationToken` value. Still, since it turns out the property simply retrieves the current value from the original `CancellationTokenSource` object, the value type copy semantics are irrelevant anyway and thus so is my previous comment. – Peter Duniho Mar 28 '15 at 16:02
  • @usr I've added assertions to the code as you requested. Test is failing as commented. Thanks. – Todd Menier Mar 28 '15 at 16:12
  • 11
    Note that this code has a race condition. The operation *might* be cancelled or it might just finish normally before it gets cancelled. You should cancel the token before starting the actual work (or ensure that the work can't possibly finish before the token is cancelled) to ensure that this doesn't happen. – Servy Mar 30 '15 at 17:29

3 Answers3

51

That's the case because HttpClient internally (in SendAsync) is using a TaskCompletionSource to represent the async operation. It returns TaskCompletionSource.Task and that's the task you await on.

It then calls base.SendAsync and registers a continuation on the returned task that cancels/completes/faults the TaskCompletionSource's task accordingly.

In the case of cancellation it uses TaskCompletionSource.TrySetCanceled which associates the canceled task with a new CancellationToken (default(CancellationToken)).

You can see that by looking at the TaskCanceledException. On top of ex.CancellationToken.IsCancellationRequested being false ex.CancellationToken.CanBeCanceled is also false, meaning that this CancellationToken can never be canceled as it wasn't created using a CancellationTokenSource.


IMO it should be using TaskCompletionSource.TrySetCanceled(CancellationToken) instead. That way the TaskCompletionSource will be associated with the CancellationToken passed in by the consumer and not simply the default CancellationToken. I think it's a bug (though a minor one) and I submitted an issue on connect about it.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • 6
    Great answer, +1 for reporting the issue. I'm not so sure agree that it's minor, as I (and [others](http://stackoverflow.com/q/12666922/62600)) rely on this to distinguish between a user cancellation and a timeout in the case of `HttpClient`. – Todd Menier Mar 30 '15 at 18:09
  • 1
    @ToddMenier this was a surprisingly interesting question. It also lead me to this interesting (I hope) finding: http://stackoverflow.com/q/29355165/885318 – i3arnon Mar 30 '15 at 19:59
  • Well, the overload you are thinking of *is internal*, so there was probably some reason why it's not supposed to be used this way. There's a lot of interesting things happening while the `TaskCompletionSource` cancellation is proceeding (for example, you can try to cancel the TCS multiple times). And neither TCS nor the `Task` it internally creates have anything to do with cancellation tokens - that's just one of those leaky abstractions. There really are two kinds of `Task`, Task-as-CPU-work vs Task-as-async-I/O, `HttpClient` using the latter. Sometimes I wish they were actually separate... – Luaan Mar 31 '15 at 07:50
  • @i3arnon The documentation is incorrect, this overload is still internal in .net 4.5.2, if you look [here](https://msdn.microsoft.com/en-us/library/dn823326(v=vs.110).aspx) under Version Information .NET Framework you can see it only lists .NET 4.6, despite being listed as part of the .NET 4.5 API [here](https://msdn.microsoft.com/en-us/library/dn765345(v=vs.110).aspx) – Lukazoid Mar 31 '15 at 12:02
  • @Lukazoid I see on on my 4.5.3 preview. Anyways... It's public now and can be used to solve that issue. – i3arnon Mar 31 '15 at 12:21
  • 1
    that connect issue link 404s for me – Tim Abell Dec 16 '15 at 16:28
  • @Luaan There was no _good_ reason why the method was marked internal, since it became public in .NET 4.6 :) – Søren Boisen Apr 06 '16 at 14:16
  • broken connect link? – ympostor Jan 12 '17 at 04:36
  • @ympostor more like they deleted the connect issue. – i3arnon Jan 12 '17 at 08:38
  • so no improvement in this area? :( – ympostor Jan 12 '17 at 10:48
  • @ympostor guess not. – i3arnon Jan 12 '17 at 22:02
  • The issue you opened is now in an archived forum. I'm not sure if that was ever the right place to report a bug in .NET, but I've opened an issue in the new bug tracking area: https://connect.microsoft.com/VisualStudio/feedback/details/3141135 – StriplingWarrior Sep 15 '17 at 21:24
  • 3
    @StriplingWarrior the issue you've opened is in the exact same place. In any case, they replied last week and said it will be fixed for .NET 4.7.2 – i3arnon Sep 17 '17 at 21:01
  • @i3arnon: Sorry, I had a bunch of related SO posts open, and was looking at a different bug report. My bad. – StriplingWarrior Sep 18 '17 at 16:08
3

@Bengie This didn't work for me. I had to alter it a little. IsCancellationRequested always returned true so i couldn't rely on that.

This worked for me:

using (CancellationTokenSource cancelAfterDelay = new CancellationTokenSource(TimeSpan.FromSeconds(timeout)))
{
    DateTime startedTime = DateTime.Now;

    try
    {
        response = await request.ExecuteAsync(cancelAfterDelay.Token);
    }
    catch (TaskCanceledException e)
    {
        DateTime cancelledTime = DateTime.Now;
        if (startedTime.AddSeconds(timeout-1) <= cancelledTime)
        {
            throw new TimeoutException($"An HTTP request to {request.Url} timed out ({timeout} seconds)");
        }
        else
            throw;
    }
}
return response;
Janett Holst
  • 146
  • 1
  • 5
0

I set the timeout to infinite to disable it, then I pass in my own cancellation token.

using(CancellationTokenSource cancelAfterDelay = new CancellationTokenSource(timespan/timeout))
...
catch(OperationCanceledException e)
{
if(!cancelAfterDelay.Token.IsCancellationRequested)
throw new TimeoutException($"An HTTP request to {request.Uri} timed out ({(int)requestTimeout.TotalSeconds} seconds)");
else
throw;
}
Bengie
  • 1,035
  • 5
  • 10