Send out message from server to all clients c#?

Question

I try to create a server program in console with C#. I use ThreadPool to create separate socket for each clients. Then, I create a static List<TcpClient> clients = new List<TcpClient>();to contains all clients connected to.

Then, what I want is, when 1 client send to server a message, server will receive and send it out to all client connected. So, I wrote:

foreach (var item in clients)
{
   ns.Write(data, 0, recv);
   //send message to all client
}

But, only client just sent the message can receive it back, another clients was receive nothing!

**** Server side:

using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Sockets;
using System.Text;
using System.Threading;
class ThreadedTcpSrvr
{
    private TcpListener client;

    //private 

    public ThreadedTcpSrvr()
    {
        client = new TcpListener(IPAddress.Parse("127.0.0.1"), 9050);
        client.Start();
        Console.WriteLine("Waiting for clients...");
        while (true)
        {
            while (!client.Pending())
            {
                Thread.Sleep(1000);
            }
            ConnectionThread newconnection = new ConnectionThread();
            newconnection.threadListener = this.client;
            Thread newthread = new Thread(new
                 ThreadStart(newconnection.HandleConnection));
            newthread.Start();
        }
    }
    public static void Main()
    {
        ThreadedTcpSrvr server = new ThreadedTcpSrvr();
    }
}
class ConnectionThread
{
    static List<TcpClient> clients = new List<TcpClient>();
    public TcpListener threadListener;
    private static int connections = 0;
    public void HandleConnection()
    {
        int recv;
        byte[] data = new byte[1024];
        TcpClient client = threadListener.AcceptTcpClient();
        NetworkStream ns = client.GetStream();
        //TcpClient clientSocket = client.AccepTcpClient();
        clients.Add(client);
        connections++;
        Console.WriteLine("New client accepted: {0} active connections",
                 connections);
        string welcome = "Welcome to my test server";
        data = Encoding.ASCII.GetBytes(welcome);
        ns.Write(data, 0, data.Length);
        while (true)
        {
            data = new byte[1024];
            recv = ns.Read(data, 0, data.Length);
            if (recv == 0)
                break;
            Console.WriteLine(Encoding.ASCII.GetString(data, 0, recv));
            //ns.Write(data, 0, recv);
            foreach (var item in clients)
            {
                ns.Write(data, 0, recv);
                //send message to client
            }
        }
        ns.Close();
        client.Close();
        connections--;
        Console.WriteLine("Client disconnected: {0} active connections",connections);
    }
}

**** Client side:

void ReceiveData(IAsyncResult iar)
{
   try
   {
       while(true)
       {
          Socket remote = (Socket)iar.AsyncState;
          int recv = remote.EndReceive(iar);
          string stringData = Encoding.ASCII.GetString(data, 0, recv);
          ListAddItem(stringData);
       }
   }
   catch
   {

   }

}

because of ns.write in your clients loop - you arent using the socket for each client but the one you received with. Which you were hinted at when you asked basically the same question earlier — BugFinder, Nov 08 '17 at 15:34
can't use item.Write(data, 0, recv); because TcpClient does not contain Write() method. — Co May Hoa, Nov 08 '17 at 15:48
In general, thread-per-socket doesn't scale well. You're better off using some form of async (there are a number of options for sockets) and letting windows lend you I/O threads on those small occasions when *there's something useful for a thread to do*, such as to process a buffer full of data that has (finally!) arrived over the network. — Damien_The_Unbeliever, Nov 08 '17 at 15:54
Others have pointed out the typo in your code. For future reference, here's something you can be on the lookout for: if you have a `for` or `foreach` loop and in the loop you **never use the loop variable**, you've probably done something wrong. This is especially true for `foreach` loops. As for your basic approach, I also agree that you should learn the async APIs so you can avoid dedicating a whole thread to each client. FWIW, you can see a detailed chat example I posted recently, here: https://stackoverflow.com/a/44942011 — Peter Duniho, Nov 09 '17 at 03:26

score 0 · Answer 1 · answered Nov 08 '17 at 16:47

Your ns (network stream is on a per client basis )

In the following code your ns is for the incoming client. So you are iterating over all clients but then only talking to the client that is currently incoming

        foreach (var item in clients)
        {
            ns.Write(data, 0, recv);
            //send message to client
        }

Agree with @Damien_The_Unbeliever this is will work for a small number of clients but when you start to scale over 2000+ clients you will run into performance problems. With the thread switching and the amount of ports that need to be opened. It would be better to use an event / async based architecture

Send out message from server to all clients c#?

1 Answers1