For .NET 3.5 how would you make a thread-safe get or add cache?

Question

I have starting working with some .NET 3.5 code and found the following extension method being used for a cache::

public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> @this, TKey key,Func<TKey,TValue> factory,bool useLocking)
{

    TValue value;
    if([email protected](key,out value))
    {
        if (useLocking)
        {
            lock ((@this as ICollection).SyncRoot)
            {
                if ([email protected](key, out value))
                {
                    @this[key] = value = factory(key);
                }
            }
        }
        else
        {
            @this[key] = value = factory(key);
        }
    }
    return value;
}

The cache in question is keyed by string keys, and with useLocking = true. It is always being accessed by this method (there is no stray TryGetValue). There is also no issue in using the SyncRoot property because the dictionary is private and no where else is it being used. The double locking is dangerous because a dictionary does not support reading while it's being written to. While technically there has been no issue reported yet as the product hasn't shipped, I feel that this approach is going to lead to race conditions.

1. Switch the Dictionary<,> to a Hashtable. We'll lose type-safety but we'll be able to support the concurrency model we're after, (1 writer, multiple readers).

2. Remove the outer TryGetValue. That way every read, requires acquiring a lock. This is potentially bad for performance, but acquiring an uncontested lock should be fairly cheap.

Both are pretty crappy. Does anyone have a better suggestion? If this was .NET 4 code I'd just switch it to a ConcurrentDictionary, but I don't have that option.

Answer 1

Yes, your suspicions are correct; there is a race condition and all methods, even TryGetValue need to be inside the lock in the implementation you presented.

As far as performance, you can look forward to the day that you can upgrade to .NET4 which includes a screaming-fast ConcurrentDictionary out of the box. Until then, you can review James Michael Hare's analysis done here:

• C#: The Curious ConcurrentDictionary

Those results suggest to me that the best implementation for .NET3.5 is Dictionary plus ReadWriteLockSlim and for good measure, here's a complete implementation:

• Thread Safe Dictionary in .NET with ReaderWriterLockSlim

Update:

I read the tables wrong and it looks like Dictionary + lock is little faster than the only other serious contender Dictionary + ReadWriteLockSlim.