Thread-safe changes to a ConcurrentDictionary

Question

I am populating a ConcurrentDictionary in a Parallel.ForEach loop:

var result = new ConcurrentDictionary<int, ItemCollection>();

Parallel.ForEach(allRoutes, route => 
{
    // Some heavy operations

    lock(result)
    {
        if (!result.ContainsKey(someKey))
        {
            result[someKey] = new ItemCollection();
        }

        result[someKey].Add(newItem);
    }
}

How do I perform the last steps in a thread-safe manner without using the lock statement?

EDIT: Assume that ItemCollection is thread-safe.

Is `ItemCollection.Add` thread-safe? If not, you'll need a lock. — dcastro, Jun 12 '14 at 13:20
@ken2k: Except my situation has a conditional element to it. If the key doesn't exist, create a new `ItemCollection`, otherwise add an item to the existing `ItemCollection` at that key. — Dave New, Jun 12 '14 at 13:21
@dcastro: Ah yes, please assume that ItemCollection is thread-safe. — Dave New, Jun 12 '14 at 13:22

Jon Skeet · Accepted Answer · 2014-06-12T13:28:35.827

4

I think you want GetOrAdd, which is explicitly designed to either fetch an existing item, or add a new one if there's no entry for the given key.

var collection = result.GetOrAdd(someKey, _ => new ItemCollection());
collection.Add(newItem);

As noted in the question comments, this assumes that ItemCollection is thread-safe.

edited Jun 12 '14 at 13:28

answered Jun 12 '14 at 13:22

Jon Skeet

1,421,763
867
9,128
9,194

That may not be as safe as it appears: http://stackoverflow.com/questions/10486579/concurrentdictionary-pitfall-are-delegates-factories-from-getoradd-and-addorup – clarkitect Jun 12 '14 at 13:24
@jeffdot: I don't think that's a problem in this case - the `ItemCollection` constructor doesn't need to be called within any locks... What issue are you envisaging? – Jon Skeet Jun 12 '14 at 13:24
Because there was discussion of locking, and because the `ConcurrentDictionary` does have an internal lock, I felt we were adding risk by leading people to think that `GetOrAdd` and `AddOrUpdate` are atomic or called within the dictionary's lock. I have since noted that you are giving fair warning that `ItemCollection` must be threadsafe. – clarkitect Jun 12 '14 at 13:28

score 1 · Answer 2 · answered Jun 12 '14 at 13:23

1

You need to use the GetOrAdd method.

var result = new ConcurrentDictionary<int, ItemCollection>();

int someKey = ...;
var newItem = ...;

ItemCollection collection = result.GetOrAdd(someKey, _ => new ItemCollection());
collection.Add(newItem);

answered Jun 12 '14 at 13:23

Sam Harwell

97,721
20
209
280

score 1 · Answer 3 · answered Jun 12 '14 at 13:23

1

Assuming ItemCollection.Add is not thread-safe, you will need a lock, but you can reduce the size of the critical region.

var collection = result.GetOrAdd(someKey, k => new ItemCollection());

lock(collection)
    collection.Add(...);

Update: Since it seems to be thread-safe, you don't need the lock at all

var collection = result.GetOrAdd(someKey, k => new ItemCollection());
collection.Add(...);

answered Jun 12 '14 at 13:23

dcastro

66,540
21
145
155

The OP replied to your comment saying that ItemCollection *is* thread-safe. – Jon Skeet Jun 12 '14 at 13:25
@JonSkeet Thanks, hadn't noticed. – dcastro Jun 12 '14 at 13:27

Thread-safe changes to a ConcurrentDictionary

3 Answers3