ConcurrentDictionary Pitfall - Are delegates factories from GetOrAdd and AddOrUpdate synchronized?

Question

The documentation of ConcurrentDictionary doesn't explicit state, so I guess we cannot expect that delegates valueFactory and updateValueFactory have their execution synchronized (from GetOrAdd() and AddOrUpdate() operations respectively).

So, I think we cannot implement use of resources inside them which need concurrent control without manually implementing our own concurrent control, maybe just using [MethodImpl(MethodImplOptions.Synchronized)] over the delegates.

Am I right? Or the fact that ConcurrentDictionary is thread-safe we can expect that calls to these delegates are automatically synchronized (thread-safe too)?

Answer 1

Yes, you are right, the user delegates are not synchronized by ConcurrentDictionary. If you need those synchronized it is your responsibility.

The MSDN itself says:

Also, although all methods of ConcurrentDictionary are thread-safe, not all methods are atomic, specifically GetOrAdd and AddOrUpdate. The user delegate that is passed to these methods is invoked outside of the dictionary's internal lock. (This is done to prevent unknown code from blocking all threads.)

See "How to: Add and Remove Items from a ConcurrentDictionary

This is because the ConcurrentDictionary has no idea what the delegate you provide will do or its performance, so if it attempted lock around them, it could really impact performance negatively and ruin the value of the ConcurrentDictionary.

Thus, it is the user's responsibility to synchronize their delegate if that is necessary. The MSDN link above actually has a good example of the guarantees it does and does not make.

Answer 2

Not only are these delegates not synchronized, but they are not even guaranteed to happen only once. They can, in fact, be executed multiple times per call to AddOrUpdate.

For example, the algorithm for AddOrUpdate looks something like this.

TValue value; do {   if (!TryGetValue(...))   {     value = addValueFactory(key);     if (!TryAddInternal(...))     {       continue;     }     return value;   }   value = updateValueFactory(key); }  while (!TryUpdate(...)) return value; 

Note two things here.

• There is no effort to synchronize execution of the delegates.
• The delegates may get executed more than once since they are invoked inside the loop.

So you need to make sure you do two things.

• Provide your own synchronization for the delegates.
• Make sure your delegates do not have any side effects that depend on the number of times they are executed.