ConcurrentDictionary.AddOrUpdate(): Add value to list on update [duplicate]

Question

I am using a ConcurrentDictionary<String,List<String>>. I want to use the AddOrUpdate method such that if a list already exists for a key, then the value gets added to the list of values. Since the function that I supply to AddOrUpdate method needs to return a List, I think I want to do this:

public void AddValue( String key, String value)
{
    _dictionary.AddOrUpdate( key, new List<string> { value},
        ( k, oldValue ) => UpdateValueList( oldValue, value) );
}

private List<String> UpdateValueList( List<String> list, String value)
{
    if ( !list.Contains( value) )
    {
        list.Add( value);
    }
    return list;
}

Is this a good way of handling this scenario, or should I do something different?

Answer 1

Given you're using a ConcurrentDictionary I assume you're looking for a thread-safe solution. Your solution isn't thread-safe, since the List<T> class isn't thread-safe.

To make a thread-safe solution you need to make sure you synchronise all access to the List<T> class from within your class, and ensure you never expose the List outside your class (if you need to, you'll need to clone it).

Difficult to be more precise without knowing more about what you're trying to achieve.

UPDATE

Just to provide an example about the lack of thread-safety.

• Consider a race condition between two threads that call your AddOrUpdate method.

• ConcurrentDictionary doesn't hold a lock while running your addValueFactory and updateValueFactory methods. This is a general design principle: don't hold locks while calling out to external code, since this can cause all sorts of problems including reentrancy.

• Therefore two threads can run the UpdateValueList method concurrently using the same input list, which isn't thread-safe.

It could probably be made thread-safe if you wrap every access to the list in a lock, but I'd need to see the rest of the code for your class to be sure.