AddOrUpdate method in ConcurrentDictionary thread safe alternative

Question

As per this link Is AddOrUpdate thread safe in ConcurrentDictionary? from the "Remarks" section on this page https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110).aspx. it says

"However, delegates for these methods are called outside the locks to avoid the problems that can arise from executing unknown code under a lock. Therefore, the code executed by these delegates is not subject to the atomicity of the operation."

The issue is I extensively use this throughout my code and realize i have a potential very nasty bug (my understanding has been wrong all along)... since in the way I use the below I did not expect the delegates could be called at the same time and overwrite the internal dictionary:

internal class HandlerProducers
{
    readonly ConcurrentDictionary<Type, Dictionary<Type, InstanceProducer>> handlerProducers
        = new ConcurrentDictionary<Type, Dictionary<Type, InstanceProducer>>();

    public void AddProducer(Type notificationType, Type concreteType, InstanceProducer producer)
    {
        this.handlerProducers
                .AddOrUpdate(
                    notificationType,
                    (key) => new Dictionary<Type, InstanceProducer> { { concreteType, producer } },
                    (key, dictionary) => { dictionary.Add(concreteType, producer); return dictionary; });
    }

    public IEnumerable<InstanceProducer> GetForHandler(Type notificationHandlerType)
    {
        if(this.handlerProducers.TryGetValue(notificationHandlerType, out var dict))
        {
            foreach (var kvp in dict)
                yield return kvp.Value;
        }
    }
}

I have a further challenge that naively putting locks in place will potentially cause a hot spot, the above class is used extensively for reading from via GetForHandler() and is written to occasionally (via the AddProducer() method.

What would be the best way to ensure this is thread safe by design by performant with massive reads and occasional writes?

Answer 1

I suggest to use a nested ConcurrentDictionary<K,V> structure:

readonly ConcurrentDictionary<Type,
    ConcurrentDictionary<Type, InstanceProducer>> handlerProducers = new();

You could then implement the AddProducer method like this:

public void AddProducer(Type notificationType, Type concreteType,
    InstanceProducer producer)
{
    ConcurrentDictionary<Type, InstanceProducer> innerDict =
        this.handlerProducers.GetOrAdd(notificationType,
            _ => new ConcurrentDictionary<Type, InstanceProducer>());

    if (!innerDict.TryAdd(concreteType, producer))
        throw new InvalidOperationException(
            $"{notificationType}.{concreteType} already exists.");
}

The method GetForHandler needs no modification.

This implementation will make your HandlerProducers class truly thread-safe, without sacrificing its efficiency.