14

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.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
TheAJ
  • 10,485
  • 11
  • 38
  • 57

5 Answers5

32

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.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • What about removing such in `List.Foreach<>`, does it still require a lock? – TheAJ Nov 28 '09 at 19:50
  • 1
    @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). – Guffa Nov 28 '09 at 19:56
  • 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. – David_001 Nov 28 '09 at 20:28
  • @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 ? – eran otzap Nov 30 '13 at 23:51
  • @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. – Guffa Dec 01 '13 at 00:16
  • @Guffa what about a scenario where one thread adds and multiple threads may remove ? – eran otzap Dec 01 '13 at 00:56
  • @eranotzap: Same there. It doesn't matter what kind of change the threads are making. – Guffa Dec 01 '13 at 01:30
4

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

tartakynov
  • 2,768
  • 3
  • 26
  • 23
2

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.

Donnie
  • 45,732
  • 10
  • 64
  • 86
  • 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. http://blogs.msdn.com/jaredpar/archive/2009/02/11/why-are-thread-safe-collections-so-hard.aspx – Mark Byers Nov 28 '09 at 19:58
0

Definitely using lock for particular code makes it thread safe, but I do not agree with it for current scenario.

You can implement method Synchronized to make collection thread safe. This link explains why and how to do that.

Another purely programmatic approach is mentioned in this link, though I never tested it firsthand but it should work.

btw, one of the bigger concern is that, are you trying to maintain something like connection pool on you own? if yes then why?

I take my answer back. Using locks in better answer that using this method.

BigBoss
  • 413
  • 8
  • 23
  • Synchronized was a bad idea. See http://blogs.msdn.com/b/jaredpar/archive/2009/02/11/why-are-thread-safe-collections-so-hard.aspx – David White Feb 24 '11 at 05:28
  • 1
    I upvoted because "you take it back" :) without deleting, which you could have done. But leaving it there might be insightful for others. – Evgeniy Berezovsky Jun 11 '15 at 23:24
-3

Actually, sometimes List<> is thread-safe, and sometimes not, according to Microsoft:

Public static members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

but that page goes on to say:

Enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with one or more write accesses, the only way to ensure thread safety is to lock the collection during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

DOK
  • 32,337
  • 7
  • 60
  • 92
  • 5
    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. – Guffa Nov 28 '09 at 20:01
  • I agree that he should definitely use a lock. Thanks for the clarification. – DOK Nov 28 '09 at 20:22
  • 1
    @Guffa: I'm pretty sure `List` 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. – supercat Dec 11 '13 at 16:34
  • @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. – Guffa Dec 11 '13 at 18:29
  • @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` 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 Dec 11 '13 at 18:47
  • ...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`, might genuinely "never" be thread-safe. – supercat Dec 11 '13 at 18:52
  • @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` is *never* thread-safe. – supercat Dec 11 '13 at 19:17