Right approach for asynchronous TcpListener using async/await

Question

I have been thinking about what is the right way of set up a TCP server by using asynchronous programming.

Usually I would spawn a thread per incoming request, but I would like to do the most of the ThreadPool, so when connections are idle, there are no blocked threads.

First I would create the listener and start to accept clients, in this case, in a Console app:

static void Main(string[] args)
{
    CancellationTokenSource cancellation = new CancellationTokenSource();
    var endpoint = new IPEndPoint(IPAddress.Parse("127.0.0.1"), 8001);
    TcpListener server = new TcpListener(endpoint); 

    server.Start();
    var task = AcceptTcpClients(server, cancellation.Token);

    Console.ReadKey(true);
    cancellation.Cancel();
    await task;
    Console.ReadKey(true);
}

In that method, I would loop accepting incoming requests and spawning a new Task to handle the connection, so the loop can get back to accept more clients:

static async Task AcceptTcpClients(TcpListener server, CancellationToken token)
{
    while (!token.IsCancellationRequested)
    {
        var ws = await server.AcceptTcpClientAsync();

        Task.Factory.StartNew(async () =>
        {
            while (ws.IsConnected && !token.IsCancellationRequested)
            {
                String msg = await ws.ReadAsync();
                if (msg != null)
                    await ws.WriteAsync(ProcessResponse(msg));
            }
        }, token);
    }
 }

Creating new Task does not necessarily mean new thread, but is this the right way? Am I taking advantage of the ThreadPool or is there anything else I can do?

Is there any potential pitfall in this approach?

Answer 1

The await task; in your Main won't compile; you'll have to use task.Wait(); if you want to block on it.

Also, you should use Task.Run instead of Task.Factory.StartNew in asynchronous programming.

Creating new Task does not necessarily mean new thread, but is this the right way?

You certainly can start up separate tasks (using Task.Run). Though you don't have to. You could just as easily call a separate async method to handle the individual socket connections.

There are a few problems with your actual socket handling, though. The Connected property is practically useless. You should always be continuously reading from a connected socket, even while you're writing to it. Also, you should be writing "keepalive" messages or have a timeout on your reads, so that you can detect half-open situations. I maintain a TCP/IP .NET FAQ that explains these common problems.

I really, strongly recommend that people do not write TCP/IP servers or clients. There are tons of pitfalls. It would be far better to self-host WebAPI and/or SignalR, if possible.

Answer 2

In order to stop a server accept loop gracefully, I register a callback that stops listening when the cancellationToken is cancelled (cancellationToken.Register(listener.Stop);).

This will throw a SocketException on await listener.AcceptTcpClientAsync(); that is easy to capture.

There is no need for Task.Run(HandleClient()), because calling an async method returns a task that is running in parallel.

    public async Task Run(CancellationToken cancellationToken)
    {
        TcpListener listener = new TcpListener(address, port);
        listener.Start();
        cancellationToken.Register(listener.Stop);
        while (!cancellationToken.IsCancellationRequested)
        {
            try
            {
                TcpClient client = await listener.AcceptTcpClientAsync();
                var clientTask = protocol.HandleClient(client, cancellationToken)
                    .ContinueWith(antecedent => client.Dispose())
                    .ContinueWith(antecedent => logger.LogInformation("Client disposed."));
            }
            catch (SocketException) when (cancellationToken.IsCancellationRequested)
            {
                logger.LogInformation("TcpListener stopped listening because cancellation was requested.");
            }
            catch (Exception ex)
            {
                logger.LogError(new EventId(), ex, $"Error handling client: {ex.Message}");
            }
        }
    }