[RESOLVED] TCP client - server doubt on Atheist's example code

[akhileshbc]

Hi guys...

I'm trying to the learn TCP client server communication.

I have gone through Atheist's example code.

But I'm having some doubt.
What I'm trying to do is, the client will connect to the server and sends data. That's all. And the server will use this data for some other purpose.

Here's the client code I have made:

Public Class frmMain
    Private client As System.Net.Sockets.TcpClient
 
    Private Sub btnConnect_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnConnect.Click
        '~~~ Checking if connection exists
        If client.Connected = True Then
            If MessageBox.Show("A connection already exists ! Do you want to terminate it and establish a new connection ?", "Connection exists !", MessageBoxButtons.YesNo, MessageBoxIcon.Warning) = Windows.Forms.DialogResult.Yes Then
                client.Close()
            Else
                Exit Sub
            End If
        End If
 
        Dim strServerIP As String = txtServerIP.Text
        Dim intPort As Integer
 
        '~~~ Checking for valid inputs
        If strServerIP.Length = 0 Then
            MessageBox.Show("Please enter a valid IP address !", "IP address ?", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
            txtServerIP.Focus()
            Exit Sub
        End If
 
        If Integer.TryParse(txtPort.Text, intPort) = False Then
            MessageBox.Show("Please enter a valid port number !", "Port Number?", MessageBoxButtons.OK, MessageBoxIcon.Error)
            txtPort.Focus()
            Exit Sub
        End If
 
        '~~~ Connecting
        Try
            client = New System.Net.Sockets.TcpClient(txtServerIP.Text, intPort)
        Catch ex As Exception
            MessageBox.Show(ex.Message)
        End Try
    End Sub
 
    '~~~ Sends a message to the server
    Private Sub SendMessage(ByVal msg As String)
        Dim sw As IO.StreamWriter
        Try
            sw = New IO.StreamWriter(client.GetStream)
            sw.Write(msg)
            sw.Flush()
        Catch ex As Exception
            MessageBox.Show(ex.ToString)
        End Try
    End Sub
 
    Private Sub frmMain_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
        client.Close()
    End Sub
 
    Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        '~~~ Check if there's a connection or not
        If client.Connected = False Then
            MessageBox.Show("No connection exists ! Please connect to the server first !", "No connection to server", MessageBoxButtons.OK, MessageBoxIcon.Warning)
            Exit Sub
        End If
        '~~~ Send the message to server
        SendMessage("abc|123")
    End Sub
End Class

But in the server side, I'm having confusion on the doRead and doListen events (of Athiest's code). Since, I'm not going to store the connected clients, I'm not going to use a class for the connected clients (in the server side).

Can you please guide me on that part ?

Thanks

[Atheist]

You need to keep a reference to the connected clients (at least their streams) in order to read from their NetworkStreams. Why do you not want to "store" the connected clients?

[akhileshbc]

You need to keep a reference to the connected clients (at least their streams) in order to read from their NetworkStreams. Why do you not want to "store" the connected clients?

Thanks for replying

My intention was to accept the data received on the server and use it for some calculation and other things. And not keep tracking of the clients.

So, I have to keep track of the clients inorder to work, right ?

[Atheist]

It sounds like you mean;
Having a class containing the logic for each connected client = keeping track of clients.
I would call it good design. However, do as you wish. There is no need to go with the class approach, just spawn a new thread when a client connects and read from the networkstream until its not needed anymore, then simply disconnect it.

[akhileshbc]

It sounds like you mean;
Having a class containing the logic for each connected client = keeping track of clients.
I would call it good design. However, do as you wish. There is no need to go with the class approach, just spawn a new thread when a client connects and read from the networkstream until its not needed anymore, then simply disconnect it.

Thanks

My idea:
Client will send data in the format :

[CONNECT]|field1|field2|field3|[CLOSE]

So, the field1, field2 and field3 is the actual data send to the server.

The idea of [CONNECT] and [CLOSE] was just for finding the start and end of the message.

So, when the received data contains the [CLOSE] command, that connection is terminated.

Am I going in the correct direction or making it a bit more complex ?

[Atheist]

So a client connects, sends 3 values like so:
field1|field2|field3
and then disconnects? Is this how it always will work?

Then I do not see any need for the [connect] and [close] messages.

[akhileshbc]

So a client connects, sends 3 values like so:
field1|field2|field3
and then disconnects? Is this how it always will work?

Then I do not see any need for the [connect] and [close] messages.

Yeah..!

The client will always sends 3 values and it has no purpose of hearing what the server has to say about these values or anything like that.

In simple, clients sends data. Server receives that data and saves it. No other information is passed back to the client. A one way approach.

Thanks

[Atheist]

May I ask what these values are?

