4
\$\begingroup\$

I wrote simple C# TCP Server application and my program use a lot of cup, i was wondering anyone can review my code and would be grateful for any hints and suggestion.

This is my code.

public class Program
    {
        static void Main(string[] args)
        {
            int _Port = Convert.ToInt32(System.Configuration.ConfigurationManager.AppSettings["TCPPort"]);
            string _IP = System.Configuration.ConfigurationManager.AppSettings["TCPIP"];
            TcpServer server = new TcpServer(_Port);
        }
    }


  public class TcpServer
    {
        private TcpListener _server;
        private Boolean _isRunning;
        private static int defaultMax = 8;
        public TcpServer(int port)
        {
            _server = new TcpListener(IPAddress.Any, port);
            _server.Start();
            _isRunning = true;          
            LoopClients();
        }



public void LoopClients()
    {
        while (_isRunning)
        {

            TcpClient newClient = _server.AcceptTcpClient();
            SentFirstTimeData_New(newClient);
            Thread t = new Thread(new ParameterizedThreadStart(HandleClient));
            t.Start(newClient);
        }
    }

    public void HandleClient(object obj)
    {

        int requestCount = 0;
        byte[] bytesFrom = new byte[209666];
        string dataFromClient = null;
        Byte[] sendBytes = null;
        requestCount = 0;
        int byteread = 0;
        string respdata = string.Empty;
        byte[] buffer = new byte[4096];
        int numberOfBytesRead;

        TcpClient client = (TcpClient)obj;         
        NetworkStream networkStream = client.GetStream();
        IPAddress address = ((IPEndPoint)client.Client.RemoteEndPoint).Address;

        Boolean bClientConnected = true;
        int id = Thread.CurrentThread.ManagedThreadId;
        while (bClientConnected)
        {

            try {
                if (networkStream.DataAvailable)
                {
                    byteread = networkStream.Read(bytesFrom, 0, (int)client.ReceiveBufferSize);
                    dataFromClient = Encoding.ASCII.GetString(bytesFrom).TrimEnd('\0');
                    System.Text.Encoding.ASCII.GetString(bytesFrom);

                    XmlDocument doc = new XmlDocument();
                    string xml = Encoding.UTF8.GetString(bytesFrom).TrimEnd('\0');
                    doc.LoadXml(xml);
                    XElement incc = XElement.Load(new XmlNodeReader(doc));
                    respdata = AysncTCPIncomeString(incc);

                    sendBytes = Encoding.ASCII.GetBytes(respdata);
                    networkStream.Write(sendBytes, 0, sendBytes.Length);
                    networkStream.Flush();

                }

            }
            catch (Exception ex)
            {
                Helper.WriteLog("Exception=" + ex.Message);
            }

        }
    }

    public void SentFirstTimeData_New(TcpClient tc)
    {
        try
        {
            int threadid = Thread.CurrentThread.ManagedThreadId;
            string Msg = HLStatusResponse.FirstTimeStatusSuccessfulResponse("STATUS", "1");
            NetworkStream _NetworkStream = tc.GetStream();
            if (_NetworkStream != null)
            {
                byte[] data = Encoding.ASCII.GetBytes(Msg);
                _NetworkStream.Write(data, 0, data.Length);

                Helper.WriteLog("ThreadId=" + threadid.ToString() + ";SentFirstTimeData :\t " + Msg);
            }

        }
        catch (Exception ex)
        {
            Helper.WriteLog("Sent Though Tcp Eror :" + ex.ToString());

        }
    }



}
\$\endgroup\$
1
\$\begingroup\$

Big picture, you probably don't want to allocate and start a new thread to handle each incoming connection. .Net has a very good ThreadPool implementation that scales up to as many threads as your machine can support without creating so many that performance suffers.

That said, it looks like your particular problem is that threads are never allowed to complete. bClientConnected is the control variable for your main while loop, and it is never set to false, even if an exception is thrown. You should always be suspicious of a catch that doesn't rethrow or at least exit the current function.

\$\endgroup\$
-3
\$\begingroup\$

Background threads tend to run at 100% CPU utilization unless you do something to prevent it. If your requirements allow for it, try adding a Thread.Sleep(10) (or some other number) to the code.

if (networkStream.DataAvailable)
{
    //...
}
else
{
    Thread.Sleep(10);
}

If you can't do that, when you start the new thread, try setting the thread priority to low or tweaking other related settings.

\$\endgroup\$
  • \$\begingroup\$ Hi Zack, Thanks for your help. Now CPU usage decrease but thread count in task manager is keep increasing. \$\endgroup\$ – Jasper Mar 4 '16 at 1:56
  • \$\begingroup\$ It looks like LoopClients() continuously makes new threads. Perhaps you should remove the while() or only make new threads if the previous thread has stopped (make the thread you create a class member) \$\endgroup\$ – Zack Mar 4 '16 at 2:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.