For .NET 3.5, how would you create thread-safe caching or adding a cache?

Question

I started working with some .NET 3.5 code and found the following extension method used for the cache:

public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> @this, TKey key,Func<TKey,TValue> factory,bool useLocking) { TValue value; if(!@this.TryGetValue(key,out value)) { if (useLocking) { lock ((@this as ICollection).SyncRoot) { if (!@this.TryGetValue(key, out value)) { @this[key] = value = factory(key); } } } else { @this[key] = value = factory(key); } } return value; } 

The cache in question is set by string keys, and using useLocking = true. This method is always accessed by this method (there is no wandering TryGetValue ). There is also no problem using the SyncRoot property, because the dictionary is closed and the other is not used. Double locking is dangerous because the dictionary does not support reading while writing. Although the problem has not yet been technically reported since the product has not been shipped, I believe that this approach will lead to race conditions.

• Switch Dictionary<,> to Hashtable . We will lose type safety, but we will be able to support the concurrency model that we are after (1 writer, several readers).

• Remove the external TryGetValue. Thus, each read requires the acquisition of a lock. This is potentially bad for performance, but getting an unfinished lock should be pretty cheap.

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

Answer 1

Yes, your suspicions are true; There is a race condition, and all methods, even TryGetValue must be inside the lock in the implementation you presented.

In terms of performance, you can count on the day when you can upgrade to .NET4, which includes a quick ConcurrentDictionary with a scream. Until then, you can view the James Michael Hare analysis done here:

• C #: Curious ConcurrentDictionary

These results tell me that the best implementation for .NET3.5 is Dictionary plus ReadWriteLockSlim and for good measure, here's the full implementation:

• Thread Safe Dictionary in .NET with ReaderWriterLockSlim

Update:

I read the tables incorrectly and looks like Dictionary + lock little faster than the only serious rival of Dictionary + ReadWriteLockSlim .

Answer 2

My recommendation would be to download Reactive Extensions for .NET (Rx) , which includes backups of collections in the System.Collections.Concurrent namespace, this includes ConcurrentDictionary<TKey, TValue> ) for .NET 3.5.

Answer 3

Remove TryGetValue. I bet you won't see the concurrency problem; CLR monitors are pretty fast and unfair, so you don’t see a problem with the escort or priority inversion.

If you see a concurrency problem, then the next best thing is ReaderWriterLockSlim. Unfortunately, you will want to create a new class for this instead of using the extension method, because you need a place to store the lock.

If you are following this route, be sure to avoid moving from reading to writer clips.

Answer 4

I think you need to block the full statement (as you said).

However, there is a clear solution on the Internet that should be prepared for a future .NET 4. update. Have a custom class instead of a dictionary, save the dictionary as a private member variable, and end all access to the dictionary in streaming mode, a safe lock instruction. A detailed description can be found here: http://www.grumpydev.com/2010/02/25/thread-safe-dictionarytkeytvalue/