Security List.Add ()

Question

I understand that in general, the list is not thread safe, however, is there something wrong with just adding elements to the list if the threads never perform any other operations in the list (for example, cross it)?

Example:

List<object> list = new List<object>(); Parallel.ForEach(transactions, tran => { list.Add(new object()); }); 

Answer 1

There are many things going on behind the scenes, including the redistribution of buffers and copy elements. This code will cause danger. Very simple, when adding to the list there are no atomic operations, at least for the “Length” property there should be updates, and the element should be placed in the right place and (if there is a separate variable), the need for the index to be updated. Multiple threads can trample each other. And if growth is required, then much more is happening. If something is written to the list, nothing should read or write to it.

In .NET 4.0, we have parallel collections that are thread safe and require no locks.

Answer 2

The current approach is not thread safe - I would suggest avoiding this at all - since you mostly do PLINQ data conversion, it might be a better approach (I know this is a simplified example, but at the end you project each transaction into a different "state" object).

 List<object> list = transactions.AsParallel() .Select( tran => new object()) .ToList(); 

Answer 3

This is not an unreasonable thing. There are times when methods that can cause thread safety problems in combination with other methods are safe if they are the only method.

However, this clearly does not apply to this case when you consider the code shown in the reflector:

 public void Add(T item) { if (this._size == this._items.Length) { this.EnsureCapacity(this._size + 1); } this._items[this._size++] = item; this._version++; } 

Even if EnsureCapacity itself was thread safe (and this certainly is not), the above code will clearly not be thread safe, given the possibility of simultaneous calls to the increment operator that causes an incorrect write.

Either block, or use a ConcurrentList, or perhaps use an unblocked queue, as a place where many threads are written to, and read from it - either directly or by filling out a list with it - after they have done their job (I ' m, assuming that multiple simultaneous writes, followed by single-threaded reading, is your template here, judging by your question, because otherwise I don’t see how the condition can be used when Add is the only method that can be used van).

Answer 4

This will cause problems, since the list will be built on top of the array and will not be thread safe, you can get an index exception outside the limits or some values ​​that override other values, depending on where the streams are. In principle, do not do this.

There are many potential problems ... Just don't. If you need a thread safe collection, use a lock or one of the System.Collections.Concurrent collections.

Answer 5

Is there something wrong just adding items to the list if the threads never perform any other operations on the list?

The short answer is yes.

Long answer: run the program below.

 using System; using System.Collections.Generic; using System.Linq; using System.Threading; class Program { readonly List<int> l = new List<int>(); const int amount = 1000; int toFinish = amount; readonly AutoResetEvent are = new AutoResetEvent(false); static void Main() { new Program().Run(); } void Run() { for (int i = 0; i < amount; i++) new Thread(AddTol).Start(i); are.WaitOne(); if (l.Count != amount || l.Distinct().Count() != amount || l.Min() < 0 || l.Max() >= amount) throw new Exception("omg corrupted data"); Console.WriteLine("All good"); Console.ReadKey(); } void AddTol(object o) { // uncomment to fix // lock (l) l.Add((int)o); int i = Interlocked.Decrement(ref toFinish); if (i == 0) are.Set(); } } 

Answer 6

As others have said, you can use parallel collections from the System.Collections.Concurrent namespace. If you can use one of them, this is preferable.

But if you really need a list that is just synchronized, you can look at the SynchronizedCollection<T> -Class in System.Collections.Generic .

Please note that you need to enable the System.ServiceModel assembly, which is also the reason I don't like this. But sometimes I use it.

Answer 7

If you want to use List.add from multiple threads and don't care about ordering, then you probably don't need the List indexing ability anyway, and instead use some of the available parallel collections.

If you ignore this advice and run add , you can make the add stream thread-safe, but in an unpredictable order:

 private Object someListLock = new Object(); // only once ... lock (someListLock) { someList.Add(item); } 

If you accept this unpredictable order, most likely you, as mentioned earlier, do not need a collection that can index, as in someList[i] .

Answer 8

Even adding items to different threads is not thread safe.

C # 4.0 has parallel collections (see http://jiezhu0815.blogspot.com/2010/08/c-40-feature-1-concurrent-collections.html ).