Threading:Lock on generic dictionary

Question

I have a generic dictionary in a multithreaded application; to implement a lock i created a property.

static object myLock=new object();
Dictionary<key,SomeRef> dict=new Dictionary<key,SomeRef>();

public Dictionary<key,SomeRef> MyDict{
      get{
        lock(myLock){
        return dict;
        }
     }
 }

Now if i write CODE#1

 MyDict.TryGetValue

or CODE#2

var result=MyDict.Values;
foreach(var item in result){
//read value into some other variable
}

so while i m runnig code 1 or 2 and at the same time if some other thread tries to do some write operation on the dictionary like ..clear dict or add new item. then, will this solution be thread safe (using a property). if not ..then is there any other ways to do this.

When i say write operation it can be take a reference of the dict through property chek key exoist or not if not create key and assign value. (thus me not using the setter in the property)

Answer 1

No, this will not be threadsafe.

The lock will only lock around getting the reference to your internal (dict) instance of the dictionary. It will not lock when the user tries to add to the dictionary, or read from the dictionary.

If you need to provide threadsafe access, I would recommend keeping the dictionary private, and make your own methods for getting/setting/adding values to/from the dictionary. This way, you can put the locks in place to protect at the granularity you need.

---

This will look something like this:

public bool TryGetValue(key thekey, out SomeRef result)
{
    lock(myLock) { return this.dict.TryGetValue(thekey, out result); }
}
public void Add(key thekey, SomeRef value)
{
    lock(myLock) { this.dict.Add(thekey, value) }
}
// etc for each method you need to implement...

The idea here is that your clients use your class directly, and your class handles the synchronization. If you expect them to iterate over the values (such as your foreach statement), you can decide whether to copy the values into a List and return that, or provide an enumerator directly (IEnumerator<SomeRef> GetValues()), etc.

Answer 2

No, this will not be safe, as the only code that's locked is the retrieval code. What you need to do is

lock(MyDict)
{
   if(MyDict.TryGetValue()...
}

and

lock(MyDict)
{
    foreach(var item in MyDict.Values) ...
}

The basic idea is to enclose your working code within the lock() block.

Answer 3

The implementation is not guaranteed to be thread safe as it is. In order to be thread safe concurrent reads/writes must all be protected by the lock. By handing out a reference to your internal dictionary, you're making it very hard to control who accesses the resource and thus you have no guarantee that the caller will use the same lock.

A good approach is to make sure whatever resources you're trying to synchronize access to is completely encapsulated in your type. That will make it much easier to understand and reason about the thread safety of the type.

Answer 4

Thread Safe Dictionary in .NET with ReaderWriterLockSlim This is a method that uses ReaderWriterLockSlim and deterministic finalization to hold and release locks.