Theading with TCPListner Optimization

maher khalil

Hi All i'm working on gps tracking system now i'm working on receiving data from gps devices throw tcp port using tcpListner class then arrange the data then save it to sql server i have created a console application to do so then convert it to windows service using NSSM it is working as expected but after about 30 minutes it consumes all the server memory and CPU it is only 4 devices connected (Test Phase) The Code

using System;
using System.Threading;
using System.Net.Sockets;
using System.Text;
using System.Text.RegularExpressions;
using System.Diagnostics;
using System.Data.SqlTypes;
using System.Data.SqlClient;
using System.Data;
using Microsoft.SqlServer.Types;
using System.Linq;

namespace ConsoleApplication1
{
class Program
{
static void Main(string[] args)
{
TcpListener serverSocket = new TcpListener(8889);
TcpClient clientSocket = default(TcpClient);
int counter = 0;

        serverSocket.Start();
        //  Console.WriteLine(" >> " + "Server Started");

        counter = 0;
        while (true)
        {
            counter += 1;
            clientSocket = serverSocket.AcceptTcpClient();
            //  Console.WriteLine(" >> " + "Client No:" + Convert.ToString(counter) + " started!");
            handleClinet client = new handleClinet();
            client.startClient(clientSocket, Convert.ToString(counter));
        }

        clientSocket.Close();
        serverSocket.Stop();
        //   Console.WriteLine(" >> " + "exit");
        Console.ReadLine();
    }
}

//Class to handle each client request separatly
public class handleClinet
{

    static void WriteLog(string message, EventLogEntryType type)
    {

        using (EventLog eventLog = new EventLog("Application"))
        {
            eventLog.Source = "Application";
            eventLog.WriteEntry(message, type, 101, 1);
        }
    }
    static int InsideDangerArea(double Lat, double Lng)
    {

        string point = "POINT(" + Lng + "  " + Lat + ")";

        string ConnStr = "Data Source =.; Initial Catalog = GPS\_Tracking;Integrated Security = True";
        using (SqlConnection conn = new SqlConnection(ConnStr))
        {
            conn.Open();
            using (SqlCommand comm = new SqlCommand

Lost User

There's a bit much here to look at so I haven't completely gone through the logic, but it seems that you're making clients on new threads and those clients never kill themselves so they're basically leaked.

maher khalil

i guess you are right please check that code part

while (true)
{
counter += 1;
clientSocket = serverSocket.AcceptTcpClient();

            handleClinet client = new handleClinet();
            client.startClient(clientSocket, Convert.ToString(counter));
        }

        clientSocket.Close();
        serverSocket.Stop();

it will never go to the close part

Paulo Zemek

I don't see you close the connection or the NetworkStream. So, it may be one of the issues.

maher khalil

i'm using

using (SqlConnection conn = new SqlConnection(ConnStr))
{
conn.Open();
using (SqlCommand comm = new SqlCommand("Select id from T_SafeArea", conn))
{

            }
            conn.Close();
            conn.Dispose();
        }

maher khalil

you are right about closing the network stream and the clients could you point out where in the code to do so

Paulo Zemek

When you call GetStream, do it with a using clause. Put an { And at the end of the method, the closing } This would solve the issue.

maher khalil

i tried that it got worse it hang after starting by 2 minutes

Paulo Zemek

Yeah... I didn't notice the GetStream was inside a while(true)... so you shouldn't dispose the stream at that moment. Yet, I think I found your issue. You put the try / catch inside the while (true). That means that as soon as there is the first exception, your code will be actively rerunning and rethrowing exceptions forever. So, the right thing would be to put the while(true) inside the try... actually, inside the using clause for the network stream. But if you want to test it without too many changes, simply put returns in your catch clauses and it should solve the eternal loop issue.

Ravindra P C

Hi, Avoid creating too many instances using new keyword. If you still need then dispose them once used immediately.

ravindrapc

maher khalil

sorry for late replay it didn't work still 92% CPU after 20 min with only 4 devices i put stream.close inside the catch i put clientsocket.close inside the catch the error was cannot access a disposed object system.net.sockets.socket should i use Async Asynchronous Server Socket

Paulo Zemek

It is not a matter of calling Dispose or Close in the catch. The problem is that you have an eternal loop and, inside the loop, you have a try/catch accessing an object that may be disposed. That means: You start running it OK. As soon as the client disconnects, you receive an exception, you catch it, maybe you close/dispose objects, it doesn't matter, the loop runs again, you try to access a disposed object, you get a new exception, you catch it, you dispose again (nothing changes) and you run again... infinitely. So, with a while(true) and a try/catch inside the while, you will always make the application consume 100% CPU when you have as many disconnected clients as you have CPUs (that is, on a 4-cpu machine, the CPU will go up to a total near of 100% as soon as 4 clients disconnect, as you simply don't allow the code to exit the eternal loop).