how to use async method inside lock

958 Views Asked by At

I have CacheService that stores a collection in the MemoryCache and then finds an element from the collection that is stored in the cache. Given the multi threaded environment i want to make sure only one Worker can store the collection in the cache and find it. So i am using lock to synchronize the call and make thread-safe.

public class MyCacheService
{
    private readonly IMemoryCache _memoryCache = null;
    static object myLock = new object();

    public MyCacheService(IMemoryCache memoryCache)
    {
        _memoryCache = memoryCache ?? throw new ArgumentNullException(nameof(memoryCache));
    }       

    public async Task<Job> Find(int key, string title, int[] skills, Func<int, Task<List<Job>>> getJobs)
    {
        lock (myLock)
        {
            List<Job> cachedJobs = null;
            if (!_memoryCache.TryGetValue(key, out cachedJobs))
            {
                // compilation error here 'cannot await in the body of a lock statement'
                var jobs = await getJobs(key);

                var cacheEntryOptions = new MemoryCacheEntryOptions()
                    .SetSlidingExpiration(TimeSpan.FromMinutes(30));
                cachedJobs = _memoryCache.Set(key, cachedJobs, cacheEntryOptions);
            }

            if (cachedJobs != null)
            {
                var job = cachedJobs.Where(j => j.Title == title &&
                                   !j.Skills.Except(skills).Any())
                              .FirstOrDefault();

                if (job == null)
                {
                    return null;
                }

                cachedJobs.Remove(job);
                return job;
            }

            return null;
        }
    }
}

The getJobs delegate is async call to get jobs from the database. So i am getting error cannot await in the body of a lock statement

I understood why i am getting error. I can use getJobs(key).GetAwaiter().GetResult() to resolve error

There is LazyCache that guarantee single evaluation of delegate whose results you want to cache and we can use async delegate, but i have not used it

are there any other options?

UPDATE 1
I tried using SemaphoreSlim as suggested however it does not work as expected. In the DEMO below i have total 10000 jobs (5000 BASIC jobs and 5000 Master jobs) and total 200 workers. First 100 Workers (1-100) are for BASIC jobs and 101 to 200 workers for Master jobs.

Expectation is any worker from 1 to 100 will get BASIC job and 101-200 will get MASTER job

SemaphoreSlim does not seems to work as expected. With this approach All 5000 BASIC jobs always get assigned to Worker with ID 1. And all MASTER jobs always get assigned to Worker with ID 101

DEMO using SemaphoreSlim

My initial approach using C# lock seems to work as expected as long as i am not using async method inside lock

DEMO using lock

3

There are 3 best solutions below

0
On

@Stephen Cleary's answer still stands and works perfectly. Your demo code just doesn't execute all the worker tasks on a background thread like you would probably expect, so you're basically waiting for the first tasks (of every job type) to finish.

In your demo code you can run all the worker tasks as background tasks like so:

// your other code...
workers.AddRange(masterWorkers);
              
var start = DateTime.Now;
var tasks = workers.Select(s => Task.Run(s.DoWorkAsync)).ToArray();

Task.WaitAll(tasks);

var end = DateTime.Now;
// your other code

Fiddle: https://dotnetfiddle.net/GPasHM

It's also like that in the example in the docs of Microsoft; although the general recommendation of the try{} finally{} is missing .

0
On

You can use IDistributedCache that supports async delegation. And using Redis for cache implementation.

Distributed caching in ASP.NET core

0
On

Given the multi threaded environment i want to make sure only one Worker can store the collection in the cache and find it.

Your current (attempted) solution has a very coarse lock: if one request tries to find a job with a given key, it can be blocked by another request that is querying the db for jobs for a different key. That said, a literal translation of your existing code can be done using SempahoreSlim:

  static SemaphoreSlim myLock = new SemaphoreSlim(1);

  public async Task<Job> Find(int key, string title, int[] skills, Func<int, Task<List<Job>>> getJobs)
  {
    await myLock.WaitAsync();
    try
    {
      ...
    }
    finally
    {
      myLock.Release();
    }
  }