async await usage with TcpClient

Question

I recently started using the new C#5.0 "async" and "await" keywords. I thought I got the twist but realized one thing that made me doubt. Here's how I was asynchronously receiving data from a remote TcpClient. Once I accept the connection I call this function :

static async void ReadAsync(TcpClient client)
{
    NetworkStream ns = client.GetStream();
    MemoryStream ms = new MemoryStream();
    byte[] buffer = new byte[1024];

    while(client.Connected)
    {
        int bytesRead = await ns.ReadAsync(buffer, 0, buffer.Length);
        ms.Write(buffer, 0, bytesRead);
        if (!ns.DataAvailable)
        {
            HandleMessage(ms.ToArray());
            ms.Seek(0, SeekOrigin.Begin);
        }
    }
}

After data is received, the loop just goes on and on without reading anything. I tested this with a Console.WriteLine in the loop. Am I using it correctly? I feel I should not be using a while loop...

So you've written data into a `MemoryStream`, you call `HandleMessage` over and over again on that same `MemoryStream` until more data is available. What do you expect to happen? — spender, Jun 08 '14 at 22:49
Why are you checking `DataAvailable` here? That has **nothing** to do with messages etc. The primary use for that is in deciding whether to read sync or async. You also aren't ever purging the back-buffer - you rewind, but you don't purge — Marc Gravell, Jun 08 '14 at 23:01
I feel like I've been posting this a lot lately, but **Only use `async void` for event handlers. Async methods that don't return anything should be `async Task`.** There are a bunch of reasons why, probably the most important of which is that unhandled exceptions can cause very bad, unexpected behavior. — Daniel Mann, Jun 08 '14 at 23:35
@Marc are you sure about `DataAvaliable`? According to MSDN: `Use the DataAvailable property to determine if data is ready to be read. If DataAvailable is true, a call to Read returns immediately.` — Yuval Itzchakov, Jun 08 '14 at 23:35
@spender actually, everytime there is data available to read, it does so and appends it to the MemoryStream. If there is no data available after reading (packet was longer than the buffer length) it calls the HandleMessage function with the data and resets the MemoryStream. My problem is more about the use of async await, it seems that once I received data, it will get stuck inside the while loop! — Philippe Paré, Jun 08 '14 at 23:44
@MarcGravell thank you for this advice, I'll use ms.SetLength(0) to free up memory is this ok? — Philippe Paré, Jun 08 '14 at 23:45
@PhilippePare take a look at this implementation: http://stackoverflow.com/questions/21013751/what-is-the-async-await-equivalent-of-a-threadpool-server/21018042#comment37127995_21018042 — Yuval Itzchakov, Jun 09 '14 at 00:13
@Yuval yes, the MSDN quote says exactly the same thing I did — Marc Gravell, Jun 09 '14 at 06:45
@MarcGravell you said it decides whether to read sync or async, yet i see no documentation of that at all? — Yuval Itzchakov, Jun 09 '14 at 06:58
@Yuval no, I said that the **caller** can use it to decide whether to read sync or async. If it non-zero, we can reasonably use a sync read and expect a prompt return. If it is zero, we should switch to async. There aren't many other uses of it, and most times I see people touching `DataAvailable`, they are *using it inappropriately* (to try to detect the end of a message / frame - which is **not** what it means) — Marc Gravell, Jun 09 '14 at 07:26
I understand why I should not use the DataAvailable, but how will I detect if the packet is larger than my receive buffer? — Philippe Paré, Jun 10 '14 at 15:27

score 6 · Answer 1 · edited Feb 07 '17 at 05:03

The TcpClient.Connected value is not updated immediately. According to MSDN:

true if the Client socket was connected to a remote resource as of the most recent operation; otherwise, false.

So having TcpClient.Connected as a while loop condition is not a good choice.

TcpClient.DataAvailable is used for synchronous operations and do not used for asynchronous.

Update your code to:

static async void ReadAsync(TcpClient client)
{
    NetworkStream ns = client.GetStream();
    MemoryStream ms = new MemoryStream();
    byte[] buffer = new byte[1024];

    while(true) {
        int bytesRead = await ns.ReadAsync(buffer, 0, buffer.Length);
        if (bytesRead <= 0)
            break;
        ms.Write(buffer, 0, bytesRead);
        HandleMessage(ms.ToArray());
        ms.Seek(0, SeekOrigin.Begin);
    }
}

When ReadAsync return 0, it means TCP connection closing or closed. In MSDN:

The value of the TResult parameter contains the total number of bytes read into the buffer. The result value can be less than the number of bytes requested if the number of bytes currently available is less than the requested number, or it can be 0 (zero) if the end of the stream has been reached.

score 0 · Accepted Answer · answered Sep 25 '15 at 00:00

0

I ended up using length-prefixed messages, prefixing every packet by 4 bytes representing the length of the data to come.

answered Sep 25 '15 at 00:00

Philippe Paré

4,279
5
36
56

async await usage with TcpClient

2 Answers2

Linked