List<T> concurrent removing and adding

Question

I am not too sure, so i thought i'd ask. Would removing and adding items to a System.Collections.Generic.List<> object be non-thread safe?

My situation:

When a connection is received, it is added to the list, but also at the same time, there's a worker that's removing dead connections and such.

Is there a problem? Will a lock do? I also want to know if i'm allowed to use a lock on the list object with it's Foreach<> method.

Solution 1

Yes, adding and removing items from a List<> is not thread safe, so you need to synchronise the access, for example using lock.

Mind that the lock keyword in no ways locks the object that you use as identifier, it only prevents two threads to enter the same code block at the same time. You will need locks around all code that accesses the list, using the same object as identifier.

Solution 2

At the time of the question there wasn't .NET Framework 4 yet, but the people who are faced the problem now should try to use collections from System.Collections.Concurrent namespace for dealing with thread-safe issues

Solution 3

List<T> is not thread-safe, so yes, you will need to control access to the list with a lock. If you have multiple threads accessing the List make sure you have them all respect the lock or you will have issues. The best way to do this would to be to subclass the List so that the locking happens automatically, else you will more than likely end up forgetting eventually.

Comments

[TheAJ]

What about removing such in List.Foreach<>, does it still require a lock?

[Guffa]

@AJ: Yes, you need a lock around all accesses to the list. If you are looping through the list and another thread removes an item, you will get an exception (or a crash in some circumstances).

[Mark Byers]

That's not the best way. It's not just because of List's implementation - also it's interface is not designed to be used by multiple threads. If you want a thread-safe way of doing this, you need a completely different interface too. The List should be an internal implementation detail, well hidden from users. blogs.msdn.com/jaredpar/archive/2009/02/11/…

[Guffa]

No, a List is never thread safe. That's just a standard text used for all regular classes. As the List class doesn't have any static members at all, there are no thread safe members.

[David_001]

This answer pretty much covers the case of "yes, you need to lock over EVERYTHING that accesses the list". Your best bet is to provide a wrapper to the list, and lock over each method in the wrapper.

[user3018301]

Synchronized was a bad idea. See blogs.msdn.com/b/jaredpar/archive/2009/02/11/…

[eran otzap]

@Guffa is the concurrent issue that the item added might not be in the index we suspected ? what if i don't care about the order of my items do i still need to lock the list ?

[Guffa]

@eranotzap: No, the concurrency issue is with the internal variables in the List object. If two threads add items at the same time, the internal variables may be corrupted because the code in the List class isn't written to take multiple threads into consideration. So, you always need to synchronise the code when multiple threads alter a list.

[eran otzap]

@Guffa what about a scenario where one thread adds and multiple threads may remove ?

[Guffa]

@eranotzap: Same there. It doesn't matter what kind of change the threads are making.

[supercat]

@Guffa: I'm pretty sure List<T> is thread-safe in the presence of multiple readers and zero writers. It's sufficiently rare for collections not to be thread-safe in the presence of multiple readers that such a level of thread-safety is assumed in the absence of documentation otherwise; nonetheless, the only collections I'd say are "never" thread-safe are things like lazy collections where two simultaneous "read" accesses may cause conflicting modifications.

[Guffa]

@supercat: You are missing the point. If you never change the list, thread safety is not an issue, but that doesn't mean that the list is thread safe "sometimes". The operations that the question is about are never thread safe.

[supercat]

@Guffa: Some types of collections may fail if two threads try to read them simultaneously, whether or not any threads are trying to write them. The List<T> collection guarantees that a read by one thread will not alter its state in any way that would interfere with a read by another state. As it happens, in present implementations a read won't alter its state at all, but a future implementation could in theory have Remove set a "deleted" flag and defer the actual removal until the next time a later item is read. An implementation which did so would then be required...

[supercat]

...to ensure that if, following modification, two simultaneous attempts were made to read the list, the deferred update performed by one would not affect the validity of data returned by the other. Note that if deferred writes didn't include safeguards to protect simultaneous reads, such a collection--unlike List<T>, might genuinely "never" be thread-safe.

[supercat]

@Guffa: The statement "It is safe to perform multiple read operations on a List(Of T)," implies a level of thread-safety in some circumstances which contradicts a claim that List<T> is never thread-safe.

[Evgeniy Berezovsky]

I upvoted because "you take it back" :) without deleting, which you could have done. But leaving it there might be insightful for others.