First message sent is lost (TCP)

Thread view:

Enable EMail Alerts

Start New Thread

Thread rating:

WP - 12 Mar 2008 01:05 GMT

Hello, I'm very new at C# and I'm writing a simple card game where two
players can play a simple card game against each other. Communication
is done through a server.
I'm using the TcpClient class and I'm trying to do asynchronous reads
using TcpClient.GetStream().BeginRead().

Anyway, my problem is that the first message sent to the client is
lost. This started to happen when I changed to reading what's on the
networkstream line by line. First I read whatever bytes where there
which worked then - I got each "message" alone and in its entirety but
as soon as I started to send messages fast they got mixed up in each
other. So I changed to reading and writing line by line but now the
first message is lost. An ugly workaround is of course to send the
very first message twice, that seems to work, but I'd like to fix it
properly.

Here's the client read callback:
private void OnReadMessage(IAsyncResult ar)
{
StreamReader sr = new StreamReader(this.tcp_client.GetStream());
String line;

do
{
line = sr.ReadLine();

this.tracebox.WriteLine(line); // Displays the message
} while (line != null);

int bytes_read = this.tcp_client.GetStream().EndRead(ar);
this.tcp_client.GetStream().BeginRead(this.message, 0,
this.message_length, new AsyncCallback(this.OnReadMessage), null);
}

Here's how the server sends a message:
public void SendMessage(String outgoing_message)
{
StreamWriter sr = new StreamWriter(this.tcp_client.GetStream());

sr.WriteLine(outgoing_message);

sr.Flush();
}

Let me know if you need to see any other code.
I looked at an example in my MSDN installation and there they call a
method called WaitOne() on an object they call tcpClientConnected (but
the example doesn't show what that type that object is). The comment
says: Wait until a connection is made and processed before continuing.
Hmm, that sounds like that could be my problem but I didn't need to do
that before and when I search for WaitOne the docs point me to
documentation for an abstract class I don't know how to use...

Thanks for reading and thanks in advance for any help!

- Eric

Reply to this Message

Jon Skeet [C# MVP] - 12 Mar 2008 01:21 GMT

<snip>

> Here's the client read callback:
> private void OnReadMessage(IAsyncResult ar)
[quoted text clipped - 13 lines]
> this.message_length, new AsyncCallback(this.OnReadMessage), null);
> }

Um, hang on: you're reading data from the stream into this.message, and
*then* you're creating a StreamReader on the stream - and ignoring the
data that you've got in this.message.

You're also assuming that you'll have the whole of a line as the result
of the read, which may well not be the case.

I strongly suggest you design your protocol so that each message is
prefixed by its length, so it's easy to tell when any particular
message has finished. Don't start looking at the message until you've
got that much data.

Signature

Jon Skeet - <skeet@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

Reply to this Message

WP - 12 Mar 2008 04:15 GMT

<snip>

> You're also assuming that you'll have the whole of a line as the result
> of the read, which may well not be the case.
[quoted text clipped - 3 lines]
> message has finished. Don't start looking at the message until you've
> got that much data.

Thanks Jon for your quick reply!
I did something slightly different, let me describe it: I introduced a
special symbol (I choose $, it's not used for anything else) to
signify the end of a message. Each time my read callback is called, I
extract all full messages. I also keep track of uncompleted messages.
The code doesn't currently handle if a message isn't completed the
next time the callback is called but I put an assertion for that case
for now and it hasn't triggered (shouldn't trigger very likely).
I had some problems first with message corruption that was very weird,
for example if I put a Trace.WriteLine() at sender it got worse on the
receiving end, but after making sure I send and receive a fixed-size
byte array and clear the receiving array each time those problems
disapperared.
Here's the read callback now, it's not very efficient, but at least it
seems to work now and I can continue implementing the actual card
game, heh:

private void OnReadMessage(IAsyncResult ar)
{
int total_bytes_read = this.tcp_client.GetStream().EndRead(ar);
String input = Encoding.ASCII.GetString(this.message, 0,
message.Length).Trim('\0');

int p1 = 0, p2 = 0;

while (p2 != -1)
{
p2 = input.IndexOf('$', p1);

if (p2 == -1)
{
Debug.Assert(being_built == false);
being_built = true;
semi_complete = input.Substring(p1);
}
else
{
String msg;
if (being_built)
{
msg = semi_complete + input.Substring(p1, p2 - p1 + 1);
being_built = false;
semi_complete = "";
}
else
{
msg = input.Substring(p1, p2 - p1 + 1);
}

p1 = p2 + 1;

tracebox.WriteLine(msg);
}
}

this.message = new byte[this.message_length]; // Important that we
clear this before each call.
this.tcp_client.GetStream().BeginRead(this.message, 0,
this.message_length, new AsyncCallback(this.OnReadMessage), null);
}

My next problem I need to tackle is the exceptions (IOExceptions
mostly I think) I get after closing the server or the client after
they have started communicating with each other. Any tips on that?

- Eric

> --
> Jon Skeet - <sk...@pobox.com>http://www.pobox.com/~skeet Blog:http://www.msmvps.com/jon.skeet
> World class .NET training in the UK:http://iterativetraining.co.uk

Reply to this Message

Jon Skeet [C# MVP] - 12 Mar 2008 09:36 GMT

> > I strongly suggest you design your protocol so that each message is
> > prefixed by its length, so it's easy to tell when any particular
[quoted text clipped - 5 lines]
> special symbol (I choose $, it's not used for anything else) to
> signify the end of a message.

Personally I prefer the length-prefix method. It can be pain if you
want to start sending a message before you know how long it is, but it
makes it much easier to make sure you've got all the data before you
start trying to do anything with it.

> Each time my read callback is called, I
> extract all full messages. I also keep track of uncompleted messages.
[quoted text clipped - 6 lines]
> byte array and clear the receiving array each time those problems
> disapperared.

Don't forget that just because you send and receive a certain size of
array doesn't mean you'll actually get that much data.

> Here's the read callback now, it's not very efficient, but at least it
> seems to work now and I can continue implementing the actual card
[quoted text clipped - 5 lines]
> String input = Encoding.ASCII.GetString(this.message, 0,
> message.Length).Trim('\0');

No - only decode the number of bytes you've read.

<snip>

> My next problem I need to tackle is the exceptions (IOExceptions
> mostly I think) I get after closing the server or the client after
> they have started communicating with each other. Any tips on that?

I would hope that EndRead would return 0 - that's how you should detect
it.

Signature

Jon Skeet - <skeet@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
World class .NET training in the UK: http://iterativetraining.co.uk

Reply to this Message

Rate this thread:

First message sent is lost (TCP)

Free Magazines