Problem while adding a new value to a hashtable when it is enumerated

Question

`hi

I am doing a simple synchronous socket programming,in which i employed twothreads one for accepting the client and put the socket object into a collection,other thread will loop through the collection and send message to each client through the socket object.

the problem is

1.i connect to clients to the server and start send messages

2.now i want to connect a new client,while doing this i cant update the collection and add a new client to my hashtable.it raises an exception "collection modified .Enumeration operation may not execute"

how to add a NEW value without having problems in a hashtable.

 private void Listen()
    {
        try
        {
            //lblStatus.Text = "Server Started Listening";
            while (true)
            {
                    Socket ReceiveSock = ServerSock.Accept();
                    //keys.Clear();
                    ConnectedClients = new ListViewItem();
                    ConnectedClients.Text = ReceiveSock.RemoteEndPoint.ToString();
                    ConnectedClients.SubItems.Add("Connected");
                    ConnectedList.Items.Add(ConnectedClients);
                    ClientTable.Add(ReceiveSock.RemoteEndPoint.ToString(), ReceiveSock);
                    //foreach (System.Collections.DictionaryEntry de in ClientTable)
                    //{

                    //    keys.Add(de.Key.ToString());
                    //}
                    //ClientTab.Add(
                    //keys.Add(

            }
            //lblStatus.Text = "Client Connected Successfully.";
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message);
        }
    }

    private void btn_receive_Click(object sender, EventArgs e)
    {
        Thread receiveThread = new Thread(new ThreadStart(Receive));
        receiveThread.IsBackground = true;
        receiveThread.Start();
    }
    private void Receive()
    {
        while (true)
        {
            //lblMsg.Text = "";
            byte[] Byt = new byte[2048];
            //ReceiveSock.Receive(Byt);
            lblMsg.Text = Encoding.ASCII.GetString(Byt);
        }
    }

    private void btn_Send_Click(object sender, EventArgs e)
    {
        Thread SendThread = new Thread(new ThreadStart(SendMsg));
        SendThread.IsBackground = true;
        SendThread.Start();
    }

    private void btnlist_Click(object sender, EventArgs e)
    {
        //Thread ListThread = new Thread(new ThreadStart(Configure));
        //ListThread.IsBackground = true;
        //ListThread.Start();
    }
    private void SendMsg()
    {
        while (true)
        {
            try
            {
                foreach (object SockObj in ClientTable.Keys)
                {
                    byte[] Tosend = new byte[2048];
                    Socket s = (Socket)ClientTable[SockObj];
                    Tosend = Encoding.ASCII.GetBytes("FirstValue&" + GenerateRandom.Next(6, 10).ToString());
                    s.Send(Tosend);
                    //ReceiveSock.Send(Tosend);
                    Thread.Sleep(300);
                }

            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

    }

Answer 1

Yes. Don't do that.

The bigger problem here is unsafe multi-threading. The most basic "answer" is just to say: use a synchronization lock on the shared object. However this hides a number of important aspects (like understanding what is happening) and isn't a real solution to this problem in my mind.

Answer 2

You simply can't modify a Hashtable, Dictionary, List or anything similar while you're iterating over it - whether in the same thread or a different one. There are concurrent collections in .NET 4 which allow this, but I'm assuming you're not using .NET 4. (Out of interest, why are you still using Hashtable rather than a generic Dictionary?)

You also shouldn't be modifying a Hashtable from one thread while reading from it in another thread without any synchronization.

The simplest way to fix this is:

• Create a new readonly variable used for locking

• Obtain the lock before you add to the Hashtable:

  lock (tableLock)
  {
      ClientTable.Add(ReceiveSock.RemoteEndPoint.ToString(), ReceiveSock);
  }
  

• When you want to iterate, create a new copy of the data in the Hashtable within a lock

• Iterate over the copy instead of the original table

Do you definitely even need a Hashtable here? It looks to me like a simple List<T> or ArrayList would be okay, where each entry was either the socket or possibly a custom type containing the socket and whatever other information you need. You don't appear to be doing arbitrary lookups on the table.