C# async Begin Receive Call back method misses some packets

Question

I send 2 successive packets from my client to the server who is listening using BeginReceive

The server always receives the first packet but not the other EXCEPT if I run the client in debug mode and slowly send the next packet after the other.

Here's a snippet from my sending function

if (soc.Connected)
{
    byte[] byData = new byte[System.Text.Encoding.ASCII.GetByteCount("Hi")];
    byData = System.Text.Encoding.ASCII.GetBytes("Hi");
    soc.Send(BitConverter.GetBytes(byData.Length));
    soc.Send(byData);
}

And here's is my call back function located inside of my server:

 private void Recieve(IAsyncResult iar)
 {
    int j = 0;

    Socket server_conn = (Socket)iar.AsyncState;
    server_conn.EndReceive(iar);

    if (!SocketConnected(server_conn))
    {   
        server_conn.Close();
        return;
    }

    if (g_bmsg.Length != 0)
    {
        logthis(server_conn.RemoteEndPoint.ToString() + ": " + Encoding.ASCII.GetString(g_bmsg, 0, g_bmsg.Length));
    }

    //Find out who sent this
    foreach (ClientData cls in clientlist)
    {
        if (server_conn.RemoteEndPoint == cls.clientsock.RemoteEndPoint)
        {
            j = cls.index;
            break;
        }
    }
     server_conn.BeginReceive(g_bmsg, 0, g_bmsg.Length, SocketFlags.None, new AsyncCallback(Recieve), server_conn);
}

Where is `g_bmsg` defined? How does it get assigned? What happens if you receive length in the first message, and then the data in the second one? Any reasonable implementation of a comm protocol should involve a FIFO buffer and a parser which would allow messages to be split. — vgru, Dec 16 '13 at 12:30
g_bmsg is defined inside of Form1 class private byte[] g_bmsg = new byte[1024]; I didn't code my server to except to read the next message in terms in of its length because I am just testing it. — InstallGentoo, Dec 16 '13 at 12:39
And probably passed to `Socket.BeginReceive` as the buffer parameter. — vgru, Dec 16 '13 at 12:40
Yea, it get passed when first starting BeginReceive. Should I create a seperate buffer for each client or what I am missing here? — InstallGentoo, Dec 16 '13 at 12:42
There's no guarantee of a one-to-one correspondence between `Send` and `Receive` calls. Since you seem to be sending small amounts of data, it's highly likely that the first `Receive` is filling the buffer with both messages that were sent separately. — Damien_The_Unbeliever, Dec 16 '13 at 13:06

score 1 · Accepted Answer · answered Dec 16 '13 at 12:47

1

When working with TCP/IP sockets (or pretty much any communication layer), you rarely have a guarantee that the entire message you intended to send will come in a single packet.

This means that you should always keep a FIFO buffer of your own, and parse it sequentially.

This is the general idea:

// fifo queue (for background thread parsing, make sure it's thread safe)
private readonly ConcurrentQueue<byte> _commQueue = new ConcurrentQueue<byte>();

In your Receive method, you should simply enqueue the data to the FIFO:

private void Receive(IAsyncResult iar)
{
    Socket server_conn = (Socket)iar.AsyncState;

    // check how many bytes we actually received
    var numBytesReceived = server_conn.EndReceive(iar);

    if (!SocketConnected(server_conn))
    {   
        server_conn.Close();
        return;
    }

    // get the received data from the buffer
    if (numBytesReceived > 0)
    {
        for (int i = 0; i < numBytesReceived; i++)
            _commQueue.Enqueue(g_bmsg[i]);

        // signal the parser to continue parsing        
        NotifyNewDataReceived();
    }

    // continue receiving
    server_conn.BeginReceive(g_bmsg, 0, g_bmsg.Length, SocketFlags.None, new AsyncCallback(Recieve), server_conn);
}

answered Dec 16 '13 at 12:47

vgru

49,838
16
120
201

Thanks a lot. From further reading, both sends translated into a single receive containing both length and the data. It wasn't just some timing issue as I was thinking. The problem is, I have to figure out a way to discover this and separate length data from the actual data. – InstallGentoo Dec 16 '13 at 13:08
1

You can either have a separate thread for parsing which will be awoken using `NotifyNewDataReceived` (e.g. through a `AutoResetEvent` or something, check [this thread](http://stackoverflow.com/a/4143523/69809) for an example of a multithread producer/consumer), or do it immediatelly inside the `Receive` method (if it's not very intensive and won't block the socket for too long). It boils down to something like: While the queue is not empty: 1. read the length, 2. check if you have enough data. 2.a. if yes, dequeue it and process it, 2.b if not, wait for the rest. – vgru Dec 16 '13 at 13:29
1

@InstallGentoo: I would nevertheless suggest you extend your protocol with a "cookie" (i.e. a known sequence of letters or numbers) and potentially even a CRC at the end, i.e. `MSG XXXX (data) CRC`, where `MSG` is your cookie, `XXXX` is length (32-bit according to your code), and `CRC` is a checksum of some sort. This way you know that your message is complete and received correctly iff it starts with a cookie and has a correct crc at the end. If something was received partially, then you can skip bytes until you get to the next cookie. – vgru Dec 16 '13 at 13:32
What's the use of the cookie, TCP packets should arrive in the order they were sent, either fragmented or lumped together but not scrambled correct? And doesn't TCP has already hash checking implemented in kernel or something? – InstallGentoo Dec 16 '13 at 15:13
1

You wrote that you have to figure out a way to separate the length info from the actual data. You can simply take the first 4 bytes like you're doing now, but a slightest bug in your code will prevent your receiver from recovering ever again. True, TCP/IP will never pass corrupted or out of order messages, but your client can easily crash in the middle of transmission. **Why risk?** This would also allow a simple switch to a non-reliable protocol (UDP) at any time. And the point of the cookie is to allow yourself to jump in the middle of the FIFO buffer and immediately see message boundaries. – vgru Dec 16 '13 at 15:40
1

Also, you get versioning for free. You changed your mind and want to use 8-bit lengths (for whatever crazy reason)? Introduce a new cookie. You want to have peer-to-peer communication over a single channel? Introduce a new cookie which supports an extended "header" (e.g. message length, sender info, recipient info, any other metadata). You are prepared for a bunch of future problems. Our company had to extend a number of poorly defined protocols and use various hacks to ensure backward compatibility, and having a wrapper message is one approach that proved great so far. – vgru Dec 16 '13 at 15:44
1

That was actually really helpful and made me change my mind on some algorithms I was thinking to implement. I think I will implement MSG XXXX (Data) without CRC checks for the sake of simplicity now. Thanks a lot Groo. – InstallGentoo Dec 16 '13 at 16:41

C# async Begin Receive Call back method misses some packets

1 Answers1