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

See http://stackoverflow.com/questions/4779165/parallel-foreach-loop-odd-behavior. — mellamokb, Feb 16 '11 at 18:26
@Martinho: Yes. I read that List 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. — stackoverflowuser, Feb 16 '11 at 18:41
@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). — R. Martinho Fernandes, Feb 16 '11 at 18:45
Does this answer your question? [Thread-safe List property](https://stackoverflow.com/questions/5874317/thread-safe-listt-property) — Michael Freidgeim, Apr 17 '23 at 06:34

score 36 · Accepted Answer · answered Feb 16 '11 at 18:25

36

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.

answered Feb 16 '11 at 18:25

Andrey

59,039
12
119
163

to be complete why: See http://msdn.microsoft.com/en-us/library/6sh2ey19.aspx near the end it has an topic on Thread Safety – rene Feb 16 '11 at 18:28
@rene: tack `#c9721fa0-1cd9-4a21-818c-98d164c9fc14` to the end of that address and it points directly to the relevant section ;) – R. Martinho Fernandes Feb 16 '11 at 18:31
Thanks for the reply. Would it be better to use ConcurrentBag as mentioned here http://stackoverflow.com/questions/4779165/parallel-foreach-loop-odd-behavior – stackoverflowuser Feb 16 '11 at 18:47
Can you pls. provide your inputs as to whether AsParallel would be better than using ConcurrentBag ? – stackoverflowuser Feb 16 '11 at 19:12
1

@stackoverflowuser yes, when you really need concurrent access to data structure you should use `ConcurrentBag`. but in reality with such high level library as PLINQ it is rarely needed. – Andrey Feb 16 '11 at 19:12

score 8 · Answer 2 · edited May 23 '17 at 12:17

8

Use:

var processed = new ConcurrentBag<Guid>();

See parallel foreach loop - odd behavior.

edited May 23 '17 at 12:17

Community

1
1

answered Feb 16 '11 at 18:27

mellamokb

56,094
12
110
136

1

The `ConcurrentQueue` [is preferable](https://stackoverflow.com/questions/15400133/when-to-use-blockingcollection-and-when-concurrentbag-instead-of-listt/64823123#64823123) to the `ConcurrentBag`. – Theodor Zoulias Jul 28 '21 at 17:06

score 5 · Answer 3 · edited Feb 06 '20 at 20:41

5

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

edited Feb 06 '20 at 20:41

KyleMit

30,350
66
462
664

answered Feb 16 '11 at 18:27

DOK

32,337
7
60
92

Agreed. See my answer, which uses a type from the System.Collections.Concurrent namespace. – mellamokb Feb 16 '11 at 18:28

score 1 · Answer 4 · edited May 23 '17 at 12:10

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.

score 1 · Answer 5 · answered Jan 24 '16 at 17:07

1

Using ConcurrentBag of type Something

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

answered Jan 24 '16 at 17:07

JWP

6,672
3
50
74

should be `var bag = new ConcurrentBag();` – Artemious Jun 14 '21 at 06:12

score 0 · Answer 6 · answered Feb 16 '11 at 18:39

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.

List thread safety

6 Answers6

Linked

Related