Performant C# Socket server

Question

I am creating a performant C# socket server. I'd like it to be able to accept as many simultaneous connections as possible. I've tested with 100 clients and all clients are fine. With 1000 however, clients begin to stop connecting. Is it possible to improve something to increase the number of simultaneous connections?

Of course, general code review would also be appreciated.

Server:

class Listener
{
 public readonly int Port;
 private readonly TcpListener _tcpListener;
 private readonly Task _listenTask;
 public Listener(int port)
 {
 Port = port;
 _tcpListener = new TcpListener(IPAddress.Any, Port);
 _tcpListener.Start();
 _listenTask = Task.Factory.StartNew(() => ListenLoop());
 }
 private async void ListenLoop()
 {
 for (; ; )
 {
 var socket = await _tcpListener.AcceptSocketAsync();
 if (socket == null)
 break;
 var client = new Client(socket);
 Task.Factory.StartNew(client.Do);
 }
 }
}
class Client
{
 private readonly Socket _socket;
 private readonly NetworkStream _networkStream;
 private readonly MemoryStream _memoryStream = new MemoryStream();
 private readonly StreamReader _streamReader;
 public Client(Socket socket)
 {
 _socket = socket;
 _networkStream = new NetworkStream(socket, true);
 _streamReader = new StreamReader(_memoryStream);
 }
 public async void Do()
 {
 byte[] buffer = new byte[4096];
 int bytesRead = await _networkStream.ReadAsync(buffer, 0, buffer.Length);
 Console.WriteLine("Doing some awesome work that takes 5 seconds.");
 Thread.Sleep(5000);
 Console.WriteLine("Finished doing work.");
 await _networkStream.WriteAsync(buffer, 0, buffer.Length);
 await _networkStream.FlushAsync();
 }
}

Client:

class Program
{
 static void Main(string[] args)
 {
 Thread.Sleep(5000);
 Console.WriteLine("Connecting...");
 TcpClient client = new TcpClient("localhost", 6666);
 Console.WriteLine("Connected.");
 try
 {
 Stream stream = client.GetStream();
 Byte[] messageToSend = new Byte[3];
 messageToSend[0] = 1;
 messageToSend[1] = 3;
 Byte[] messageReceived = new Byte[3];
 stream.WriteAsync(messageToSend, 0, 3);
 stream.Flush();
 stream.Read(messageReceived, 0, 3);
 // Echo
 Console.WriteLine(messageReceived[0] + messageReceived[1]);
 stream.Close(); 
 }
 finally
 {
 client.Close();
 }
 }
}

Answer 1

You are holding on to some IDisposable resources that you shouldn't be, which will impact scalability, GC pressure and likely performance. Here's the augmented Listener class:

internal sealed class Listener : IDisposable
{
 private readonly int port;
 private readonly TcpListener tcpListener;
 private readonly IList<Client> clients = new List<Client>();
 public Listener(int port)
 {
 this.port = port;
 this.tcpListener = new TcpListener(IPAddress.Any, this.port);
 this.tcpListener.Start();
 Task.Factory.StartNew(this.ListenLoop);
 }
 public int Port
 {
 get
 {
 return this.port;
 }
 }
 public void Dispose()
 {
 foreach (var client in this.clients)
 {
 client.Dispose();
 }
 }
 private async void ListenLoop()
 {
 while (true)
 {
 var socket = await this.tcpListener.AcceptSocketAsync();
 if (socket == null)
 {
 break;
 }
 var client = new Client(socket, this.ClientDisconnected);
 this.clients.Add(client);
 await Task.Factory.StartNew(client.Do);
 }
 }
 private void ClientDisconnected(Client client)
 {
 if (client == null)
 {
 return;
 }
 client.Dispose();
 this.clients.Remove(client);
 }
}

Here's the augmented Client class:

internal delegate void ClientDisconnectedDelegate(Client client);
internal sealed class Client : IDisposable
{
 private readonly Socket socket;
 private readonly ClientDisconnectedDelegate doDisconnect;
 private readonly Stream networkStream;
 private readonly Stream memoryStream = new MemoryStream();
 private readonly TextReader streamReader;
 public Client(Socket socket, ClientDisconnectedDelegate doDisconnect)
 {
 this.socket = socket;
 this.doDisconnect = doDisconnect;
 Task.Factory.StartNew(this.CheckForConnection);
 this.networkStream = new NetworkStream(this.socket, true);
 this.streamReader = new StreamReader(this.memoryStream);
 }
 public void Dispose()
 {
 this.streamReader.Dispose();
 this.networkStream.Dispose();
 this.socket.Dispose();
 this.memoryStream.Dispose();
 }
 public async void Do()
 {
 var buffer = new byte[4096];
 var bytesRead = await this.networkStream.ReadAsync(buffer, 0, buffer.Length);
 Console.WriteLine("Doing some awesome work that takes 5 seconds.");
 Thread.Sleep(5000);
 Console.WriteLine("Finished doing work.");
 await this.networkStream.WriteAsync(buffer, 0, bytesRead);
 await this.networkStream.FlushAsync();
 }
 private void CheckForConnection()
 {
 while (true)
 {
 bool isDisconnected;
 try
 {
 isDisconnected = this.socket.Poll(1, SelectMode.SelectRead) && (this.socket.Available == 0);
 }
 catch (SocketException)
 {
 isDisconnected = true;
 }
 if (isDisconnected && (this.doDisconnect != null))
 {
 this.doDisconnect(this);
 return;
 }
 Thread.Sleep(100);
 }
 }
}

And here's the augmented Client program:

internal static class Program
{
 private static void Main()
 {
 Thread.Sleep(5000);
 Console.WriteLine("Connecting...");
 using (var client = new TcpClient("localhost", 6666))
 {
 Console.WriteLine("Connected.");
 using (var stream = client.GetStream())
 {
 var messageToSend = new byte[3];
 messageToSend[0] = 1;
 messageToSend[1] = 3;
 var messageReceived = new byte[3];
 stream.WriteAsync(messageToSend, 0, 3);
 stream.Flush();
 stream.Read(messageReceived, 0, 3);
 // Echo
 Console.WriteLine(messageReceived[0] + messageReceived[1]);
 Console.ReadLine();
 }
 }
 }
}

• Felix: In your server, why aren't you disposing your client? Is it not necessary?
• Jesse C. Slicer: @Felix Excellent question. Let me get back to you on that.
• Jesse C. Slicer: @Felix Updated the answer to dispose of clients necessary.
• piers7: So OP wanted to deal with multiple clients, but by having the server listener loop await'ing client.Do() surely your sample code above can only service one client at a time? Why do you think you need that await?
• Jesse C. Slicer: @piers7 that's truly not how await works at all.