Extension methods to make ConcurrentDictionary GetOrAdd and AddOrUpdate thread safe when using valueFactory delegates

The ConcurrentDictionary in .NET 4.0 is thread safe but not all methods are atomic.

This points out that:


... 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.

Example Problem:

It is possible for the delegate method to be executed more than once for a given key.

public static readonly ConcurrentDictionary store =
    new ConcurrentDictionary();

[TestMethod]
public void UnsafeConcurrentDictionaryTest()
{

    Thread t1 = new Thread(() =>
    {
        store.GetOrAdd(0, i =>
        {
            string msg = "Hello from t1";
            Trace.WriteLine(msg);
            Thread.SpinWait(10000);
            return msg;
        });
    });

    Thread t2 = new Thread(() =>
    {
        store.GetOrAdd(0, i =>
        {
            string msg = "Hello from t2";
            Trace.WriteLine(msg);
            Thread.SpinWait(10000);
            return msg;
        });
    });

    t1.Start();
    t2.Start();
    t1.Join();
    t2.Join();
}

The result shown in the Trace window shows "Hello from t1" and "Hello from t2". This is NOT the desired behavior for most implementations that we are using and confirms the problem noted in the MSDN link above. What we want is for only one of those delegates to be executed.

Proposed Solution:

I have to use the delegate overloads of these methods which led me to investigate this matter further. I stumbled onto this post which suggests using the Lazy class to ensure the delegate is only invoked once. With that in mind I wrote the following extension methods to mask the adding of a Lazy wrapper to the value.

```
public static V GetOrAdd(this ConcurrentDictionary dictionary, T key, Func valueFactory)
where U : Lazy
{
U lazy = dictionary.GetOrAdd(key, (U)new Lazy(() => valueFactory(key)));
return lazy.Value;
}

public static V AddOrUpdate(this ConcurrentDictionary dictionary, T key, Fun

-
Allowing the caller to provide the type argument U implies that they are allowed to use a subclass of Lazy, but this will not work as your implementations always creates a new List and cast it to U. Since this means U must always be a Lazy then why not do away with the extra type argument.

public static V GetOrAdd(this ConcurrentDictionary> dictionary, T key, Func valueFactory)

-
The name of the new extension methods conflict with the names of the existing methods. For the consumer to use yours instead of the existing methods, they would need to either access via your static class or use explicit type arguments. This could lead to subtle bugs when consumers try to use it as a extension method with type inference.

ExtensionHost.GetOrAdd(safeStore, 7, (i) => i.ToString());             // uses yours
safeStore.GetOrAdd, string>(6, (i) => i.ToString()); // uses yours
safeStore.GetOrAdd(5, (i) => i.ToString());                            // uses existing

StackExchange Code Review Q#2025, answer score: 16