[akhileshbc]

May I ask what these values are?

an id, a value corresponding to that particular id, a message

Example:
1, 100, bad

[Atheist]

I would go with a binaryreader/writer approach.
Each client would do something like so:

binaryWriter.Write(myByteValueID)
binaryWriter.Write(mySecondByteValue)
binaryWriter.Write(myStringMessage)

And on the server side;

Dim id as Byte = binaryReader.ReadByte()
Dim value as Byte = binaryReader.ReadByte()
Dim message as String = binaryReader.ReadString()
' Disconnect client.

This takes advantage of the fact that you know the size of the two first fields (I have assumed bytes here), which removes the need for delimiting tokens.

My original code treated everything as strings which is not always the most elegant thing.

[akhileshbc]

Thanks

That would be a good idea.

I'm having confusion on the server side.
Am I supposed to start reading the data just after accepting the connection ?

Private Sub doListen()
        Dim incomingClient As System.Net.Sockets.TcpClient
        Do
            incomingClient = listener.AcceptTcpClient 
            '~~~ here start reading the bytes
 
        Loop
    End Sub

Am I correct ?

[Atheist]

Yes thats correct. Wether or not you choose to use multithreading at that point or not determines if your server will be able to accept multiple clients simultaneously or not.

[akhileshbc]

Yes thats correct. Wether or not you choose to use multithreading at that point or not determines if your server will be able to accept multiple clients simultaneously or not.

Thanks

Here it goes...

Private Sub doListen()
        Dim incomingClient As System.Net.Sockets.TcpClient
        Do
            incomingClient = listener.AcceptTcpClient 'Accept the incoming connection. This is a blocking method so execution will halt here until someone tries to connect.
            Dim myBinReader As New BinaryReader(incomingClient.GetStream)
            Dim i As Byte = myBinReader.ReadByte
            Me.Text = i.ToString
        Loop
    End Sub

I didn't tested the above code.

Since it continuously loops, wouldn't it cause some issues ? I mean, even if a client is not connected, it would try to read the byte and process it !
This line:

incomingClient = listener.AcceptTcpClient

how can I make changes to the code, to avoid the continuous loop problem ?

[akhileshbc]

Am I doing correct here:

Private Sub doListen()
        Dim incomingClient As System.Net.Sockets.TcpClient
        Do
            incomingClient = listener.AcceptTcpClient 'Accept the incoming connection. This is a blocking method so execution will halt here until someone tries to connect.
 
            If incomingClient.Connected = True Then
                Dim myBinReader As New BinaryReader(incomingClient.GetStream)
                Dim i As Byte = myBinReader.ReadByte
                Me.Text = i.ToString
            End If
            incomingClient.Close()
        Loop
    End Sub

Thanks

[Atheist]

AcceptTcpClient is a blocking call. It will return once a client has connected. Of course, there is no guarantee that the client IS still connected once it reaches the reading part of your code, but such is life with networking.

[akhileshbc]

Thanks a lot for clearing my doubts...

Will play with the code and get in touch with you, if I have some doubts

[akhileshbc]

Of course, there is no guarantee that the client IS still connected once it reaches the reading part of your code, but such is life with networking.

Another doubt, did you mean that creating a new sub doRead for reading from the client (just like you did in the example) will overcome this issue ?

Or, it's good to continue using the one shot data reading with BinaryReader ?

Thanks

[Atheist]

No that won't make any difference. You can continue with the approach you showed in your last code snippet.

[akhileshbc]

No that won't make any difference. You can continue with the approach you showed in your last code snippet.

Ok.. Thanks...

Another question, I heard some people saying "be extremely cautious when opening ports". I'm confused !

Are they pointing to the TCPListener's port (in the server side) ?
How do we close it ?
Or it is automatically closed when the listener is closed ?

objTCPListner.Stop()

[Atheist]

I would assume they mean opening ports in your router.
Are you familiar with the concept of Network Address Translation (NAT)?

[akhileshbc]

Are you familiar with the concept of Network Address Translation (NAT)?

No.. I haven't heard about that yet.

So, there's no need to worry about the "opening of ports" right ?

Thanks

[Atheist]

When you call Listen on your TcpListener, you will immediately begin listening to your local port X (if your PC firewall doesnt block it). Other computers on your local network can connect to your computer by your local IP and port X.
The problem arises when a computer outside of your local network wants to connect. It is not possible for this outside computer to connect to your local IP for obvious reasons, so it must use your external IP. When the router receives the connection request from this outside computer it does not know what to do with it. Which computer inside your local network is this connection request directed to? This is where port forwarding ("opening" ports) come in.
You forward port Y on your router to your local IP and port X to make it accessible from outside hosts. This is what is considered "dangerous". Anyone could attempt to connect to your external IP on port Y.
Software that is poorly written might present security issues if a malicious client connects.

