SemaphoreSlim not working under extreme conditions?

Question

We are developing an application using .NET 4.5.1 and implemented my own "async lock" using SemaphoreSlim in the background. To lock we use the following method:

public async Task<IDisposable> LockAsync(CancellationToken cancellationToken = default(CancellationToken))
{
    await SyncRoot.WaitAsync(cancellationToken).ConfigureAwait(false);
    return this;
}

Where the SyncRoot is the new SemaphoreSlim(1) instance.

This seems to work fine "in production", but it fails with the following unit-test:

for (int i = 0; i < numberOfTasks; i++)
{
    tasks[i] = Task.Run<Task>(async () =>
    {
        Interlocked.Increment(ref counterAtomicBegin);
        using (await alock.LockAsync().ConfigureAwait(false))
        {
            counterLocked += 1;
        }
        Interlocked.Increment(ref counterAtomicEnd);
    });
}

There the counterLocked and counterAtomicBegin are not equal at the end (at 100k tasks they are ~1k off).

Am I doing something wrong or is this a problem of SemaphoreSlim?

UPDATE: Remove nested logic as recommended and text adapted.
See the following code (can be executed in LINQPad) to test it out: http://pastebin.com/WXecZxqu

Answer 1

Your logic is incorrect.

Asynchronous locks are not thread-affine, so the entire concept of a thread holding a lock is incorrect. Rather, a certain section of code holds the lock, and parts of that code may execute on different threads.

In my opinion, recursive locks are a bad idea (link is to my blog post that goes into details). It is theoretically possible to write a recursive asynchronous lock (the only implementation I know of is part of the test suite for my AsyncEx library), but it would only work on .NET 4.5 full framework, and I still think it would be a bad idea.