CODEWITHSUNDEEP
c#.netasynchronouslockingsemaphore
Christoph Fink
Christoph Fink

Reputation: 23113

SemaphoreSlim not working under extreme conditions?

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

Upvotes: 2

Views: 2330

Answers (1)

Stephen Cleary
Stephen Cleary

Reputation: 457197

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.

Upvotes: 2

Related Questions