How should I make my hashtable cache implementation thread safe when it gets cleared?

Question

I have a class used to cache access to a database resource. It looks something like this:

//gets registered as a singleton
class DataCacher<T>
{
    IDictionary<string, T> items = GetDataFromDb();

    //Get is called all the time from zillions of threads
    internal T Get(string key)
    {
         return items[key];
    }

    IDictionary<string, T> GetDataFromDb() { ...expensive slow SQL access... }

    //this gets called every 5 minutes
    internal void Reset()
    {
          items.Clear();
    }
}

I've simplified this code somewhat, but the gist of it is that there is a potential concurrency issue, in that while the items are being cleared, if Get is called things may go awry.

Now I can just bung lock blocks into Get and Reset, but I'm worried that the locks on the Get will reduce performance of the site, as Get is called by every request thread in the web app many many times.

I can do something with a doubly checked locks I think, but I suspect there is a cleaner way to do this using something smarter than the lock{} block. What to do?

edit: Sorry all, I didn't make this explicit earlier, but the items.Clear() implementation I am using is not in fact a straight dictionary. Its a wrapper around a ResourceProvider which requires that the dictionary implementation calls .ReleaseAllResources() on each of the items as they get removed. This means that calling code doesn't want to run against an old version that is in the midst of disposal. Given this, is the Interlocked.Exchange method the correct one?

Answer 1

I would start by testing it with just a lock; locks are very cheap when not contested. However - a simpler scheme is to rely on the atomic nature of reference updates:

public void Clear() {
    var tmp = GetDataFromDb(); // or new Dictionary<...> for an empty one
    items = tmp; // this is atomic; subsequent get/set will use this one
}

You might also want to make items a volatile field, just to be sure it isn't held in the registers anywhere.

This still has the problem that anyone expecting there to be a given key may get disappointed (via an exception), but that is a separate issue.

The more granular option might be a ReaderWriterLockSlim.

Answer 2

One option is to completely replace the IDictionary instance instead of Clearing it. You can do this in a thread-safe way using the Exchange method on the Interlocked class.

Answer 3

See if the database will tell you what data has change. You could use

• Trigger to write changes to a history table
• Query Notifications (SqlServer and Oracle has these, other must do as well)
• Etc

So you don’t have to reload all the data based on a timer.

---

Failing this.

I would make the Clear method create a new IDictionary by calling GetDataFromDB(), then once the data has been loaded set the “items” field to point to the new Dictionary. (The garbage collector will clean up the old dictionary once no threads are accessing it.)

I don’t think you care if some threads get “old” results while reloading the data – (if you do then you will just have to block all threads on a lock – painful!)

If you need all thread to swap over to the new dictionary at the same time, then you need to declare the “items” field to be volatile and use the Exchange method on the Interlocked class. However it is unlikely you need this in real life.