TCP server implementation in .NET

Question

I wrote a couple programs in the past that use VB6's Winsock to achieve TCP communications, but still I'm kinda new to socket programming in .NET.

I needed the same functionality of the Winsock to use in .NET with the advantages of multi-threading and asynchronous support (I'm kinda new to that as well). I couldn't find a complete example that achieves this, so I decided to write it myself.

I believe it's wrong to run each client on a separate thread, and that's why I used the Socket class instead of TcpClient in order to use BeginReceive's async method.

What I want some help with:

1. If I can count on this in performance or if it requires any modifications.
2. Is it okay to run all the socket work on one thread (other than the UI thread)?
3. Will raising events like MessageReceived block the TCP work?
4. If most of the messages will be small length strings (but a lot of clients to handle per second), should I really use async, or instead just use the TcpClient.GetStream?

Server class:

Public Class TcpServer
 Private _sckListener As TcpListener
 Private _thread As Thread
 Private _running As Boolean
 Private _clientCounter As Long
 Private _clients As SortedList(Of Long, ServerClient)
 Public Property Clients() As SortedList(Of Long, ServerClient)
 Get
 Return _clients
 End Get
 Private Set(ByVal value As SortedList(Of Long, ServerClient))
 _clients = value
 End Set
 End Property
 Private _port As Integer = -1
 Public Property Port() As Integer
 Get
 Return _port
 End Get
 Set(ByVal value As Integer)
 _port = value
 End Set
 End Property
 Private _localIP As IPAddress
 Public Property LocalIP() As IPAddress
 Get
 Return _localIP
 End Get
 Set(ByVal value As IPAddress)
 _localIP = value
 End Set
 End Property
 Public Sub New()
 _localIP = IPAddress.Any
 End Sub
 Public Sub New(ListenPort As Integer)
 _port = ListenPort
 _localIP = IPAddress.Any
 End Sub
 Public Sub New(IP As IPAddress, ListenPort As Integer)
 _localIP = IP
 _port = ListenPort
 End Sub
 Public Sub New(StrIP As String, ListenPort As Integer)
 If Not IPAddress.TryParse(StrIP, _localIP) Then
 Throw New FormatException("'StrIP' is not a valid IP.")
 End If
 _port = ListenPort
 End Sub
 Public Sub Listen()
 If _port = -1 Then
 Throw New NullReferenceException("Server port not assigned.")
 End If
 StartSocket()
 Clients = New SortedList(Of Long, ServerClient)
 _running = True
 _thread = New Thread(AddressOf DoListen)
 _thread.Start()
 End Sub
 Public Sub Listen(ListenPort As Integer)
 _port = ListenPort
 Listen()
 End Sub
 Private Sub StartSocket()
 _sckListener = New TcpListener(_localIP, _port)
 _sckListener.Start()
 End Sub
 Public Sub Close()
 _running = False
 StopSocket()
 For Each client In Clients.Values
 client.Disconnect()
 Next
 End Sub
 Private Sub StopSocket()
 Try
 _sckListener.Stop()
 Catch ex As Exception
 End Try
 End Sub
 Private Sub DoListen()
 Do While _running
 Try
 Dim sckClient As Socket = _sckListener.AcceptSocket()
 If sckClient.Connected Then
 AddNewClient(sckClient)
 End If
 Catch ex As Exception
 StopSocket()
 Thread.Sleep(1000)
 If Not _running Then Exit Sub
 Try
 StartSocket()
 Catch
 End Try
 End Try
 Loop
 End Sub
 Private Sub Client_Disconnected(sender As Object, e As ServerClientEventArgs)
 RaiseEvent ClientDisconnected(Me, e)
 End Sub
 Private Sub AddNewClient(ByVal clientSocket As Socket)
 Dim clientID As Long = NewID()
 Dim client As New ServerClient(clientSocket, clientID)
 Clients.Add(clientID, client)
 RaiseEvent ClientConnected(Me, New ServerClientEventArgs(client))
 client.Start()
 AddHandler client.Disconnected, AddressOf Client_Disconnected
 End Sub
 Private Function NewID() As Long
 _clientCounter += 1
 Return _clientCounter
 End Function
 Public Event ClientConnected As EventHandler(Of ServerClientEventArgs)
 Public Event ClientDisconnected As EventHandler(Of ServerClientEventArgs)
End Class

Another class to handle communications with client:

Public Class ServerClient
 Private _sckClient As Socket
 Private Const ReceiveBufferSize As Integer = 4 * 1024
 Private ReadOnly _buffer As Byte()
 Private _running As Boolean
 Private _clientID As Long
 Public ReadOnly Property ClientID() As Long
 Get
 Return _clientID
 End Get
 End Property
 Public Sub New(ByVal ClientSocket As Socket, ByVal ID As Long)
 _sckClient = ClientSocket
 _clientID = ID
 _buffer = New Byte(ReceiveBufferSize - 1) {}
 End Sub
 Public Sub Start()
 _running = True
 _sckClient.BeginReceive(_buffer, 0, _buffer.Length, 0, New AsyncCallback(AddressOf ReceiveCallback), Nothing)
 End Sub
 Private Sub ReceiveCallback(ar As IAsyncResult)
 If Not _running Then Exit Sub
 Try
 'Get received bytes count
 Dim bytesRead = _sckClient.EndReceive(ar)
 If bytesRead > 0 Then
 'Copy received bytes to a new byte array
 Dim receivedBytes = New Byte(bytesRead - 1) {}
 Array.Copy(_buffer, 0, receivedBytes, 0, bytesRead)
 RaiseEvent MessageRecieved(Me, New MessageRecievedEventArgs(receivedBytes))
 Else
 Throw New SocketException("Tcp socket is closed")
 End If
 'Read more bytes if still running
 If _running Then
 _sckClient.BeginReceive(_buffer, 0, _buffer.Length, 0, New AsyncCallback(AddressOf ReceiveCallback), Nothing)
 End If
 Catch ex As Exception
 Disconnect()
 End Try
 End Sub
 Public Sub Send(Message As String)
 Send(Encoding.Unicode.GetBytes(Message))
 End Sub
 Public Sub Send(PacketArr As Byte())
 _sckClient.Send(PacketArr)
 End Sub
 Public Sub Disconnect()
 _running = False
 Try
 If _sckClient.Connected Then
 _sckClient.Close()
 End If
 _sckClient.Dispose()
 Catch ex As Exception
 End Try
 RaiseEvent Disconnected(Me, New ServerClientEventArgs(Me))
 End Sub
 Public Event Disconnected As EventHandler(Of ServerClientEventArgs)
 Public Event MessageRecieved As EventHandler(Of MessageRecievedEventArgs)
End Class

• Mark Hurd: Just answering your question "Will raising events like MessageReceived block the TCP work?": Yes, a normal event completes all event handlers, until an unhandled exception occurs, and then returns, or the exception is thrown. You can produce a custom exception that does things differently, like calling BeginInvoke on each event handler, and then handle the exceptions thrown by EndInvoke in some asynchronous way. But then definitely document that your events are run asynchronously!
• 41686d6564: @MarkHurd Thank you, but what do you mean by "until unhandled exception occurs"? I don't get what does throwing an exception have to do with raising this event.
• 41686d6564: @D.Jurcau I'm already using TcpListener!
• Mark Hurd: @GeniuSBraiN If an event handler (often not your code) throws an unhandled exception, the default event processing code just allows that exception to pass back into your code, through the RaiseEvent (and does not call any further event handlers added to the event).
• 41686d6564: @MarkHurd Good point, I didn't know that :)

Answer 1

• You shouldn't Thread.Sleep(). It's better to replace the forever loop with a timer from the System.Threading namespace.
• Your client should implement IDisposable. It contains a disposable member (field) and implementing IDisposable is the idiomatic way to ensure the resources get cleaned up.
• You have a ton of properties that just pass through to their backing field. Using auto properties would shave a fair number of lines from your code, decluttering your implementation.
• Have you looked into what libraries/frameworks/software is available for this task? I find it hard to believe that there isn't something you could use "out of the box". SignalR comes to mind, but I'm not sure it meets your specific needs.

• 41686d6564: 1- Yes I think you're right. I'll see about the timer. 2- I already implemented IDisposable. 3- That's right, I'll fix that. 4- AFAICT, SignalR isn't specially designed for TCP communications [I guess] (goo.gl/pPQVZG). And yes I did my best trying to find a fair wrapper for this. The best I could found is [this library] (goo.gl/wLE5nC), I think it's well implemented and it even have more advanced stuff that I don't need, but it uses some sort of serialization to send/receive messages. So I had to choose between either modifying it or create a simple wrapper by myself.