List<T> thread safety
Solution 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. lock
ing at each iteration forces totally sequential execution, bunch of threads will be waiting for single thread.
Solution 2
Use:
var processed = new ConcurrentBag<Guid>();
See parallel foreach loop - odd behavior.
Solution 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
Solution 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.
Solution 5
Using ConcurrentBag of type Something
var bag = new ConcurrentBag<List<Something>>;
var items = GetAllItemsINeed();
Parallel.For(items,i =>
{
bag.Add(i.DoSomethingInEachI());
});
stackoverflowuser
Updated on July 19, 2022Comments
-
stackoverflowuser almost 2 years
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.
-
mellamokb about 13 yearsAgreed. See my answer, which uses a type from the System.Collections.Concurrent namespace.
-
rene about 13 yearsto be complete why: See msdn.microsoft.com/en-us/library/6sh2ey19.aspx near the end it has an topic on Thread Safety
-
R. Martinho Fernandes about 13 years@rene: tack
#c9721fa0-1cd9-4a21-818c-98d164c9fc14
to the end of that address and it points directly to the relevant section ;) -
stackoverflowuser about 13 yearsThanks for the reply. Would it be better to use ConcurrentBag<T> as mentioned here stackoverflow.com/questions/4779165/…
-
stackoverflowuser about 13 yearsCan you pls. provide your inputs as to whether AsParallel would be better than using ConcurrentBag<T> ?
-
Andrey about 13 years@stackoverflowuser 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. -
Artemious almost 3 yearsshould be
var bag = new ConcurrentBag<Something>();
-
Theodor Zoulias almost 3 years