Canceling a SemaphoreSlim.WaitAsync(CancellationToken) does not always throw OperationCanceledException if semaphore immediately released

Question

Consider the following .Net 6 console program:

// See https://aka.ms/new-console-template for more information
Console.WriteLine("Hello, World!");

var semaphore = new SemaphoreSlim(1, 1);
var cts = new CancellationTokenSource();

var tasks = Enumerable.Range(0, 10).Select(WaitingTask);

var t = Task.Run(async () =>
{
    await Task.WhenAll(tasks);
    Console.WriteLine("Tasks complete");
});

await Task.Delay(500);
Console.WriteLine("Press any key to Cancel waiting tasks, then immediately release semaphore");

Console.ReadKey();
cts.Cancel();
semaphore.Release();

await t;

async Task WaitingTask(int i)
{
    try
    {
        Console.WriteLine($"{i} Waiting");
        await semaphore.WaitAsync(cts.Token);
        Console.WriteLine($"{i} Aquired");
        await Task.Delay(50);
        Console.WriteLine($"{i} Complete");
    }
    catch (OperationCanceledException)
    {
        Console.WriteLine($"{i} Cancelled");
    }
}

It creates 10 tasks that try to acquire a lock on a semaphore that only allows 1 entry at a time. After the first task has reported completion, and the other nine tasks report that they are waiting for the semaphore, I wish to cancel the token passed to the waiting tasks, and then immediately release the lock on the semaphore.
Expected: the remaining 9 tasks throw and handle OperationCanceledException, and report "Canceled".
Actual: 8 of the remaining tasks do this, but 1 of them will sucessfully enter the semaphore and complete normally. I.e. you cannot reliably cancel calls to WaitAsync(CancellationToken)

Commenting out the line semaphore.Release(); results in all 9 tasks reporting canceled as expected.

I'm assuming a race condition somewhere but my question is: Am I wrong to expect my stated behaviour, and if so why?

Many thanks.

Example output:

Hello, World!
0 Waiting
0 Aquired
1 Waiting
2 Waiting
3 Waiting
4 Waiting
5 Waiting
6 Waiting
7 Waiting
8 Waiting
9 Waiting
0 Complete
Press any key to Cancel waiting tasks, then immediately release semaphore
1 Aquired
4 Cancelled
8 Cancelled
5 Cancelled
9 Cancelled
6 Cancelled
3 Cancelled
2 Cancelled
7 Cancelled
1 Complete
Tasks complete

Answer 1

First and foremost, generally cancelling does not guarantee an exception to be thrown, merely that the Task terminates in the near future, this can be short-circuited by returning.

A semaphore should always be used, similar to this or the synchronized equivalent:

await sem.WaitAsync(ct)
try {
    [...]
} finally {
    sem.Release();
}

Never ever release a not acquired semaphore.

Anyway, looking at your snippet in sharplab.io it becomes apparent, that delaying a long time before cancelling solves the issue, we got a race condition. CLR could technically reorder the statements (all thought async methods do not generally reorder), so that the cts.Cancel(); occurs after the semaphore.Release(), but that is not the case.

Let's have a look at the SemaphoreSlim source code, specifically SemaphoreSlim.WaitAsync(CancellationToken), which queues waiting tasks in a locking linked list, extending Task. Which means TAP handles cancellation. In most cases, such as with Tasks the cts is periodically checked by the consumer, so that there is no guarantee for immediate cancellation. The semaphore explicitly enures that a OperationCanceledException is always thrown, when cancelled from a different thread.

My best guess as to why await Task.Delay(10); terminates correctly is, that TAP verifies some/all cts when a Task is awaited. That's why the task queue of the semaphore cancels.

Onto the solution. You should use a Monitor lock to verify the state if you want exact timing, or better in your case check the CancellationToken manually

using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;


// See https://aka.ms/new-console-template for more information
Console.WriteLine("Hello, World!");

var semaphore = new SemaphoreSlim(1, 1);
var cts = new CancellationTokenSource();

var tasks = Enumerable.Range(0, 10).Select( i => WaitingTask(i, cts.Token));

var t = Task.Run(async () =>
{
    await Task.WhenAll(tasks);
    Console.WriteLine($"Tasks complete {Thread.CurrentThread.ManagedThreadId}");
});

await Task.Delay(500);
Console.WriteLine($"Cancelling {Thread.CurrentThread.ManagedThreadId}");

cts.Cancel();
//await Task.Delay(10); // comment out and see
semaphore.Release();

await t;

static async Task WaitingTask(int i, CancellationToken ct)
{
    int tid = Thread.CurrentThread.ManagedThreadId;

    try
    {
        Console.WriteLine($"{i} Waiting {tid}");
        await semaphore.WaitAsync(ct);
        ct.ThrowIfCancellationRequested();
        Console.WriteLine($"{i} Aquired {tid}");
        await Task.Delay(50);
        Console.WriteLine($"{i} Complete {tid}");
    }
    catch (OperationCanceledException)
    {
        Console.WriteLine($"{i} Cancelled {tid}");
    }
}

Please note, that you should never use the cts inside a closure, instead obtain a copy of the cts.Token: CancellationToken. Your life will be easier, if your async methods are stateless.

Answer 2

CancellationToken uses a "cooperative" model of cancellation, meaning it is non-blocking and dependent on the consumer of the token to cancel, which you are not doing in each of your task-bound methods.

As a result, if there is a delay in responding to the cancellation request, it is possible to experience the type of race condition that you've described. You have created this by calling .Release() prior to ensuring that the Task.WhenAll() call is completed, meaning that it is possible for the following to occur:

• cancellation is requested
• One task successfully completes semaphore.WaitAsync() but is then held up by Task.Delay
• Release of the semaphore is called
• The majority of the tasks cancel (those that did not successfully complete entry past semaphore.WaitAsync().

The only reason this is possible in the first place is because you add an artificial delay before calling release. Removing await Task.Delay(500) results in an exception.

If you want to avoid this sort of behavior with what you have, you can change your call order to the following:

cts.Cancel();
await t;
semaphore.Release();

This prevents the semaphore from releasing prior to all tasks completing, allowing each task to cooperatively cancel, even though the work of one task will still have completed. It gives the following output:

Hello, World!
0 Waiting
0 Aquired
1 Waiting
2 Waiting
3 Waiting
4 Waiting
5 Waiting
6 Waiting
7 Waiting
8 Waiting
9 Waiting
0 Complete
Press any key to Cancel waiting tasks, then immediately release semaphore
9 Cancelled
3 Cancelled
1 Cancelled
7 Cancelled
8 Cancelled
4 Cancelled
5 Cancelled
6 Cancelled
2 Cancelled
Tasks complete

Finally, note that in the real world, you shouldn't write code like this. Each task that completes work should release the semaphore after it is completed to avoid the myriad of race conditions that you have created.