[akhileshbc]

When the router receives the connection request from this outside computer it does not know what to do with it. Which computer inside your local network is this connection request directed to? This is where port forwarding ("opening" ports) come in.
You forward port Y on your router to your local IP and port X to make it accessible from outside hosts. This is what is considered "dangerous". Anyone could attempt to connect to your external IP on port Y.
Software that is poorly written might present security issues if a malicious client connects.

Thanks for explaining...

Is this port forwarding facility done implicitly when we start the listening or it has to be done explicitly ?
My development PC is connected to a cable modem.

[Atheist]

It is done explicitly.
So you are not behind a router? In that case - no need to forward any ports.

[akhileshbc]

It is done explicitly.
So you are not behind a router? In that case - no need to forward any ports.

Thanks...

I did some testing and everything works fine..

Here's the code for server. Any improvements needed in the closing or disposing parts ?

Server:

'~~~ This sub will work in a background thread
    Private Sub doListen()
        Dim incomingClient As System.Net.Sockets.TcpClient
 
        Dim myBinReader As BinaryReader
 
        Dim bytHValue As Byte
        Dim strPid As String
 
        Do
            Try
                incomingClient = listener.AcceptTcpClient 'Accept the incoming connection. This is a blocking method so execution will halt here until someone tries to connect.
 
                myBinReader = New BinaryReader(incomingClient.GetStream)
                bytHValue = myBinReader.ReadByte
                strPid = myBinReader.ReadString
                myBinReader.Close()
 
                '~~~ Pass the messages from client for further processing
                MessageFromClient(bytHValue.ToString, strPid)
 
            Catch ex As Exception
                MessageBox.Show(ex.Message)
            End Try
        Loop
    End Sub
 
'..........
 
'~~~ start
    Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        listener = New System.Net.Sockets.TcpListener(System.Net.IPAddress.Any, 43001) 'The TcpListener will listen for incoming connections at port 43001
        listener.Start() 'Start listening.
        listenThread = New System.Threading.Thread(AddressOf doListen) 'This thread will run the doListen method
        listenThread.IsBackground = True 'Since we dont want this thread to keep on running after the application closes, we set isBackground to true.
        listenThread.Start() 'Start executing doListen on the worker thread.
    End Sub
'~~~ stop
    Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.Click
        listenThread.Abort()
        listener.Stop()
    End Sub

Client:

Public Class frmMain
    Private client As New System.Net.Sockets.TcpClient
 
    Dim strServerIP As String
    Dim intPort As Integer
 
    Private Sub btnConnect_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnConnect.Click
 
        strServerIP = txtServerIP.Text
 
        '~~~ Checking for valid inputs
        If strServerIP.Length = 0 Then
            MessageBox.Show("Please enter a valid IP address !", "IP address ?", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
            txtServerIP.Focus()
            Exit Sub
        End If
 
        If Integer.TryParse(txtPort.Text, intPort) = False Then
            MessageBox.Show("Please enter a valid port number !", "Port Number?", MessageBoxButtons.OK, MessageBoxIcon.Error)
            txtPort.Focus()
            Exit Sub
        End If
 
        '~~~ Connecting
        Try
            'client = New System.Net.Sockets.TcpClient(txtServerIP.Text, intPort)
            client.Connect(txtServerIP.Text, intPort)
            MessageBox.Show("Connected!")
        Catch ex As Exception
            MessageBox.Show(ex.Message, "Connection problem", MessageBoxButtons.OK, MessageBoxIcon.Error)
        End Try
    End Sub
 
    '~~~ Sends a message to the server
    Private Sub SendMessage(ByVal msg As String)
        Try
            If client.Connected = False Then
                client.Connect(strServerIP, intPort)
            End If
        Catch ex As Exception
            MessageBox.Show(ex.Message & vbNewLine & vbNewLine & "Try re-entering the IP address and port. Then connect to the server again.", "Connection Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
        End Try
 
        Try
            '~~~ Send the message to server
            Dim myBinWriter As New System.IO.BinaryWriter(client.GetStream)
            myBinWriter.Write(CByte(123)) '~~~ example
            myBinWriter.Write("123456")   '~~~ example
            myBinWriter.Close()
        Catch ex As Exception
            MessageBox.Show(ex.ToString, "Error on sending data", MessageBoxButtons.OK, MessageBoxIcon.Error)
        End Try
    End Sub
 
    Private Sub frmMain_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
        client.Close()
    End Sub
End Class

Thanks

[Atheist]

That looks alright as far as I can see!

[akhileshbc]

That looks alright as far as I can see!

Ok.. Thanks...