Eitan
Eitan

Reputation: 1494

TCP/IP chat program is sending me mixed messages

I'm building a simple tcp/ip chat program and I'm having difficulty sending messages separately. If for example I send two messages and both have content larger than the buffer which can hold 20 characters, 20 characters of the first message is sent and then 20 characters of the next message is sent and then the the rest of the first message and then the rest of the last message. So when I parse and concatenate the strings I get two messages, the beginning of the first message and the beginning of the second and the end of the first and end of the second, respectively. I want to know how to send a message, and queue the next messages until the first message has already been sent. As a side note I'm using asynchronous method calls and not threads.

My code:

Client:

protected virtual void Write(string mymessage)
{


               var buffer = Encoding.ASCII.GetBytes(mymessage);
               MySocket.BeginSend(buffer, 0, buffer.Length, 
SocketFlags.None,EndSendCallBack, null);

               if (OnWrite != null)
               {
                   var target = (Control) OnWrite.Target;
                   if (target != null && target.InvokeRequired)
                   {
                       target.Invoke(OnWrite, this, new EventArgs());
                   }
                   else
                   {
                       OnWrite(this, new EventArgs());
                   }
               }
        }

and the two calls that get mixed:

 client.SendMessage("CONNECT",Parser<Connect>.TextSerialize(connect));
  client.SendMessage("BUDDYLIST","");

and finally the read function (I use a number at the beginning of every message to know when the message ends surround by brackets):

private void Read(IAsyncResult ar)
        {

            string content;
            var buffer = ((byte[]) ar.AsyncState);
            int len = MySocket.EndReceive(ar);
            if (len > 0)
            {
                string cleanMessage;
                content = Encoding.ASCII.GetString(buffer, 0, len);
                if (MessageLength == 0)
                {
                    MessageLength = int.Parse(content.Substring(1, content.IndexOf("]", 1) - 1));
                    cleanMessage = content.Replace(content.Substring(0, content.IndexOf("]", 0) + 1), "");
                }
                else
                    cleanMessage = content;

                if(cleanMessage.Length <1)
                {
                    if(MySocket.Connected)
                        MySocket.BeginReceive(buffer, 0, buffer.Length, SocketFlags.None, new AsyncCallback(Read), buffer);
                    return;
                }

                MessageLength = MessageLength > cleanMessage.Length? MessageLength - cleanMessage.Length : 0;
                amessage += cleanMessage;

                if(MessageLength == 0)
                {
                    if (OnRead != null)
                    {
                        var e = new CommandEventArgs(this, amessage);
                        Control target = null;
                        if (OnRead.Target is Control)
                            target = (Control)OnRead.Target;
                        if (target != null && target.InvokeRequired)
                            target.Invoke(OnRead, this, e);
                        else
                            OnRead(this, e);
                    }
                    amessage = String.Empty;
                }
                MySocket.BeginReceive(buffer, 0, buffer.Length, SocketFlags.None, new AsyncCallback(Read), buffer);
                    return;
            }
        }

Upvotes: 0

Views: 2311

Answers (2)

Tedd Hansen
Tedd Hansen

Reputation: 12362

If you send a whole message in one send call it will arrive in sequence. You need a delimiter to separate your messages. On the receiving side you need to buffer messages into a buffer. If you are not sending large amounts of data you can allow yourself a bit sloppy, but easy coding to process incoming messages:

// Note: Untested pseudocode
buffer += stringDataRead;
while (buffer.Contains("\r\n")) {
  // You may want to compensate for \r\n by doing a -2 here and +2 on next line
  line = buffer.Substring(0, buffer.IndexOf("\r\n"));
  buffer = buffer.Remove(0, line.Length);

  DoSomehingWithThisLine(line);
}

No magic required. :)

Upvotes: 1

jgauffin
jgauffin

Reputation: 101130

TCP do not guarantee that you will receive your entire message in one read. Therefore you need to be able to detect where a message starts and where it ends.

You normally do that by adding some special characters at the end of the message. Or use a length header before the actual message.

You normally do not need to use BeginSend in a client. Send should be fast enough and will also reduce complexity. Also, I usually do not use BeginSend in servers either unless the server should be really performant.

Update

The actual socket implementation will never ever mix your messages, only your code can do that. You cannot send a message by calling multiple sends, since then your messages will be mixed if your application is multi threaded.

In other words, this will not work:

_socket.BeginSend(Encoding.ASCII.GetBytes("[" + message.Length + "]"))
_socket.BeginSend(Encoding.ASCII.GetBytes(message));

You have to send everything with one send.

Update 2

Your read implementation do not take into account that two messages can come in the same Read. It's most likely that that's the cause to your mixed messages.

If you send:

[11]Hello world
[5]Something else

They can arrive as:

[11]Hello World[5]Some
thing else

In other words, part of the second message can arrive in the first BeginRead. You should always build a buffer with all received contents (use StringBuilder) and remove the handled parts.

Pseudo code:

method OnRead
    myStringBuilder.Append(receivedData);
    do while gotPacket(myStringBuilder)
        var length = myStringBuilder.Get(2, 5)
        if (myStringBuilder.Length < 7 + length)
           break;

        var myMessage = myStringBuilder.Get(7, length);
        handle(myMessage);

        myStringBuilder.Remove(0, 7+length);
    loop
end method

Do you see what I'm doing? I'm always appending the stringbuilder with the received data and then remove complete messages. I'm using a loop since multiple messages can arrive at once.

Upvotes: 1

Related Questions