How to handle client disconnect in Socket Programming?

Question

I have written a server side code using Sockets, Its working fine but have one problem I don't know how to handle this scenario: if client just closes application without sending Disconnect request, my server side program crashes. What do I need to do to avoid this? Please guide me I am new to Socket programming.

private void OnReceive(IAsyncResult result)
{
    try
    {
        Socket clientSocket = (Socket)result.AsyncState;
        clientSocket.EndReceive(result);

        command = responseMessage = string.Empty;
        command = ByteToString(receviedData);
        receviedData = new byte[30];

        if (command=="Connect")
        {
            ClientInfo clientInfo = new ClientInfo();
            clientInfo.socket = clientSocket;
            clientInfo.IP = clientSocket.RemoteEndPoint.ToString();
            connectedClients.Add(clientInfo);
            responseMessage = "Connection established...";
        }
        else if (command=="Disconnect")
        {
            for (int i = 0; i < connectedClients.Count; i++)
            {
                if (connectedClients[i].socket == clientSocket)
                {
                    connectedClients.RemoveAt(i);
                    break;
                }
            }
            clientSocket.Close();
        }
        else
        {
            responseMessage = "Error";
        }

        byte[] responseStatus = StringToByte(responseMessage);
        for (int i = 0; i < connectedClients.Count; i++)
        {
            if (connectedClients[i].socket==clientSocket)
            {
                connectedClients[i].socket.BeginSend(responseStatus, 0, responseStatus.Length,SocketFlags.None, new AsyncCallback(OnSend), connectedClients[i].socket);
                break;
            }
        }

    }
    catch(Exception ex)
    {
      throw new Exception(ex.Message);
    }
}

Answer 1

Your application crashes, because you throw an exception in the catch block of your method.

If you don't want your application to crash, you need to remove the throw new Exception(ex.Message); line from the catch block. Replace it with code that handles the error and gracefully restores your application to a safe state. From reading your code, this should be done by removing the clientSocket from connectedClients

Secondly, it is better to just use throw; instead of throw new Exception(ex.Message);. throw; will re-throw the original exception object and thus preserve the stack trace and other vital information that helps in debugging your software. Using new Exception("Message") will create a completely new exception object with the current stack trace.

private void OnReceive(IAsyncResult result)
{
    try
    {
        Socket clientSocket = (Socket)result.AsyncState;
        clientSocket.EndReceive(result);

        command = responseMessage = string.Empty;
        command = ByteToString(receviedData);
        receviedData = new byte[30];

        if (command=="Connect")
        {
            ClientInfo clientInfo = new ClientInfo() {
               socket = clientSocket,
               IP = clientSocket.RemoteEndPoint.ToString(),
               }; 
            connectedClients.Add(clientInfo);
            responseMessage = "Connection established...";
        }
        else if (command=="Disconnect")
        {
            removeClientInfo(clientSocket);
            clientSocket.Close();
        }
        else
        {
            responseMessage = "Error";
        }

        byte[] responseStatus = StringToByte(responseMessage);
        for (int i = 0; i < connectedClients.Count; i++)
        {
            if (connectedClients[i].socket==clientSocket)
            {
                connectedClients[i].socket.BeginSend(responseStatus, 0, responseStatus.Length,SocketFlags.None, new AsyncCallback(OnSend), connectedClients[i].socket);
                break;
            }
        }

    }
    catch(Exception ex)
    {
      // add error handling and gracefully recover
      // caution: The way done here, might work, but it smells :-/
      removeClientInfo((Socket)result.AsyncState);
      ((Socket)result.AsyncState).Close();
    }
}

/// removes the client info from the connectedClients enumerable
private void removeClientInfo(Socket socket)
{
    for (int i = 0; i < connectedClients.Count; i++)
    {
        if (connectedClients[i].socket == socket)
        {
            connectedClients.RemoveAt(i);
            break;
        }
    }
}

Answer 2

You are throwing a new exception inside the catch block, which doesn't make much sense unless you're doing some logging or similar. Change the catch block like:

catch(SocketException)
{ 
}

Also, you should check the number of bytes read returned from EndReceive. If you receive zero bytes, that means the client has shutdown the connection:

int numReceived = clientSocket.EndReceive(result);
if(numReceived == 0)
{
    //client has shutdown the connection
}