I've recently started learning about computer networks and decieded to try TCP/IP server and client. They both work, but I'm having issues with sending mutliple data to the server. I've made it to look like a chat service between clients but the server accepts only one client and closes the connection after the data is sent and the client for some reason stops responding after sending data to server (I think the problem comes from the server and not the client itself), no error message, only on the server side when I force close the client. This is how my server looks like...
static void Main(string[] args)
{
//User can define port
Console.WriteLine("open a port:");
string userInputPort = Console.ReadLine();
//listening for connections
TcpListener listener = new TcpListener(System.Net.IPAddress.Any, Convert.ToInt32(userInputPort));
listener.Start();
Console.WriteLine("listening...");
while (true)
{
//waiting for client to connect to server
Console.WriteLine("Waiting for connection...");
//when user connects to server, server will accept any request
TcpClient client = listener.AcceptTcpClient();
Console.WriteLine("Client Accepted");
NetworkStream stream = client.GetStream();
StreamReader streamR = new StreamReader(client.GetStream());
StreamWriter streamW = new StreamWriter(client.GetStream());
while (true)
{
if(client.Connected)
{
if (stream.CanRead)
{
//buffer
byte[] buffer = new byte[1024];
stream.Read(buffer, 0, buffer.Length);
int recv = 0;
foreach (byte b in buffer)
{
if(b != 0)
{
recv ;
}
}
string request = Encoding.UTF8.GetString(buffer, 0, recv);
Console.WriteLine("request recived: " request);
streamW.Flush();
}
}
}
}
}
}
}
and this is how the client looks like...
...
try
{
//try to connect
client = new TcpClient(textBoxIP.Text, Convert.ToInt32(textBoxPort.Text));
}
...
static void sendMessage(string message, TcpClient client)
{
int byteCount = Encoding.ASCII.GetByteCount(message);
byte[] sendData = new byte[byteCount];
sendData = Encoding.ASCII.GetBytes(message);
NetworkStream stream = client.GetStream();
stream.Write(sendData, 0, sendData.Length);
StreamReader streamReader = new StreamReader(stream);
string respone = streamReader.ReadLine();
stream.Close();
client.Close();
}
Like I said, I'm still learning about computer networking and any comment to this code will help! Thank you
CodePudding user response:
while (true)
{
if(client.Connected)
{
if (stream.CanRead)
{
I don't see any code, that exits the outer while the loop if either client.Connected
or stream.CanRead
become false. So, when the client disconnects and they become false, it seems to me that the server just loops forever.
You should at least do all error handling (close all necessary streams) and break out of the loop.
As the next problem, you code can only have one client at a time. If the client is not actually closing the connection. I do not know for sure what the correct C# solution is, but i think it is spawning a separate thread for each connected client.
CodePudding user response:
It helps if you give yourself some idea of what you're actually expecting from the code you're writing. It seems to me that you make a lot of automatic assumptions without actually making sure to put them in your code.
- Your server can only ever at best accept a single client. Not one client at a time, but one ever. You never exit from your reading loop, so after the client disconnects, you end up in a wonderful infinite busy loop. Your intent was probably to serve another client when one disconnects, but that's not what you're doing.
- You assume the server will send a response to the client. But you never actually send any response! For a client to read something, the server first must send something for the client to read.
- You assume the string sent by the client will be zero-terminated, or that the target buffer for
Read
will be zeroed. If you want zero-termination, you have to send it yourself from the client - theStreamWriter
certainly doesn't do that. Strings aren't zero-terminated as a rule - it's just one C-style way of representing strings in memory. You shouldn't assume anything about the contents of the buffer beyond what the return value fromRead
tells you was returned.
Those are issues with things you forgot to quite put in, presumably. Now to the incorrect assumptions on part of how TCP works. To keep clarity, I will tell the way it is, rather than the incorrect assumption.
- A single write can result in multiple reads on the other side, and a single read can read data from multiple writes on the other side. TCP doesn't send (and receive) messages, it deals with streams. You need to add a messaging protocol on top of that if streams aren't good enough for you.
Read
returns how many bytes were read. Use that to process the response, instead of looking for a zero. WhenRead
returns a zero, it means the connection has been closed, and you should close your side as well. This is all that you need, instead of all thewhile (true)
,if (Connected)
andif (CanRead)
- loop untilRead
returns zero. Process data you get as it gets to you.- The TCP stream is a bit trickier to work with than most streams; it behaves differently enough that using helpers like
StreamReader
is dangerous. You have to do the work yourself, or get a higher-abstraction library to work with networking. TCP is very low level. - You cannot rely on getting a response to a
Read
. TCP uses connections, but it doesn't do anything to keep the connection alive on its own, or notice when it is down - it was designed for a very different internet than today's, and it can happily survive interruptions of service for hours - as long as you don't try to send anything. If the client disconnects abruptly, the server might never know.
You should also make sure to clean up all the native resources properly - it really helps to use using
whenever possible. .NET will clean up eventually, but for things like I/O, that's often dangerously late.