Implementing a good TCP Socket Server

I'd like a code review on my very simple server application that validates whether the serial number retrieved from the client is a valid one or not.

-
Is there a better way to handle the start/stop on the service? If you look at my Start and Service methods, I'm basically looking at isServerRunning boolean variable and getting the service out of the while loop whenever it has been toggled to false, but this doesn't seem like the ideal approach since the thread is basically paused at listner.AcceptSocket().

-
Does this guarantee N:1 connectivity, 1 being the server?

-
I'm going to set a send/receive timeout on my socket. What is a generally reasonable timeout value to set?

I'd also like to hear comments on the overall coding style and any other suggestions on improving this code.

```
public partial class ServerForm : Form
{
#region Fields
private bool isServerRunning = false;
private const int CLIENT_LIMIT = 10;
private TcpListener listener;

#endregion

#region Event Handlers

private void btnStart_Click(object sender, EventArgs e)
{
try
{
int port;

if (String.IsNullOrEmpty(txtPort.Text))
{
MessageBox.Show(Constant.ERROR_PORT_NUMBER_EMPTY);
return;
}

if (isServerRunning)
return;

if (!int.TryParse(txtPort.Text, out port))
{
MessageBox.Show(Constant.ERROR_PORT_ONLY_INT_ALLOWED);
return;
}

if (port 65535)
{
MessageBox.Show(Constant.ERROR_PORT_NUMBER_OUT_OF_RANGE);
return;
}

Start(port);
}
catch (Exception ex)
{
LogWriter.WriteExceptionLog(ex.ToString());
}
}

private void btnStop_Click(object sender, EventArgs e)
{
if (isServerRunning)
{
isServerRunning = false;

You may want to look some other guides/implementations about that:

CodeProject | C# SocketAsyncEventArgs High Performance Socket

Stack Overflow | How to write a scalable Tcp/Ip based server

About your code:

• You should seperate the UI and the server code, so you can be able to use it on a console application or -more likely- a windows service.

• TcpListener is a good class that does its job pretty well but if you want something more scalable and customizable, I think it's too high-level for you. For this you should deal with Socket class directly.

You should also know,

There are so many concepts that you should be aware of about TCP and socket programming in general; in order to write a robust, scalable TCP server. You should know about framing protocols, find a good way to handle your buffers, be experienced at asynchronous code and debugging that code.

Edit: This FAQ page by Stephen Cleary is a really good place to get started.

I suggest you to take a look at the links I mentioned above. They provide some good implementations and explain why they do the things they do in a comprehensive way. Then you can roll your own, avoiding common pitfalls.

Back to your code:

• Creating a new thread for each connection is brutal. You should use the asynchronous methods of the TcpListener class. Or you can at least use ThreadPool and the easiest way to do this is using the Task class.

• LocalIPAddress() method returns a string that you parse again to an IPAddress object to use. Don't convert it to a string, return it directly as an IPAddress.

• AddressFamily is an Enum. You can check whether it is InterNetwork without converting it to a string: ip.AddressFamily == AddressFamily.InterNetwork

• Try to use your streams in using blocks or dispose them in finally blocks so you can be sure they're closed even if an exception is thrown in your code.

• Timeout really depends on the client, network and the size of the data you're sending/receiving. Whatever amount of inactive time you think that means "there's something wrong" will be a sufficient timeout value.

Nonetheless:

My advice is not to continue this and start another after you look into some other implementations like the ones I mentioned. You need a UI independent project that can be referenced by your other applications that need to use (or that need to be) a TCP server.

Edit: Although I said that you should be able to, it doesn't mean you should have a TCP server in a GUI application. You need a Windows service to run on the background, deal with high amounts of connections and manage the data traffic between itself and its clients. Doing these kinds of operations in a GUI application is usually a bad idea.

StackExchange Code Review Q#17832, answer score: 24