2

I am writting an API that has a ValueTask<T> return type, and accepts a CancellationToken. In case the CancellationToken is already canceled upon invoking the method, I would like to return a canceled ValueTask<T> (IsCanceled == true), that propagates an OperationCanceledException when awaited. Doing it with an async method is trivial:

async ValueTask<int> MyMethod1(CancellationToken token)
{
    token.ThrowIfCancellationRequested();
    //...
    return 13;
}

ValueTask<int> task = MyMethod1(new CancellationToken(true));
Console.WriteLine($"IsCanceled: {task.IsCanceled}"); // True
await task; // throws OperationCanceledException

I decided to switch to a non-async implementation, and now I have trouble reproducing the same behavior. Wrapping a Task.FromCanceled results correctly to a canceled ValueTask<T>, but the type of the exception is TaskCanceledException, which is not desirable:

ValueTask<int> MyMethod2(CancellationToken token)
{
    if (token.IsCancellationRequested)
        return new ValueTask<int>(Task.FromCanceled<int>(token));
    //...
    return new ValueTask<int>(13);
}

ValueTask<int> task = MyMethod2(new CancellationToken(true));
Console.WriteLine($"IsCanceled: {task.IsCanceled}"); // True
await task; // throws TaskCanceledException (undesirable)

Another unsuccessful attempt is to wrap a Task.FromException. This one propagates the correct exception type, but the task is faulted instead of canceled:

ValueTask<int> MyMethod3(CancellationToken token)
{
    if (token.IsCancellationRequested)
        return new ValueTask<int>(
            Task.FromException<int>(new OperationCanceledException(token)));
    //...
    return new ValueTask<int>(13);
}

ValueTask<int> task = MyMethod3(new CancellationToken(true));
Console.WriteLine($"IsCanceled: {task.IsCanceled}"); // False (undesirable)
await task; // throws OperationCanceledException

Is there any solution to this problem, or I should accept that my API will behave inconsistently, and sometimes will propagate TaskCanceledExceptions (when the token is already canceled), and other times will propagate OperationCanceledExceptions (when the token is canceled later)?

Try it on Fiddle.


Update: As a practical example of the inconsistency I am trying to avoid, here is one from the built-in Channel<T> class:

Channel<int> channel = Channel.CreateUnbounded<int>();

ValueTask<int> task1 = channel.Reader.ReadAsync(new CancellationToken(true));
await task1; // throws TaskCanceledException

ValueTask<int> task2 = channel.Reader.ReadAsync(new CancellationTokenSource(100).Token);
await task2; // throws OperationCanceledException

The first ValueTask<int> throws a TaskCanceledException, because the token is already canceled. The second ValueTask<int> throws an OperationCanceledException, because the token is canceled 100 msec later.

Try it on Fiddle.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • https://stackoverflow.com/questions/34359129/operationcanceledexception-vs-taskcanceledexception-when-task-is-canceled#comment56461089_34359129? – GSerg Sep 02 '21 at 10:37
  • @GSerg it may not matter in practical usage, but it matters if you evaluate an API by its consistency. Currently I have two different unit tests for this API, `Assert.ThrowsException(() =>` for immediate cancellation and `Assert.ThrowsException(() =>` for delayed cancellation, which looks clunky to say the least. – Theodor Zoulias Sep 02 '21 at 10:59
  • You could use an assertion library that can handle matching derived types to the specified exception type. An example would be xUnit’s Assert.ThrowAny to handle both of your cases – Moho Sep 03 '21 at 05:48
  • @Moho to be honest I am more concerned about my API having inconsistent behavior, and switching to a unit testing library that offers more relaxed assertions is not in my priority list. :-) – Theodor Zoulias Sep 03 '21 at 11:25

2 Answers2

3

The best approach here is probably to simply have an async helper method you can defer to, here; i.e.:

ValueTask<int> MyMethod3(CancellationToken token)
{
    if (token.IsCancellationRequested) return Cancelled(token);
    // ... the rest of your non-async code here

    static async ValueTask<int> Cancelled(CancellationToken token)
    {
        token.ThrowIfCancellationRequested();
        // some dummy await, or just suppress the compiler warning about no await
        await Task.Yield(); // should never be reached
        return 0; // should never be reached
    }
}

There is a third option that might work, but it is a lot more complicated and doesn't avoid allocation (IValueTaskSource<TResult> - noting that you'd still need somewhere to stash the relevant token)

Sneakier version:


#pragma warning disable CS1998
static async ValueTask<int> Cancelled(CancellationToken token)
#pragma warning restore CS1998
{
    token.ThrowIfCancellationRequested();
    return 0; // should never be reached
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • You would have to question why not just letting this all being async to start with, compiler no allocations free pass, or is there there something I am missing here? – TheGeneral Sep 02 '21 at 11:20
  • 1
    @TheGeneral in some hot path scenarios, it is genuinely useful to avoid the `async` machinery if it turns out that you *can* answer the query synchronously - and only add that it you *need* to actually `await` something incomplete, or if you need to throw *and* you care about returning a faulted task/valuetask rather than simply throwing on the sync path – Marc Gravell Sep 02 '21 at 11:23
  • Thanks Marc, this looks promising. My API may throw multiple consecutive exceptions with the same `CancellationToken`. Do you know if it's wise to cache a canceled `ValueTask`, and return the same value to multiple callers? – Theodor Zoulias Sep 02 '21 at 11:30
  • 1
    @TheodorZoulias absolutely do not ever do that; you must **only** await a `ValueTask` once - behaviour is undefined against multiple awaits, largely due to `IValueTaskSource`, and handing out the same value-task to multiple callers would invalidate that. You *can* create a shared `Task` using the same trick, and return a `new ValueTask(GetSharedTask(token))`, however - i.e. the same underlying `Task` is fine – Marc Gravell Sep 02 '21 at 11:31
  • I tested waiting the same canceled `Task` by multiple threads, and the stack traces were all over the place. I got similar results with what is shown in [this](https://stackoverflow.com/questions/49534361/throw-same-exception-instance-multiple-times "Throw same exception instance multiple times") question. Wrapping the cached `Task` in a `ValueTask` results to the same mess. So it seems unavoidable that the `Cancelled()` method should be invoked every time. Now I am going to measure how much is the allocation overhead. Hopefully it will be dwarfed by the size of the `Exception` instance. – Theodor Zoulias Sep 02 '21 at 12:12
  • [Demo on fiddle](https://dotnetfiddle.net/SUZX5E). Shows what happens when a task is awaited concurrently by multiple asynchronous workflows. – Theodor Zoulias Sep 02 '21 at 12:21
  • I measured the allocations of the `Cancelled` approach. Running the test in my PC on .NET 5 in Release mode without debugger attached, the allocations per operation are going from 481 bytes to 953 bytes (doubled). As for the duration of each operation, it goes from ~70 μsec to ~140 μsec (also doubled). These results are not very satisfying, but I am accepting your answer anyway because probably no better solution exists. [Here](https://dotnetfiddle.net/1m2awI) is the test (the results are different, probably because the Fiddle server runs on debug mode). – Theodor Zoulias Sep 03 '21 at 02:24
0

Although it throws a TaskCanceledException as well, the ValueTask.FromCanceled method has been introduced with .NET 5 and above, which may be relevant here.

mducharm
  • 31
  • 2