Async socket client using TcpClient class C#

Question

I have implemented a socket client using TcpClient class. So I can send and receive data, everything works good. But I ask some of the guru's out there :) Is there something wrong with my implementation? maybe there is a better way of doing things. In particular, how do I handle disconnects? Is there some indicator (or maybe I can write one myself) that tells me that socket has disconnected?

I also have looked into async await features of Socket class, but can't wrap my head around "SocketAsyncEventArgs", why is it there in the first place. Why cant i just: await Client.SendAsync("data"); ?

public class Client
{
    private TcpClient tcpClient;

    public void Initialize(string ip, int port)
    {
        try
        {
            tcpClient = new TcpClient(ip, port);

            if (tcpClient.Connected)
                Console.WriteLine("Connected to: {0}:{1}", ip, port);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
            Initialize(ip, port);
        }
    }

    public void BeginRead()
    {
        var buffer = new byte[4096];
        var ns = tcpClient.GetStream();
        ns.BeginRead(buffer, 0, buffer.Length, EndRead, buffer);
    }

    public void EndRead(IAsyncResult result)
    {
        var buffer = (byte[])result.AsyncState;
        var ns = tcpClient.GetStream();
        var bytesAvailable = ns.EndRead(result);

        Console.WriteLine(Encoding.ASCII.GetString(buffer, 0, bytesAvailable));
        BeginRead();
    }

    public void BeginSend(string xml)
    {
        var bytes = Encoding.ASCII.GetBytes(xml);
        var ns = tcpClient.GetStream();
        ns.BeginWrite(bytes, 0, bytes.Length, EndSend, bytes);
    }

    public void EndSend(IAsyncResult result)
    {
        var bytes = (byte[])result.AsyncState;
        Console.WriteLine("Sent  {0} bytes to server.", bytes.Length);
        Console.WriteLine("Sent: {0}", Encoding.ASCII.GetString(bytes));
    }
}

And the usage:

static void Main(string[] args)
{
    var client = new Client();
    client.Initialize("127.0.0.1", 8778);

    client.BeginRead();
    client.BeginSend("<Names><Name>John</Name></Names>");

    Console.ReadLine();
}

Answer 1

Okay it took me 10 seconds to find biggest issue you could've done :

public void BeginRead()
{
    var buffer = new byte[4096];
    var ns = tcpClient.GetStream();
    ns.BeginRead(buffer, 0, buffer.Length, EndRead, buffer);
}

But don't worry that's why we are on SO.

---

First let me explain why it's such a big issue.

Assume that you're sending message which is 4097 bytes long. Your buffer can only accept 4096 bytes meaning that you cannot pack whole message into this buffer.

Assume you're sending message which is 12 bytes long. You're still allocating 4096 bytes in your memory just to store 12 bytes.

How to deal with this?

Every time you work with networking you should consider making some kind of protocol ( some people call it message framing, but it's just a protocol ) that will help you to identify incomming package.

Example of a protocol could be :

[1B = type of message][4B = length][XB = message]
- where X == BitConvert.ToInt32(length);

• Receiver:

  byte messageType = (byte)netStream.ReadByte();
  byte[] lengthBuffer = new byte[sizeof(int)];
  int recv = netStream.Read(lengthBuffer, 0, lengthBuffer.Length);
  if(recv == sizeof(int))
  {
      int messageLen = BitConverter.ToInt32(lengthBuffer, 0);
      byte[] messageBuffer = new byte[messageLen];
      recv = netStream.Read(messageBuffer, 0, messageBuffer.Length);
      if(recv == messageLen)
      {
          // messageBuffer contains your whole message ...
      }
  }
  

• Sender:

  byte messageType = (1 << 3); // assume that 0000 1000 would be XML
  byte[] message = Encoding.ASCII.GetBytes(xml);
  byte[] length = BitConverter.GetBytes(message.Length);
  byte[] buffer = new byte[sizeof(int) + message.Length + 1];
  buffer[0] = messageType;
  for(int i = 0; i < sizeof(int); i++)
  {
      buffer[i + 1] = length[i];
  }
  for(int i = 0; i < message.Length; i++)
  {
      buffer[i + 1 + sizeof(int)] = message[i];
  }
  netStream.Write(buffer);
  

---

Rest of your code looks okay. But in my opinion using asynchronous operations in your case is just useless. You can do the same with synchronous calls.