16 December 2012

Preventing high speed socket communication on Windows Phone 8 going south when using async/await

Currently I am working on a Windows Phone 8 action game in which you can fight a kind of duel. This involves pairing the phones via NFC and then obtaining a socket trough which the phones can exchange game information. Liking to steal from examples myself (why do you think I name my blog this way, eh?) I stole code from the Bluetooth app to app sample at MSDN. Now this is a great sample but it has one kind of problem for me.

I’ll skip the pairing, the obtaining of the socket and the definition of the data reader – that’s all documented in the MSDN sample. The method to send a message to the opponent was, in condensed form:

public async void SendMessage(string message)
{
  if (dataWriter == null)
  {
    dataWriter = new DataWriter(socket.OutputStream);
  }

  dataWriter.WriteInt32(message.Length);
  await dataWriter.StoreAsync();

  dataWriter.WriteString(message);
  await dataWriter.StoreAsync();
}

while at the same time both opponenents where listening using a simple method like

public async Task GetMessage()
{
  if (dataReader == null) dataReader = new DataReader(socket.InputStream);
  await dataReader.LoadAsync(4);
  var messageLen = (uint)dataReader.ReadInt32();
  Debug.WriteLine(messageLen);

  await dataReader.LoadAsync(messageLen);
  var message = dataReader.ReadString(messageLen);
  Debug.WriteLine(message);
  return message;
}

From the debug statements in this code you can see things weren’t exactly working as planned. Now the way this is supposed to work is as follows: as the opponent sends a message using SendMessage, the other phone is receiving a message via the socket in GetMessage. The first four bytes contain an unsigned integer containing the length of the rest of the message – which is supposed to be a string.

I noticed that while things went off to a good start, sooner or later one of the two phones would stop receiving messages or both games crashed simultaneously. I got all kind of null value errors, index out of range and whatnot. When I started debugging, I found out that while the app on phone 1 said it sent a 3-character string, the receiving app on phone 2 sometimes got a huge number for the message length, that obviously wasn’t sent, it read past the end of the stream – crash.

The MSDN sample works fine, as long as it is used for the purpose it was written, i.e. sending out (chat) messages at a kind of sedate pace – not for sending a lot of events per second to keep a game in sync. The essence of a stream is that it’s a stream indeed – things have to be written and read in the right order. For what happened of course was a race condition between two or more events in a short timeframe. The first event wrote the message length, the second event as well, then possibly a third, before the first one came to writing the actual message, and whatever arrived on the other phone was a garbled jumble of bytes that did exactly reflect what happened on the sending phone, but wasn’t the orderly message length – message payload stream the other phone expected.

The way I solved it – in a ‘there-I-fixed-it’ kind of way – was to use a lock on the write code so at least stuff went in the stream in the order the other phone was expecting it:

public async void SendMessage(string message)
{
  lock (this)
  {
    if (dataWriter == null)
    {
      dataWriter = new DataWriter(socket.OutputStream);
    }

    dataWriter.WriteInt32(message.Length);
    dataWriter.StoreAsync();

    dataWriter.WriteString(message);
    dataWriter.StoreAsync();
  }
}

Note you have to remove the “await” before the StoreAsync methods as those are not allowed within a lock. This method makes sure one message – and the whole message – is written on the stream and nothing comes in between.

Update – the first version of this posting contained a small error – which has been graciously pointed out to me by a few readers (see comments section). Also, I’ve been pointed to an article by Scott Hanselman about a similar subject. I’ve tried using the AsyncLock by Stephen Toub he describes but found out that although it works very good and my game does become a bit more fluid, the lag in getting messages across is a lot higher. Net effect: while the game runs more smoothly on both phones, the game state actually gets out of sync very fast, making the game unplayable. Apparently the AsyncLock approach doesn’t work for high speed continuous message exchange, so for now I’ll stick to the approach I described above.

7 comments:

Michael L Perry said...

lock (new Object()) won't protect against another thread trying to enter the lock. That other thread will get a different object. Instead, create one Object outside of the method (e.g. an instance field or static member) and lock on that.

Or if you want to protect a resource while still allowing async and await, check out Hanselman's comparison of two solutions to that problem.

rising-wind said...

lock (new Object())

WTF???

Joost van Schaik said...

Right. Copied the wrong bit of source code. Thanks for pointing this out. Thanks for the Hanselman link as well, I missed that

Hafiz Shafiq said...

I am working on sockets communication from windows phone 8 with local server via visual studio 2012 express. I could not found any tutorial about it. please help me about. I am a student not a professional

Joost van Schaik said...

@Hafiz: I am sorry but I don't have any (successful) experience with that. I hope one of my readers has.

SARWAN said...

Is that possible to communicate with more then one winodws phone. i.e: One WP8 act as server and other two as client. If Client quits the communication then, it will sends to server and other client. like that.?

Joost van Schaik said...

@sarwam I have no idea. Haven't tried that yet.