Locking during cache population

Question

Here I want to lock when populating a particular cache object, without blocking other calls to the same method requesting Foos for other barIds. I realise the MemoryCache will be thread safe, but if two or more concurrent calls come in it seems like it would be better if only one of them populates the cache and locks the others out while this is done.

using System.Runtime.Caching;
using Microsoft.Practices.Unity;

public class FooCacheService : IFooService
{
        private static readonly ConcurrentDictionary<string, object> FooServiceCacheLocks = new ConcurrentDictionary<string, object>();

        [Dependency("Explicit")]
        public IFooService ExplicitService { get; set; }

        public IEnumerable<Foo> GetFoosForBar(int barId)
        {
            string key = barId.ToString();

            object lockObject = FooServiceCacheLocks.GetOrAdd(key, new object());

            object cached = MemoryCache.Default[key];

            if (cached == null)
            {
                lock (lockObject)
                {
                    cached = MemoryCache.Default[key];

                    if (cached == null)
                    {
                        IEnumerable<Foo> foosForBar = this.ExplicitService.GetFoosForBar(barId);

                        MemoryCache.Default.Add(key, foosForBar, DateTime.Now.AddMinutes(5));

                        return foosForBar;
                    }
                }
            }

            return (IEnumerable<Foo>)cached;
        }
}

Assume GetFoosForBar is a CPU/IO intensive process and takes a short while to complete. Does this seem like a reasonable way to achieve this? I'm surprised that System.Runtime.Caching does not provide any support for this as default - or have I overlooked something?

Answer 1

What I don't like about your approach is that the lock objects are never removed from FooServiceCacheLocks, even when the object is removed from the cache.

One way to simplify your code would be to combine MemoryCache with Lazy: instances of Lazy are cheap (as long as you don't access its Value) so you can create more of them than needed. With that your code would look something like this:

public IEnumerable<Foo> GetFoosForBar(int barId)
{
    var newLazy = new Lazy<IEnumerable<Foo>>(
        () => this.ExplicitService.GetFoosForBar(barId));

    var lazyFromCache = (Lazy<IEnumerable<Foo>>)MemoryCache.Default.AddOrGetExisting(
        barId.ToString(), newLazy, DateTime.Now.AddMinutes(5));

    return (lazyFromCache ?? newLazy).Value;
}