How to properly lock with concurrent multiple requests

Question

I have an auto refresh cache in our system, which is running into some issues due to race conditions.

During start up the _internalCache which is a concurrent dictionary is empty.

This was implemented years ago as a generic auto refresh cache used across our system.

The refresh action which is causing a most of the trouble, refreshes a few thousand rows from the database.

public bool TryGet(TKey key, out TValue value)
{
    if (_internalCache.TryGetValue(key, out value))
    {
        return true;
    }
    lock (_internalCache.SyncRoot)
    {
        this._refreshCacheAction(this._internalCache);
        return _internalCache.TryGetValue(key, out value);
    }
}

If multiple requests come in at the same time (which happens more often than I wish would) Then, we refresh our cache multiple times.

Edit: After further discussion from the comments, it looks like this cache is Seriously broken. Several of our customers are experiencing timeouts, which I need a quick hotfix.

How can I prevent multiple refreshes to the cache? (Jenky hacks are welcome)

Answer 1

In general the design looks flawed. Maybe even a standard component like ConcurrentDictionary or MemoryCache can be used.

However, one possible hotfix is to check again for the value in the internal cache inside the lock. This should reduce the number of times the refresh action is executed.

public bool TryGet(TKey key, out TValue value)
{
    if (_internalCache.TryGetValue(key, out value))
    {
        return true;
    }
    lock (_internalCache.SyncRoot)
    {
        // cache has already been refreshed
        if (_internalCache.TryGetValue(key, out value))
        {
           return true;
        }

        // refresh cache
        this._refreshCacheAction(this._internalCache);
        return _internalCache.TryGetValue(key, out value);
    }
}