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

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.