I have a server side app running in C# that is supposed to be receiving data from two different clients connected to the same port (and I would like to be able to send data to both clients in one go). I have a while(true) loop that accepts a tcp client with the TcpListener object. When one is acquired, I start a new thread to handle this client (is this absolutely necessary? I'm really struggling to understand when/where to use async, different threads, etc.). The problem is that while everything works fine with one client, when I send a message from a new client that is also connected, the first set of bytes I get is completely empty, although the networkStream.read function does get processed because my windows form does output some text, just with an empty byte array.
The way this seems to happen is that the server will get nothing when a second client sends data the first time, then its like it switches to that client because the next message and all the others afterward go through just fine. The echoing back works fine as well, except the client not doing the messaging doesn't get the echo. In fact, the first client gets the echo when the 2nd client sends its first (ends up blank) message, and thereafter does not get any more echos (for the following successful messages). The blank message attempt does not send an echo back to the blank message's sender, but the other messenger does get it, but just that once.
It basically seems to be switching back and forth between the clients.
I see that i have networkstream=client.getstream() at the beginning of my receivehandler's while(true) loop, so it makes sense that there would be a problem, but I'm not sure how to restructure my code to make this work out. Clearly I have a problem with the Network stream object getting re-assigned a different stream from a different thread over and over, and then that also points out the flaw that I have a single
I've tried a lot of different debugging things but its just clear to me by now that after looking at the code real good I need to figure out a way of having different clients for the same port without re-assigning the networkStream object to a different client's .streamRead function (new client is being handled from a different thread)
public class Handler
{
string portNumber;
TcpClient Client;
TcpListener port;
TextBox textbox;
TextBox sendbox;
Task thisTask;
int clientnumber=0;
CancellationTokenSource cts;
Button button;
Form1 form1;
public const int BufferSize = 1024;
public byte[] buffer = new byte[BufferSize];
byte[] sendbytes = new byte[BufferSize];
NetworkStream networkStream;
public StringBuilder sb = new StringBuilder();
public Handler(IPAddress ip, int portnum, string portnumber, Form1
form)
{
port = new TcpListener(ip, portnum);
port.Start();
form1 = form;
portNumber = portnumber;
foreach (Control t in form1.Controls) if (t.Name == portnumber +
"chatbox") textbox = (TextBox)t;
foreach (Control s in form1.Controls) if (s.Name == portnumber +
"sendbox") sendbox = (TextBox)s;
foreach (Control b in form1.Controls) if (b.Name == "button" +
portnumber[4]) button = (Button)b;
button.Click += SendData;
cts = new CancellationTokenSource();
thisTask = Task.Run(listen, cts.Token);
}
private async Task listen()
{
try
{
while (true)
{
Client = await port.AcceptTcpClientAsync();
// Start a thread to handle this client...
SetText("A new client " + ++clientnumber + " has connected to
" + portNumber + "\r\n");
new Thread(() => HandleClient(Client,clientnumber)).Start();
}
}
catch (OperationCanceledException) when
(cts.Token.IsCancellationRequested)
{
//ignore this ex
}
}
delegate void SetTextCallback(string text);
public void HandleClient(TcpClient Client, int cnum)
{
while (true)
{
try
{
networkStream = Client.GetStream();
Client.ReceiveBufferSize = 1024;
networkStream.Read(buffer, 0, BufferSize);
SetText("\r\n Client " + cnum.ToString() + " sent: " +
Encoding.ASCII.GetString(buffer) + "\r\n");
sendbytes = Encoding.ASCII.GetBytes(portNumber + " has
processed a message from client " + cnum.ToString() + ".");
networkStream.Write(sendbytes, 0, sendbytes.Length);
networkStream.Flush();
buffer = new byte[BufferSize];
}
catch (Exception ex)
{
SetText("\r\n\r\n" + ex.ToString());
}
}
}
private void SetText(string text)
{
if (textbox.InvokeRequired)
{
SetTextCallback d = new SetTextCallback(SetText);
textbox.Invoke(d, new object[] { text });
}
else textbox.Text += text;
}
The main thing I've left out is that I'm using a middle man to pass data from clients to server. The middle man is basically websockify.js from github, but i think they've done some updates since then. When a client goes to a certain webaddress, websockify will connect websockets to that client and also see if there is a TcpListener at the specified ports, if there is it will assign a bunch of event emitters to the handler for that TcpConnection to the server, named "target". Like so: target.on('data,msg){client.send(data.toString());}
I would think that if I get data from the target AT ALL that the .on event emitter that was set up for both clients will get triggered and both clients will get the data, so I'm very confused as to how both clients could manage to not be getting the echo back from the server.
NEXT TRY AFTER FIRST REPLY:
public class Handler
{
string portNumber;
List<TcpClient> Clients = new List<TcpClient>();
TcpListener port;
TextBox textbox;
TextBox sendbox;
Task thisTask;
int clientnumber=0;
CancellationTokenSource cts;
Button button;
Form1 form1;
public const int BufferSize = 1024;
public StringBuilder sb = new StringBuilder();
public Handler(IPAddress ip, int portnum, string portnumber, Form1 form)
{
port = new TcpListener(ip, portnum);
port.Start();
form1 = form;
portNumber = portnumber;
foreach (Control t in form1.Controls) if (t.Name == portnumber +
"chatbox") textbox = (TextBox)t;
foreach (Control s in form1.Controls) if (s.Name == portnumber +
"sendbox") sendbox = (TextBox)s;
foreach (Control b in form1.Controls) if (b.Name == "button" +
portnumber[4]) button = (Button)b;
button.Click += SendData;
cts = new CancellationTokenSource();
thisTask = Task.Run(listen, cts.Token);
}
private async Task listen()
{
try
{
while (true)
{
var dummyclient = await port.AcceptTcpClientAsync();
Clients.Add(dummyclient);
// Start a thread to handle this client...
SetText("A new client " + clientnumber++ + " has connected
to " + portNumber + "\r\n");
new Task(() => HandleClient(dummyclient,
clientnumber)).Start();
}
}
catch (OperationCanceledException) when
(cts.Token.IsCancellationRequested)
{
//ignore this ex
}
}
public void HandleClient(TcpClient client, int cnum)
{
client.ReceiveBufferSize = 1024;
var networkStream = client.GetStream();
while (true)
{
try
{
var sendbytes = new byte[BufferSize];
var buffer = new byte[BufferSize];
networkStream.Read(buffer, 0, BufferSize);
SetText("\r\n Client " + cnum.ToString() + " sent: " +
Encoding.ASCII.GetString(buffer) + "\r\n");
sendbytes = Encoding.ASCII.GetBytes(portNumber + " has
processed a message from client " + cnum.ToString() + ".");
SendGlobally(sendbytes);
networkStream.Flush();
}
catch (Exception ex)
{
SetText("\r\n\r\n" + ex.ToString());
}
}
}
delegate void SetTextCallback(string text);
private void SetText(string text)
{
if (textbox.InvokeRequired)
{
SetTextCallback d = new SetTextCallback(SetText);
textbox.Invoke(d, new object[] { text });
}
else textbox.Text += text;
}
public void SendData(object sender, EventArgs e)
{
foreach (TcpClient tc in Clients)
{
var networkStream = tc.GetStream();
if (networkStream != null)
{
var sendbytes = Encoding.ASCII.GetBytes(sendbox.Text);
SetText("\r\n Server sent: " + sendbox.Text + "\r\n");
networkStream.Write(sendbytes, 0, sendbytes.Length);
networkStream.Flush();
sendbox.Text = "";
}
else SetText("\r\n A client has not connected to this port yet.
\r\n");
}
}
public void SendGlobally(byte[] data)
{
foreach (TcpClient tc in Clients)
{
var networkStream = tc.GetStream();
networkStream.Write(data, 0, data.Length);
networkStream.Flush();
}
}
public void close()
{
cts.Cancel();
port.Stop();
foreach (TcpClient tc in Clients)
if (tc != null) { tc.Close(); }
}
}
}
Race condition. You create a new thread for each client, and receive/send data from/to this client there, using member variables that are shared between threads. Don't do this. E.g. despite you correctly passed TcpClient to HandleClient, inside it you stored its NetworkStream in a member variable. Now it's perfectly possible that another client immediately after that overrides it with its own NetworkStream. If you have two clients, there're two threads each executing HandleClient, in parallel, both reading/modifying same member variables.
Don't use member variables in HandleClient until you really need them. In general, store data as locally as you can. In your case - local vars. If you do need to share some data between clients you can use member variables for this, but you need to synchronise access to them by mutex.