0

This is a part of chat with multi users and I want to deserialize in loop so every message I am getting for each user I have a can publish (this is the server side)

public class ServerDLL
{
    public TcpClient client { get; set; }
    public TcpListener listner { get; set; }

    public List<NetworkStream> clientStream = new List<NetworkStream>();

    public List<TcpClient> clientsList = new List<TcpClient>();

    string clientMsg;

    BinaryFormatter formatter = new BinaryFormatter();

    private object clientListLock = new object();

    public void startConnection()
    {
        Thread listnerThread = new Thread(ListnerFunc);
        listner.Start();
        listnerThread.Start();
        Thread waitForMeesage = new Thread(WaiterFunc);
        waitForMeesage.Start();
    }
    public void ListnerFunc()
    {
        while (true)
        {
            client = listner.AcceptTcpClient();
            clientStream.Add(client.GetStream());
            if (client.Connected)
            {
                lock (clientListLock)
                {
                    clientsList.Add(client);
                }
            }
        }
    }
    public void WaiterFunc()
    {
        while (true)
        {
            lock (clientListLock)
            {
                foreach (NetworkStream stream in clientStream)
                {
                    if (stream != null)
                    {
                        clientMsg = formatter.Deserialize(stream).ToString();
                    }
                }
            }
        }
    }

now the exception pops when I send the message from the client..

The ice man
  • 33
  • 1
  • 8

1 Answers1

4

First, you really should put some sort of wait in your WaiterFunc(). Spinning the CPU like that is not a good idea.

That being said, you have a cross-thread shared resource in the clientStream collection. You can't modify a collection during enumeration (which your while loop does constantly), thus throwing the exception.

You need a lock around access to this list:

private object clientListLock = new object();

public void ListnerFunc()
{
    while (true)
    {
        client = listner.AcceptTcpClient();

        lock(clientListLock)
        {
           clientStream.Add(client.GetStream());
           if (client.Connected)
           {
               clientsList.Add(client);
           }
        }
    }
}
public void WaiterFunc()
{
    while (true)
    {
        lock (clientListLock)
        {
           foreach (NetworkStream stream in clientStream)
           {
               clientMsg = formatter.Deserialize(stream).ToString();
           }
        }
    }
}
BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
  • Your suggestion is fixing the issue but it lacks the usage of threads. – Hamlet Hakobyan Apr 29 '14 at 21:27
  • @HamletHakobyan, it has just as many threads as the OP code, and I tried to explain the cross-thread access being the problem. What are you suggesting I change? – BradleyDotNET Apr 29 '14 at 21:28
  • I think it must change the OP and I have commented the OP. – Hamlet Hakobyan Apr 29 '14 at 21:29
  • what do you mean by putting wait in my WaiterFunc() as I know the while loop isnt really working it waits in the deserialize line till it get something to deserialize it isn't deserialize null over and over.... and thank you for the quick answer when you explained I understood what was my problem – The ice man Apr 29 '14 at 21:35
  • @Theiceman If "Deserialize" is waiting until it gets something, then you have a different issue. You should probably have a different listen thread for each client (instead of doing them all in the same thread) so that each client can send whenever they want. Yours will block until the client that it is on sends, and only then process the others. – BradleyDotNET Apr 29 '14 at 21:38
  • your right so i added if(stream!=null) and now when I try to send the message from the client the exception comes again =/ – The ice man Apr 29 '14 at 21:48
  • @Theiceman, could you edit your question to include the change? You likely just need to expand the scope of your lock. – BradleyDotNET Apr 29 '14 at 21:50
  • @Theiceman looking at your updated code, the lock in ListenerFunc isn't big enough. Please make it match my code and try again. Also, is it still "Cannot modify collection while enumerating"? If not, you have a different problem :) – BradleyDotNET Apr 29 '14 at 22:04