views:

86

answers:

5

I have a List and two threads that using the List.

The first thread is getting new connections, each new connection is added to the List.

The second thread looping over the List to handle the connections (using foreach).

The problem is that sometimes while the second thread is looping over the List, the List changes before the loop ends. I even tried creating a new copy of the list and looping over it. But it produces some other problems.

I don't want to create a new thread to handle each new connection, because I understood that too many threads can hurt performance. Is there any other way to handle the connections?

A: 

I really don't know much about this, but the first idea that came to my mind was:

Store the length of the list and make the loop go

while (var i < lengthoflist)
{
//whatever fancy stuff your code does
i++;
}

But I know nothing about sockets, that was just a generic idea I thought up, so I don't know if that will work.

meman32
It won't work (safely). It probably won't throw, but it might skip over items or stuff like that.
Jouke van der Maas
+1  A: 

The easiest solution to this is to use a lock on the list in each thread.

First thread:

lock(yourList)
{
    yourList.Add(...);
}

Second thread:

lock(yourList)
{
    foreach(var item in yourList)
    {
        ...
    }
}

This will prevent the first thread from being to add a connection while the second thread is in the middle of iterating over it. If the second thread is iterating over the list and the first tries to enter the lock-ed section of code, it will wait until the second thread finishes looping over the list.

Adam Robinson
@Jouke: Yes, which is what I stated at the end of the answer. This is what it sounded like the OP was asking for.
Adam Robinson
+2  A: 

2 problems.

1) You need locking around the list. You need to ensure you have mutual exclusive access to the list, so it can't be enumerated over while it's modified. A first solution is to use locks:

class Mailbox {
    List<int> list;

    void Send(int a) {
         lock(list) {
              list.Add(a);
         }
     }

     int Receive() {
         lock(list) {
             // Enumerate
             return ...;
         }
      }
}

More elegantly, you could use one of the new collections in the Concurrent namespace, like BlockingCollection. The latter in not enumeration-safe, but provides a Take() method that can be used to retrieve objects from it, while producers are inserting them.

2) Avoid creating a gazillion threads. You could use the .NET thread pool to enqueue as many request as you like, and the framework will take care of mapping them onto actual threads, without killing the system.

Mau
`BlockingCollection` only synchronizes *modifications* to the list. Iteration still requires an explicit lock.
Adam Robinson
Yes, you are right, enumeration must be eliminated and one can use `Take()` instead.
Mau
+1  A: 

As others have written, you'll need to use locking to manage the concurrency with these two threads.

As far as avoiding multiple threads, I think it largely depends on how many threads we're talking about. If you have just a few socket connections that you're working with, then maybe doing synchronous reads from each in its own thread is okay. But if you're talking about a lot of socket connections, then I wouldn't use a dedicated thread for each one.

By far the most efficient way to read from multiple sockets is to use an asynchronous read (Socket.BeginReceive()). Under the hood, these async methods use I/O Completion Ports, which are highly efficient. It's relatively easy to implement custom TCP servers that can handle many thousands of concurrent connections using the async Socket methods.

Jack Leitch
+1  A: 

The best choice here is to use a BlockingCollection. It's part of the System.Collections.Concurrent namespace and is thread-safe. It also has some handy methods useful in a producer/ consumer scenario, like the one you have described here.

BleuM937
I disagree. For someone who is not used to multithreading issues, recommending a collection that is "thread safe" (which is inaccurate; it simply synchronizes *modifications* to the list, not all access) is dangerous.
Adam Robinson