List<T> thread safety

Question

I am using the below code

var processed = new List<Guid>();
Parallel.ForEach(items, item => 
{
    processed.Add(SomeProcessingFunc(item));
});

Is the above code thread safe? Is there a chance of processed list getting corrupted? Or should i use a lock before adding?

var processed = new List<Guid>();
Parallel.ForEach(items, item => 
{
    lock(items.SyncRoot)
        processed.Add(SomeProcessingFunc(item));
});

thanks.

Roughcast

See #4779665. – Vilmavim

@Martinho: Yes. I read that List<T> is not thread safe. But I am unable to understand that even if multiple threads are adding to the list how can that corrupt the list. – Roughcast

@stackoverflowuser: an example: the list keeps track of how many elements it has. When you add one, it places it in the next position, and increment the count. Well, there's a race condition right there: two threads could both add an element, and count increase only by one (and one element getting lost in the process). – Koller

Does this answer your question? Thread-safe List<T> property – Daliladalis

Answer 1

No! It is not safe at all, because processed.Add is not. You can do following:

items.AsParallel().Select(item => SomeProcessingFunc(item)).ToList();

Keep in mind that Parallel.ForEach was created mostly for imperative operations for each element of sequence. What you do is map: project each value of sequence. That is what Select was created for. AsParallel scales it across threads in most efficient manner.

This code works correctly:

var processed = new List<Guid>();
Parallel.ForEach(items, item => 
{
    lock(items.SyncRoot)
        processed.Add(SomeProcessingFunc(item));
});

but makes no sense in terms of multithreading. locking at each iteration forces totally sequential execution, bunch of threads will be waiting for single thread.

Nescience

to be complete why: See msdn.microsoft.com/en-us/library/6sh2ey19.aspx near the end it has an topic on Thread Safety – Cosmetician

@rene: tack #c9721fa0-1cd9-4a21-818c-98d164c9fc14 to the end of that address and it points directly to the relevant section ;) – Koller

Thanks for the reply. Would it be better to use ConcurrentBag<T> as mentioned here #4779665 – Roughcast

Can you pls. provide your inputs as to whether AsParallel would be better than using ConcurrentBag<T> ? – Roughcast

@Roughcast yes, when you really need concurrent access to data structure you should use ConcurrentBag<T>. but in reality with such high level library as PLINQ it is rarely needed. – Nescience

Answer 2

Use:

var processed = new ConcurrentBag<Guid>();

See parallel foreach loop - odd behavior.

Vilmavim

The ConcurrentQueue<T> is preferable to the ConcurrentBag<T>. – Codicil

Answer 3

From Jon Skeet's Book C# in Depth:

As part of Parallel Extensions in .Net 4, there are several new collections in a new System.Collections.Concurrent namespace. These are designed to be safe in the face of concurrent operations from multiple threads, with relatively little locking.

These include:

• IProducerConsumerCollection<T>
• BlockingCollection<T>
• ConcurrentBag<T>
• ConcurrentQueue<T>
• ConcurrentStack<T>
• ConcurrentDictionary<TKey, TValue>
• and others

Richly

Agreed. See my answer, which uses a type from the System.Collections.Concurrent namespace. – Vilmavim

Answer 4

As alternative to the answer of Andrey:

items.AsParallel().Select(item => SomeProcessingFunc(item)).ToList();

You could also write

items.AsParallel().ForAll(item => SomeProcessingFunc(item));

This makes the query that is behind it even more efficient because no merge is required, MSDN. Make sure the SomeProcessingFunc function is thread-safe. And I think, but didn't test it, that you still need a lock if the list can be modified in an other thread (adding or removing) elements.

Coleorhiza

Answer 5

Using ConcurrentBag of type Something

var bag = new ConcurrentBag<List<Something>>;
var items = GetAllItemsINeed();
Parallel.For(items,i =>                          
   {
      bag.Add(i.DoSomethingInEachI());
   });

Circumnavigate

should be var bag = new ConcurrentBag<Something>(); – Ken

Answer 6

reading is thread safe, but adding is not. You need a reader/writer lock setup as adding may cause the internal array to resize which would mess up a concurrent read.

If you can guarantee the array won't resize on add, you may be safe to add while reading, but don't quote me on that.

But really, a list is just an interface to an array.

Addend