C#. SemaphoreSlim.WaitAsync does not throw OperationCanceledException when the task is canceled

Question

First of all, excuse my English. I will be brief, in the attached code after Task.WhenAny what I expected was that at least three of the five tasks would be canceled, but all ended satisfactorily. SemaphoreSlim.WaitAsync does not throw OperationCanceledException when the tasks are canceled.

class Program
{
    private static CancellationTokenSource methodRequests = new CancellationTokenSource();
    private static SemaphoreSlim semaphore = new SemaphoreSlim(1, 1);

    static void Main(string[] args)
    {
        int[] delays = new int[] { 5000, 5010, 5020, 5030, 5040 };

        IEnumerable<Task> tasks = from delay in delays select MethodAsync(delay, new CancellationTokenSource().Token);

        Task.WhenAny(tasks).Wait();

        methodRequests.Cancel();

        Console.ReadKey();
    }

    static async Task MethodAsync(int milliseconds, CancellationToken cancellationToken)
    {
        var methodRequest = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, methodRequests.Token);

        try
        {
            await semaphore.WaitAsync(methodRequest.Token);

            Thread.Sleep(milliseconds);

            Console.WriteLine($"Task finished {milliseconds}");
        }
        catch (OperationCanceledException)
        {
            Console.WriteLine($"Task canceled {milliseconds}");
        }
        finally
        {
            semaphore.Release();
        }
    }
}

What am I doing wrong?

Thanks.

Answer 1

The problem in your code is that the MethodAsync() method is never returning until the Thread.Sleep() method completes. This means that each task doesn't even start until the previous one finishes. Here is a version of your code that illustrates this more clearly:

private static CancellationTokenSource methodRequests = new CancellationTokenSource();
private static SemaphoreSlim semaphore = new SemaphoreSlim(1, 1);

static void Main(string[] args)
{
    int[] delays = new int[] { 5000, 5010, 5020, 5030, 5040 };

    IEnumerable<Task> tasks = from delay in delays select MethodAsync(delay, new CancellationTokenSource().Token);

    Task.WhenAny(tasks).Wait();

    methodRequests.Cancel();

    ReadKey();
}

static async Task MethodAsync(int milliseconds, CancellationToken cancellationToken)
{
    var methodRequest = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, methodRequests.Token);

    try
    {
        WriteLine($"waiting semaphore (will wait {milliseconds} ms)");
        await semaphore.WaitAsync(methodRequest.Token);

        WriteLine($"waiting {milliseconds} ms");
        Thread.Sleep(milliseconds);

        WriteLine($"Task finished {milliseconds}");
    }
    catch (OperationCanceledException)
    {
        WriteLine($"Task canceled {milliseconds}");
    }
    finally
    {
        semaphore.Release();
    }
}

The output for that is:

waiting semaphore (will wait 5000 ms)
waiting 5000 ms
Task finished 5000
waiting semaphore (will wait 5010 ms)
waiting 5010 ms
Task finished 5010
waiting semaphore (will wait 5020 ms)
waiting 5020 ms
Task finished 5020
waiting semaphore (will wait 5030 ms)
waiting 5030 ms
Task finished 5030
waiting semaphore (will wait 5040 ms)
waiting 5040 ms
Task finished 5040

As you can see, you never even see the "waiting for semaphore..." message until the previous task completes. This is because your LINQ select cannot proceed to the next element in the sequence until the MethodAsync() method returns the value for the current element, and that doesn't happen until the Thread.Sleep() completes.

You may have thought that the await semaphore.WaitAsync() should yield to the caller, allowing the Task to be returned and the enumeration of the select to continue. But, this would happen only if the semaphore was unavailable. It's available on each call, because each call happens only after the previous call completes, because when that previous call was made, the semaphore was available. Since the semaphore is available when the call to WaitAsync() is made, the await completes synchronously. I.e. the code proceeds directly to the Thread.Sleep() rather than yielding to the caller.

The net effect is that the call to WhenAny() doesn't even happen until all of the Thread.Sleep() calls (and of course, all of the semaphore.WaitAsync() calls as well) have completed.

Presumably this came up in some real-world scenario, and the code you posted is just for demonstration purposes. So, it's difficult to say exactly what you should fix. But, in the code example you posted, it's sufficient to just switch to Task.Delay() instead of Thread.Sleep(). Since the delay is always non-zero, the method will always yield at that point, even if the semaphore itself was available. This allows the select to proceed to the next call to MethodAsync() before the "work" in the current call has completed.

In this way, all of the tasks are in fact created concurrently as you'd originally expected.

Whatever the real-world code looks like, what you want to do is make sure that you've got an asynchronous operation after the semaphore acquisition, to allow the method to actually return to the caller, so that the next operation(s) can be started as well.