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.

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.