CODEWITHSUNDEEP
c#async-awaitthread-synchronization
Andy
Andy

Reputation: 13517

Calling method that locks from async method. Is this proper?

I am creating something that reads from a resource asynchronously, but it should only read every so often and the other times return a cached result. This method is called 10's of thousands of times a minute in an WebAPI ASP.NET Core 2.1 application.

I would like to synchronize the the cached data using a lock of sorts without the overhead of a semaphoreslim object, so I was wondering if this is proper:

    private const int LastReadTTL = 5000;
    private static int _lastTickCountRead;
    private static Models.MyModel _lastRead;
    private static readonly object _lock = new object();

    private static bool ShouldRead()
    {
        lock(_lock)
        {
            var currentTickCount = Environment.TickCount;
            if ((_lastRead == null) || (currentTickCount > (_lastTickCountRead + LastReadTTL)) || (currentTickCount < _lastTickCountRead))
            {
                _lastTickCountRead = currentTickCount;
                return true;
            }
        }
        return false;
    }

    public async Task<Models.MyModel> ReadSomethingAsync()
    {
        if (ShouldRead())
        {
            _lastRead = await SomethingToReadAsync.ConfigureAwait(false);
        }
        return _lastRead;
    }

If this isn't proper, why? and is the use of SemaphoreSlim more appropriate in this case?

Thank you

Upvotes: 1

Views: 247

Answers (1)

zivkan
zivkan

Reputation: 14961

This question belong on codereview.stackexchange.com rather than stackoverflow. In case the question doesn't get moved, my answer would be no, generally it's not a good pattern to lock in an async method.

The reason is that async methods are designed to yield control when unable to progress (typically waiting for IO to complete), so the thread can switch to other tasks that are ready to continue. By blocking the thread, it forces the operating system to do a context switch to another thread, later when the lock is ready for the blocked thread, do another context switch back again. Async was designed specifically to reduce the performance penalty of context switching, so blocking an async method is removing the benefit of the feature.

I recommend using any synchronization object that returns a task you can await on, whether that's semaphoreslim, something from the AsyncEx nuget package, or anything else. Hopefully anyone who created these know more about async in .NET than you or me, so should work better than anything we implement ourselves. There's a good chance they will use a blocking synchronization object, but do so only to avoid race conditions and quickly fall back to async waits when it didn't successfully get the lock, therefore giving the advantages of async while giving the synchronization guarantees required by concurrent systems.

Having said all that, if your use-case is just to update a value periodically, you should look at Interlocked.Exchange which gives you a lock-free way of updating a value or object instance.

Upvotes: 2

Related Questions