C#: The best way to use shared resources?

Question

Hi there.

I need to serialize access to shared resource(ie Cache). I use pattern described later, but sometimes, especially the first two threads, loads data twice. Where is the problem?

public class Entity { }

public class CacheManager
{
 public static CacheManager Instance = new CacheManager();

 private CacheManager()
 {
  _thisLock = new Object();
  _cache = new Dictionary<int, Entity>();
 }

 private readonly Object _thisLock;
 private readonly IDictionary<int, Entity> _cache;

 public Entity GetEntity(int id)
 {
  Entity entity;
  if ( !_cache.TryGetValue(id, out entity) ) {
   /* Only one thread, at a time, can go inside lock statement.
    * So, if threads [1] and [2] came here at the same time, .NET shoud pass thread [1] inside and [2] waits. */
   lock ( _thisLock ) {
    if ( !_cache.TryGetValue(id, out entity) ) { /* If we are [2] : check(and loads from cache) if [1] did work for us. */
     /* we are [1] so let's load entity from repository and add it to cache */
     entity = new Entity(); // simulate repository access
     _cache.Add(id, entity);
    }
   }
  }
  return entity;
 }
}

Why both threads steps inside lock statement? Is it becouse i have quad core proccessor? If i add "Thread.Sleep(1);" before the lock statement, everything works fine. Should i use Mutex class?

Thank you.

Answer 1

I would recomend adding the lock before the if statement.

Answer 2

Your code is not thread safe. Since you're not locking in your first TryGetValue, a thread might try to access the dictionary while it's being modified (with the Add). In the current .Net Dictionary implementation, TryGetValue might raise an exception if the underlying dictionary is being modified.

You'd better dump the outer TryGetValue entirely or, better, use a ReaderWriterLockSlim.

To answer your question, it might come from the fact you're not accessing your singleton (Instance) from the calling code? Anyway, don't use this locking strategy, it's brittle. Use a ReaderWriterLockSlim or dump the first TryGetValue.

Answer 3

Feryt, Here's a scary thought: I've run into an acknowledged MS bug 3.5sp1 CLR in code generated for lock() in debug mode only(I'll try to find it if you interested). i have found that during a debug session I'll have two threads executing code inside of a lock() statement - not good. You can verify this by using a try/finally pair within the lock() that contain an interlocked increment/decrement on an int. I'd then call debugger.break() if that interlocked variable exceeds 1. You can also create a another variable that is incremented just before the lock and then immediately decremented once inside the lock (again using the interlock increment). These variables will show the number of threads waiting on a lock and the number of threads currently executing code within the lock() statement itself Good Luck!