Lock for ConcurrentDictionary with AddOrUpdate-ing?

Question

I use ConcurrentDictioanry<string, HashSet<string>> to access some data in many threads.

I read in this article (scroll down) that the AddOrUpdate method AddOrUpdate not execute in a lock, so it can compromise threads security.

My code is as follows:

 //keys and bar are not the concern here ConcurrentDictioanry<string, HashSet<string>> foo = new ...; foreach(var key in keys) { foo.AddOrUpdate(key, new HashSet<string> { bar }, (key, val) => { val.Add(bar); return val; }); } 

Do I have to enclose the AddOrUpdate call in the lock statement to make sure everything is thread safe?

Answer 1

Locking during AddOrUpdate alone will not help - you still have to block every time you read from the set.

If you intend to treat this collection as thread safe, you really need values ​​that can also be thread safe. You need a ConcurrentSet , ideally. Now this does not exist within the framework (unless I missed something), but you could create your own ConcurrentSet<T> that used ConcurrentDictionary<T, int> (or any other TValue that you like) as your basic data structure. Basically, you ignore the meaning in the dictionary and simply consider the presence of the key as an important part.

You do not need to implement everything that is inside ISet<T> - just the bit that you really need.

Then you would create a ConcurrentDictionary<string, ConcurrentSet<string>> in your application code, and you are absent - there is no need to block.

Answer 2

You will need to fix this code, it creates a lot of garbage. You create a new HashSet, even if it is not required. Use another overload that accepts a valueFactory delegate. Thus, a HashSet is created only when the key is not already present in the dictionary.

The Factory value can be called several times if several threads simultaneously try to add the same key value, but it is not. Very low odds, but not zero. Only one of these hashes will be used. Not a problem, creating a HashSet has no side effects that can cause problems with streams, additional copies simply collect garbage.

Answer 3

The article says that the add delegate is not running in the dictionary lock and that the item you receive may not be the item created by the add delegate in this thread. This is not a thread safety issue; the state of the dictionary will be consistent, and all callers will receive the same instance, even if a different instance was created for each of them (and all but one were deleted).

Answer 4

It seems the best answer would be to use Lazy in this article about the methods that are passed in in the delegate.

Also another good article. Here in Lazy loading add a delegate.