Cancellation of SemaphoreSlim.WaitAsync keeping semaphore lock

Question

In one of our classes, we make heavy use of SemaphoreSlim.WaitAsync(CancellationToken) and cancellation of it.

I appear to have hit a problem when a pending call to WaitAsync is cancelled shortly after a call to SemaphoreSlim.Release()(by shortly, I mean before the ThreadPool has had a chance to process a queued item), it puts the semaphore in a state where no further locks may be acquired.

Due to the non-deterministic nature of whether a ThreadPool item executes between the call to Release() and Cancel(), the following example does not always demonstrate the problem, for those circumstances, I have explicitly said to ignore that run.

This is my example which attempts to demonstrate the problem:

void Main()
{
    for(var i = 0; i < 100000; ++i)
        Task.Run(new Func<Task>(SemaphoreSlimWaitAsyncCancellationBug)).Wait();
}

private static async Task SemaphoreSlimWaitAsyncCancellationBug()
{
    // Only allow one thread at a time
    using (var semaphore = new SemaphoreSlim(1, 1))
    {
        // Block any waits
        semaphore.Wait();

        using(var cts1 = new CancellationTokenSource())
        {
            var wait2 = semaphore.WaitAsync(cts1.Token);
            Debug.Assert(!wait2.IsCompleted, "Should be blocked by the existing wait");

            // Release the existing wait
            // After this point, wait2 may get completed or it may not (depending upon the execution of a ThreadPool item)
            semaphore.Release();         

            // If wait2 was not completed, it should now be cancelled
            cts1.Cancel();             

            if(wait2.Status == TaskStatus.RanToCompletion)
            {
                // Ignore this run; the lock was acquired before cancellation
                return;
            }

            var wasCanceled = false;
            try
            {
                await wait2.ConfigureAwait(false);

                // Ignore this run; this should only be hit if the wait lock was acquired
                return;
            }
            catch(OperationCanceledException)
            {
                wasCanceled = true;
            }

            Debug.Assert(wasCanceled, "Should have been canceled");            
            Debug.Assert(semaphore.CurrentCount > 0, "The first wait was released, and the second was canceled so why can no threads enter?");
        }
    }
}

And here a link to the LINQPad implementation.

Run the previous sample a few times and sometimes you will see the cancellation of WaitAsync no longer allows any threads to enter.

Update

It appears this is not reproducible on every machine, if you manage to reproduce the problem, please leave a comment saying so.

I have managed to reproduce the problem on the following:

• 3x 64 bit Windows 7 machines running an i7-2600
• 64 bit Windows 8 machine running an i7-3630QM

I have been unable to reproduce the problem on the following:

• 64 bit Windows 8 machine running an i5-2500k

Update 2

I have filed a bug with Microsoft here, however so far they are unable to reproduce so it would really be helpful if as many as possible could try and run the sample project, it can be found on the attachments tab of the linked issue.

Answer 1

SemaphoreSlim was changed in .NET 4.5.1

.NET 4.5 Version of WaitUntilCountOrTimeoutAsync method is:

private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, int millisecondsTimeout, CancellationToken cancellationToken)
{ 
    [...]

    // If the await completed synchronously, we still hold the lock.  If it didn't, 
    // we no longer hold the lock.  As such, acquire it. 
    lock (m_lockObj)
    { 
        RemoveAsyncWaiter(asyncWaiter);
        if (asyncWaiter.IsCompleted)
        {
            Contract.Assert(asyncWaiter.Status == TaskStatus.RanToCompletion && asyncWaiter.Result, 
                "Expected waiter to complete successfully");
            return true; // successfully acquired 
        } 
        cancellationToken.ThrowIfCancellationRequested(); // cancellation occurred
        return false; // timeout occurred 
    }
}

Same method in 4.5.1:

private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, int millisecondsTimeout, CancellationToken cancellationToken)
{
    [...]

    lock (m_lockObj)
    {
        if (RemoveAsyncWaiter(asyncWaiter))
        {
            cancellationToken.ThrowIfCancellationRequested(); 
            return false; 
        }
    }

    return await asyncWaiter.ConfigureAwait(false);
}

asyncWaiter is basically a task that always returns true (completes in separate thread, always with True result).

Release method calls RemoveAsyncWaiter and schedules worker to complete with true.

Here is a possible issue in 4.5:

    RemoveAsyncWaiter(asyncWaiter);
    if (asyncWaiter.IsCompleted)
    {
        Contract.Assert(asyncWaiter.Status == TaskStatus.RanToCompletion && asyncWaiter.Result, 
            "Expected waiter to complete successfully");
        return true; // successfully acquired 
    } 
    //! another thread calls Release
    //! asyncWaiter completes with true, Wait should return true
    //! CurrentCount will be 0

    cancellationToken.ThrowIfCancellationRequested(); // cancellation occurred, 
    //! throws OperationCanceledException
    //! wasCanceled will be true

    return false; // timeout occurred 

In 4.5.1 RemoveAsyncWaiter will return false, and WaitAsync will return true.