Why does my TCP application misses some packets when sent rapidly

Question

I'm writing a multiple clients per server application in C# using async sockets. Each client on connection sends 10 items. The problem is, when starting lots of clients quickly, it appears that each client sent less than the 10 items and sometimes it sends nothing, the server just logs them connecting only.

The packet structure is the first 4 bytes are an int with size of data in them. Here's a part of server code. Each connected client has its own receive buffer which BeginReceive should write to.

private void Recieve(IAsyncResult iar) //Called when socket receives something.
{
    Socket server_conn = (Socket)iar.AsyncState;

    if (!SocketConnected(server_conn))
    {
        server_conn.Close();
        logthis("Client Disconnected");
        return;
    }
    int n = server_conn.EndReceive(iar);  //Stop Receiving and parse data, n is number of bytes received

    ClientData asdf = null;
    foreach (ClientData cls in clientlist)
    {
        if (server_conn.RemoteEndPoint == cls.clientsock.RemoteEndPoint) //Who sent this data
        {
            asdf = cls; //cls is who sent this data

            //Start a new thread and pass received bytes to it in order to be parsed
            var t = new Thread(() => parse(cls, n,cls.recvbuffer));
            t.Start();
            Thread.Sleep(100);
            break;
        }
    }

    asdf.recvbuffer = new byte[1024];      //Clear buffer of client
    server_conn.BeginReceive(asdf.recvbuffer, 0, asdf.recvbuffer.Length, SocketFlags.None, new AsyncCallback(Recieve), server_conn); //Start receiving again

}

private void parse(ClientData theclient, int nobytesreceived, byte[] bytesreceived)
{
    ClientData cls = theclient;
    int n = nobytesreceived;
    byte[] receivedbytes = bytesreceived;


    lock(s)
    {
    if (!cls.dataphase)  //If there's no fragmented packets still waiting to be read
    {
        cls.dataphase = true;   
        byte[] sizeinbytes = new byte[4];
        for (int i = 0; i < 4; i++)
        {
            sizeinbytes[i] = receivedbytes[i];
        }
        int size = BitConverter.ToInt32(sizeinbytes, 0);  //Read first four bytes of packet to get size of data

        if ((n - 4) == size)  //If received number of bytes - 4 is equals to datasize
        {
            byte[] payload = new byte[size];
            Array.Copy(receivedbytes, 4, payload, 0, (n - 4)); //Copy data to separately to payload array to be displayed to user
            logthis(cls.clientsock.RemoteEndPoint.ToString());
            logthis(Encoding.ASCII.GetString(payload));
            cls.dataphase = false;       //packet read successfully 
        }
        else if ((n - 4) < size) //If received amount of bytes is less than data size (fragmented data)
        {
            cls.data = new byte[size];
            for (int i = 4; i <= n - 4; i++)
            {
                cls.data[i - 4] += receivedbytes[i];
            }

            cls.datasize = size; //And cls.dataphase will remain true so it can be handled correctly the next time we receive something from same client
        }
        else if((n-4) > size)  //If received amount of bytes is bigger than data (lumped packets)
        {
            byte[] payload = new byte[size];
            byte[] otherpacket = new byte[(n - 4) - size];

            for(int i = 0; i < size; i++)
            {
                payload[i] += receivedbytes[i + 4];

            }

            logthis(cls.clientsock.RemoteEndPoint.ToString());
            logthis(Encoding.ASCII.GetString(payload));
            Array.Copy(receivedbytes, (size + 4), otherpacket, 0, ((n - 4) - size));
            receivedbytes = new byte[(n - 4) - size];
            receivedbytes = otherpacket;
            cls.dataphase = false;
            parse(cls, ((n - 4) - size), receivedbytes); //Send rest of packet to read again
        }

    }
    else
    {
        //not implemented, supposed to handle fragmented packets
        if (n >= cls.datasize)
        {

        }
        else if (n < cls.datasize)
        {

        }

    }

}

}

Answer 1

Your problem comes from

else
{
    //not implemented, supposed to handle fragmented packets

I would put money on the fact that you are hitting that else statement and loosing data. As soon as you don't have a complete read of a packet or two packets lumped together returned from your read function (which is a lot more common than you think) your client is now stuck in cls.dataphase = true; and will never get out of it.