Simple TCP server using TPL

Question

I have a working C# code of a TCP server that runs inside a windows service. It accepts incoming connections and starts sending/receiving data based on some complex rules.

The top-level class for this server looks as follows:

public async void StartListen()
{
 var endpoint = new IPEndPoint(IPAddress.Parse(_host), _port);
 _serverSocket = new Socket(endpoint.Address.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
 _serverSocket.Bind(endpoint);
 _serverSocket.Listen((int)SocketOptionName.MaxConnections);
 while (!_shutdown)
 {
 var socket = await AcceptClient();
 ProcessTransaction(socket);
 }
}
private async Task<Socket> AcceptClient()
{
 var socket = await Task<Socket>.Factory.FromAsync(_serverSocket.BeginAccept, _serverSocket.EndAccept, null);
 return socket;
}
private void ProcessTransaction(Socket socket)
{
 Task.Factory.StartNew(() => ProcessTransactionAsync(socket));
}
private void ProcessTransactionAsync(Socket socket)
{
 // complex logic involving socket.Send, socket.Receive, serialization, etc...
}

The server is started by calling StartListen, and then it should run indefinitely (as long as the Windows service it is hosted in is up and running).

This code works, but I have certain doubts about it. I don't have much experience with TPL, but, for example, I know that async void method is an anti-pattern (as stated in many TPL tutorials). What problems can it cause? Are there any other flaws in this code? This server is supposed to handle high load somewhere in the future, so its speed and efficiency is also a concern.

• Ron Beyer: This MSDN Article by Stephen Cleary (a member here) pretty well tells why you shouldn't use async void. I would change it to async Task if you don't have a result to return.

Answer 1

private void ProcessTransactionAsync(Socket socket)

You've mixed the naming conventions a little bit. We add the Async suffix to methods that are awaitable. This one isn't. You could however easily fix it.

---

private void ProcessTransaction(Socket socket)
{
 Task.Factory.StartNew(() => ProcessTransactionAsync(socket));
}

Rename it and mark it propertly with async. Let its return value be Task. In place of the Task.Factory.StartNew you can use the shortcut Task.Run(someAction):

private async Task ProcessTransactionAsync(Socket socket)
{
 await Task.Run(() => ProcessTransaction(socket));
}

now you can await it in StartListen:

while (!_shutdown)
{
 var socket = await AcceptClient();
 await ProcessTransaction(socket);
}

finally as Ron Beyer wrote you should change the return value of the StartListen method to Task.

• Ahmadali Shafiee: wouldn't awaiting ProcessTransaction stop the while from continuing and prevent the app from processing new connections?