Cache class and modified collections

Question

I've written a generic Cache class designed to return an in-memory object and only evaluate src (an IQueryable, or a function returning IQueryable) occasionally. Its used in a couple of places in my app where fetching a lot of data via entity framework is expensive.

It is called

    //class level
    private static CachedData<Foo> fooCache= new CachedData<Foo>(); 

    //method level
    var results = fooCache.GetData("foos", fooRepo.Include("Bars"));

Although it appeared to work OK in testing, running on a busy web server I'm seeing some issues with "Collection was modified; enumeration operation may not execute." errors in the code that consumes the results.

This must be because one thread is overwriting the results object inside the lock, while another is using them, outside the lock.

I'm guessing my only solution is to return a copy of the results to each consumer rather than the original, and that I cannot allow the copy to occur while inside the Fetch lock, but that multiple copies could occur simultaneously.

Can anyone suggest a better way, or help with the locking strategy please?

public class CachedData<T> where T:class 
{
    private static Dictionary<string, IEnumerable<T>> DataCache { get; set; } 
    public  static Dictionary<string, DateTime> Expire { get; set; }
    public int TTL { get; set; }
    private object lo = new object();

    public CachedData()
    {
        TTL = 600;
        Expire = new Dictionary<string, DateTime>();
        DataCache = new Dictionary<string, IEnumerable<T>>();
    }

    public IEnumerable<T> GetData(string key, Func<IQueryable<T>> src)
    {
        var bc = brandKey(key);
        if (!DataCache.ContainsKey(bc)) Fetch(bc, src);
        if (DateTime.Now > Expire[bc]) Fetch(bc, src);
        return DataCache[bc];
    }


    public IEnumerable<T> GetData(string key, IQueryable<T> src)
    {
        var bc = brandKey(key);
        if ((!DataCache.ContainsKey(bc)) || (DateTime.Now > Expire[bc])) Fetch(bc, src);
        return DataCache[bc];
    }

    private void Fetch(string key, IQueryable<T> src )
    {
        lock (lo)
        {
            if ((!DataCache.ContainsKey(key)) || (DateTime.Now > Expire[key])) ExecuteFetch(key, src);
        }
    }

    private void Fetch(string key, Func<IQueryable<T>> src)
    {
        lock (lo)
        {
            if ((!DataCache.ContainsKey(key)) || (DateTime.Now > Expire[key])) ExecuteFetch(key, src());
        }
    }

    private void ExecuteFetch(string key, IQueryable<T> src)
    {
        if (!DataCache.ContainsKey(key)) DataCache.Add(key, src.ToList());
        else DataCache[key] = src.ToList();
        if (!Expire.ContainsKey(key)) Expire.Add(key, DateTime.Now.AddSeconds(TTL));
        else Expire[key] = DateTime.Now.AddSeconds(TTL);
    }


    private string brandKey(string key, int? brandid = null)
    {
        return string.Format("{0}/{1}", brandid ?? Config.BrandID, key);
    }
 }

Answer 1

I usually use a ConcurrentDictionary<TKey, Lazy<TValue>>. That gives you a lock per key. It makes the strategy to hold the lock while fetching viable. This also avoids cache-stampeding. It guarantees that only one evaluation per key will ever happen. Lazy<T> automates the locking entirely.

Regarding your expiration logic: You could set up a timer that cleans the dictionary (or rewrites it entirely) every X seconds.