0

I do have a problem with a multithreaded TCPClient application, every Client object has a Thread that recievies and sends messages and a thread that handle the Tasks that should be handled (depending on the messages)... (for example creates and answer that the msg threads sends). But something is going wrong... the application almost allways uses 100% cpu (if any thread have a task, and thats most of the time). I also have a feeling that some threads get prioritised less then others (can see in some loggs that an operation takes longer in thread 1 then in thread 2 for exampel... Is there any nice way to handel this problem?

I would love some help or some hints! Anything unclear just ask :)

Thanks! /Nick

//Waiting for TCP-connections and creating them as they arrive. Starts a Thread that handles the messages recieved and sent with this thread.
private void ListenForClients()
{
    try
    {

        this.tcpListener.Start();
        while (true)
        {
            TcpClient client = this.tcpListener.AcceptTcpClient();

            Connection c = new Connection(this.parent);
            connectionCollection.Add(c);
            Thread clientThread = new Thread(new ParameterizedThreadStart(c.HandleClientComm));

            threadCollection.Add(clientThread);
            clientThread.Start(client);
        }
    }
    catch (Exception e)
    {
    }
}
//Connection constructor, creates a ToDo-thread, this handles the messages (DB, Filewriting etc.) recieived and    creats new ones to be sent.
public Connection()
{
    this.todo = new ArrayList();
    todoT = new Thread(handleToDo);
    todoT.Start();
}
//Messagehandeling-thread
 public void HandleClientComm(object client)
{
    try
    {

        TcpClient server = (TcpClient)client;

        NetworkStream ns = server.GetStream();
        byte[] data = new byte[1024];
        string input, stringData;
        online = true;
        DateTime lastTime = DateTime.Now;

        while (true && this.online)
        {
            try
            {
                if (lastTime.AddMinutes(2) < DateTime.Now)
                    break;

                data = new byte[1024];
                if (ns.DataAvailable && ns.CanRead)
                {
                    int recv = ns.Read(data, 0, data.Length);
                    if (recv > 0)
                    {
                        lastTime = DateTime.Now;
                        if ((byte)data[recv - 1] == (byte)255)
                        {
                            int cnt = -1;
                            for (int i = 0; i < recv; i++)
                            {
                                if (data[i] == (byte)254)
                                {
                                    cnt = i;
                                    break;
                                }
                            }

                            int nr = recv - cnt - 2;
                            byte[] tmp = new byte[nr];

                            for (int i = 0; i < nr; i++)
                            {
                                tmp[i] = data[cnt + i + 1];
                            }
                            string crc = Encoding.UTF8.GetString(tmp);
                            stringData = Encoding.UTF8.GetString(data, 0, cnt);

                            MsgStruct msgs = new MsgStruct(stringData);
                            msgs.setCrc(crc);

                            addTodo(msgs);
                            if (msgs.getMsg()[0] == 'T' && this.type == 1)
                                this.parent.cStructHandler.sendAck(msgs, this.ID);
                            Console.WriteLine(todo.Count);

                        }
                    }
                }
                if (parent.cStructHandler.gotMsg(this.ID))
                {
                    MsgStruct tmpCs = parent.cStructHandler.getNextMsg(this.ID);

                    if (tmpCs.getMsg().Length != 0 && ns.CanWrite)
                    {
                        byte[] ba = Encoding.UTF8.GetBytes(tmpCs.getMsg());

                        if (tmpCs.getCrc() == "")
                        {
                            ulong tmp = CRC.calc_crc(ba, ba.Length);
                            tmpCs.setCrc(tmp.ToString("X"));
                        }

                        if (tmpCs.canSendByTimeout())
                        {
                            string crcStr = "?" + tmpCs.getCrc() + "?";
                            byte[] bb = Encoding.UTF8.GetBytes(crcStr);
                            crcStr = Encoding.UTF8.GetString(bb);
                            byte[] fullMsg = new byte[ba.Length + bb.Length];
                            bb[0] = 254;
                            bb[bb.Length - 1] = 255;

                            ba.CopyTo(fullMsg, 0);
                            bb.CopyTo(fullMsg, ba.Length);
                            string s = System.Text.UTF8Encoding.ASCII.GetString(fullMsg);

                            ns.Write(fullMsg, 0, fullMsg.Length);
                            if (!tmpCs.isAckNeeded())
                                parent.cStructHandler.removeNextMsg(this.ID);
                        }
                    }
                }
            }
            catch (Exception e)
            {
                break;
            }

        }
        ns.Close();
        server.Close();
        dead = true;
    }
    catch (Exception e)
    {
        dead = true;
    }
}

//Todo-thread

public void handleToDo()
{
    try
    {
        int cnt = 0;
        while (true)
        {
            if (todo.Count > 0)
            {
                 //SWITCH CASE FOR DIFFERENT MESSAGE TYPES, DOING TASKS DEPENDING ON WHAT ONES...
            } 
            else
            {
                if (dead)
                {
                    todoT.Abort();
                    todoT = null;
                    break;
                }
            }
       }
  }
}
Nick3
  • 639
  • 1
  • 14
  • 40

1 Answers1

1

Stop checking if data is available etc. and just let the read() block. That's how it's supposed to work!

If you want to write stuff to the socket, do it from another thread, (direct from parent thingy?), or change your design to use async reads/writes.

Looping around, polling stuff, sleep() etc. is just a waste of CPU and/or adding avoidable latency to your app.

Martin James
  • 24,453
  • 3
  • 36
  • 60