Async Send to multiple clients and correct usage o

Question

I have 500 connected clients on the server. Server sends data to all clients in every N sec.

When the server sends data to clients the result is:

-----------Result-----------
Client1 received : 102 datas
Client2 received : 109 datas
Client3 received : 105 datas
Client4 received : 108 datas
Client5 received : 108 datas
ClientN received : 107 datas

But it should be 110 datas at the end.

I am using the following code to send data to clients

public static class AsyncSend
{
    public static Task<int> SendAsync(this Socket socket, byte[] buffer, int offset, int count)
    {
        return new Task<int>(() =>
        {
            return Task.Factory.FromAsync<int>(
                           socket.BeginSend(buffer, offset, count, SocketFlags.None, null, socket),
                     socket.EndSend).Result;
        });
    }
}

class ServerTest
{
  private Thread sendThread;
  private Dictionary<String, Client> connectedClients;

  public ServerTest()
  {
    connectedClients = new Dictionary<String, Client>();
  }

  public void receive()
  {
    //here server receives data from another server
    //When server receives new data then fires the newDataReceived(String data) method.

    newDataReceived(data);
  }

  public void newDataReceived(String data)
  {
    sendThread = new Thread(new ParameterizedThreadStart(sendToClients));
    sendThread.Start(data);
  }

  private void sendToClients(object dataObj)
  {
    try
    {
      int totalConnectedClients = connectedClients.Count;

      if (totalConnectedClients > 0)
      {
        lock (connectedClients)
        {
          byte[] buffer = Encoding.UTF8.GetBytes((string)dataObj);
          byte[] cBuffer = new SCompressor().Compress(buffer);

          foreach (Client client in connectedClients.Values)
          {
            Task<int> task = AsyncSend.SendAsync(client.Socket, cBuffer, 0, cBuffer.Length);
            task.Start();
          }
        }
      }
    }
    catch (Exception ex)
    {
      //ERROR.
    }
  }

Is there another way or how to send data to multiple clients correctly and efficiently?

Please show me examples if you can.

Thanks in advice.

Answer 1

Does the SCompressor class add a length prefix or unique end marker to the data it returns?

Without that the client code won't be able to know how many bytes to recieve before decompressing.

This is the only potential bug I can see that would affect the amount of data recieved by the clients assuming thats what the client result diagnosic is showing.

If so would be easy to fix - just add the length of the compressed data to the start of the buffer that the server sends:

        var sendBuffer = new byte[cBuffer.Length + 4];
        BitConverter.GetBytes(cBuffer.Length).CopyTo(sendBuffer, 0);
        cBuffer.CopyTo(sendBuffer, 4);

Then change the client to read 4 bytes, decode the length as int, keep reading until length bytes are received, decompress.

Answer 2

One point that has not been made on this thread is that TCP is a stream protocol. This means that if the server sends 110 bytes, the client can receive anything between 1-110 bytes on a single read call. If the client knows that 110 bytes are expected, then it needs to call Read repeatedly until all 110 bytes have arrived.

Answer 3

I have some points for you:

You can improve efficiency by getting rid of one superfluous task per send:

public static Task<int> SendAsync(this Socket socket, byte[] buffer, int offset, int count)
{
        return Task.Factory.FromAsync<int>(
                       socket.BeginSend(buffer, offset, count, SocketFlags.None, null, socket),
                 socket.EndSend);
//Don't explicitly start the return value of this method. The task will already be started.
}

You usage of asynchronous sending is correct as far as I can tell (except for the unneeded task).

Are you using UDP? If yes, that would explain why some data is missing.

Is there an error that is being caught? If yes, the loop will abort and not all clients will get their data.

You have a race condition:

int totalConnectedClients = connectedClients.Count;

This should happen inside the lock. You cannot safely read Count while other threads might update this dictionary.

How are you receiving the data? Maybe the bug is on the receive-side. Please post some code.

Edit:

What if two concurrent receives are running? This would cause a bug: Clients could receive the newest data first, then the older data second. I recommend that you do not start a new thread every time you receive data. Instead, have a thread running permanently which is pulling new data from a BlockingCollection using the GetConsumerEnumerable method. Your newDataReceived method would just enqueue into this BlockingCollection. You can find an excellent introduction here: http://blogs.msdn.com/b/csharpfaq/archive/2010/08/12/blocking-collection-and-the-producer-consumer-problem.aspx

Answer 4

I have absolutely not idea what does “102 datas” received mean, or what are you actually asking, but your SendAsync() method could be greatly improved like this:

public static Task<int> SendAsync(this Socket socket, byte[] buffer, int offset, int count)
{
    return Task.Factory.FromAsync<int>(
        socket.BeginSend(buffer, offset, count, SocketFlags.None, null, socket),
        socket.EndSend);
}

This way, you're not blocking any thread and so you should get better efficiency.

One difference between the modified code and your code is that the Task is already started, so you shouldn't call Start() on it.