Please check this code snippet (network server stack)

[merendo]

Hello folks.

I have written a basic network server code, which can receive and store messages into memory, and only release them for further processing once they are deemed to be fully received. Can you please take a look at this code and make comments as to it's reliability, possible sources of trouble or even stuff I'm doing which can be safely omitted. Thanks!

Code: Select all

; I'm assuming at this point that the server is already running - that much is easy.

#SERVER_RECEPTION_LENGTH = 1024 ; How many bytes to receive per round
#NETWORKMESSAGE_TIMEOUT = 5000 ; Timeout for non-finalised messages
#NETWORKMESSAGE_SEPARATOR = Chr(254) ; Separator character between messages.

Structure NetworkMessage ; Contains the received network messages.
	Sender.l ; Holds the value returned by EventClient()
	Content.s ; The actual content of the message
	Finalised.b ; #True if the message is fully received, #False if more data is waiting.
	Timeout.l ; When the message isn't yet Finalised, this will hold an ElapsedMilliseconds() timestamp. When a message hasn't been updated for #NETWORKMESSAGE_TIMEOUT milliseconds, it gets discarded.
EndStructure

Global NewList NetworkMessages.NetworkMessage()
Global *server_reception_buffer = AllocateMemory(#SERVER_RECEPTION_LENGTH)

Procedure Server_HandleNetworkEvents()
	Protected ServerEvent = NetworkServerEvent(), EventClient, messagematch.b, server_reception_length.w

	; Delete old, non-finalised network messages.
	ForEach NetworkMessages()
		If NetworkMessages()\Finalised = #False And ElapsedMilliseconds()-NetworkMessages()\Timeout > #NETWORKMESSAGE_TIMEOUT
			DeleteElement(NetworkMessages())
		EndIf
	Next
	
	
	If ServerEvent
	  EventClient = EventClient()
	
	  Select ServerEvent
	     Case #PB_NetworkEvent_Connect
	        ; No code yet...
	
	     Case #PB_NetworkEvent_Data
	     
	     		; Check if a non-finalised message by this Client is already present.
	     		messagematch = 0
	     		ForEach NetworkMessages()
	     			If NetworkMessages()\Sender = EventClient And NetworkMessages()\Finalised = #False
	     				messagematch = #True
	     				NetworkMessages()\Timeout = ElapsedMilliseconds()
	     				Break
	     			EndIf
	     		Next
	     		If messagematch = 0 ; If no non-finalised message by this Client is present, create a new one.
	     			AddElement(NetworkMessages())
	     			NetworkMessages()\Sender = EventClient
	     			NetworkMessages()\Timeout = ElapsedMilliseconds()
	     		EndIf
	     
	        Repeat ; Keep looping until all data has been received
	          server_reception_length = ReceiveNetworkData(EventClient, *server_reception_buffer, #SERVER_RECEPTION_LENGTH)
	          If server_reception_length > 0
	            NetworkMessages()\Content = NetworkMessages()\Content + PeekS(*server_reception_buffer, server_reception_length)
	            
	            If Right(NetworkMessages()\Content, 1) = #NETWORKMESSAGE_SEPARATOR ; If the separator has been found at the end of the message, finalise the message.
	            	NetworkMessages()\Finalised = #True
	            	NetworkMessages()\Content = RTrim(NetworkMessages()\Content, #NETWORKMESSAGE_SEPARATOR)
	            EndIf
	          EndIf
	          
	          If server_reception_length = -1 ; Seems something went wrong...
	          	LogMessage("Error in network message reception.", #LOGLEVEL_ERROR)
	          EndIf
	        Until Not server_reception_length = #SERVER_RECEPTION_LENGTH
	     
	     Case #PB_NetworkEvent_Disconnect
	        ; No code yet...
	     
	  EndSelect
	EndIf
EndProcedure

At this point, a question arises: What happens if a server receives multiple messages by multiple clients inbetween invidual event queries? Do their messages get mixed up and all written in the same buffer, or does each client which is connected to the server have it's own reception buffer on the server, so that no messages by different clients can ever get mixed up?

If I keep querying ReceiveNetworkData(), can it be said with absolute certainty that I will only receive data sent by the client previously identified with EventClient(), or is it possible, that every call to RecieveNetworkData() may give me data sent by a different client?!

Sorry for making something seemingly so easy so difficult, but I really want my reception stack to be reliable and as flawless as possible. A lot is at stake...

[RichAlgeni]

Without the rest of your code, it's difficult to make a judgement as to how well this code will run, but from first glance, it looks like it should be ok. Since you appear to be writing a single threaded process, there shouldn't be a problem receiving messages in the wrong order, as that is what TCP is built to handle. The caveat with this is that messages should be sent in one packet. You could increase your message read length from 1024 to as high as 65535, which could cut down on the number of loops needed to handle your reads. The drawback with this would be that you are allocating much more memory, but with today's systems, it shouldn't really be a problem.

It CAN be said with certainty that you will only receive data for an individual event client. Only after you re-interrogate NetworkServerEvent() and EventClient() will the client change.

After your clients send their data, will they disconnect? If not ReceiveNetworkData will block until more data is received, or the client disconnects. This means that your program will effectively stop until one of those two situations occur. Just something to keep in mind.

Will your clients ever send multiple messages at one time? From your code, it looks like they won't, which is an easier situation to program. Will your clients be sending from multiple sources? If so, you can use 'IPString(GetClientIP(EventClient())) to get the source IP address, then save your messages based upon this address.

Test your code to make sure it handles one client correctly first. Note that if the clients sending you data are not on the same subnet (basic location), it may take several reads to gather your data. Once you have a single client working correctly, then hit it with multiple connections at a single time.

Keep in mind that there are ways to tell how much data is in the read buffer prior to calling ReceiveNetworkData(), let me know if you'd like to look at that code.

Good luck!

Rich Algeni

[merendo]

First of all, thank you very much for your detailled answer, Rich!

I really burnt my hands last time I tried to mess with threaded server applications, so I'll stay a safe distance away from those. I did now increase the reception buffer size to 65535, which really won't make a difference on a machine with 4 GBytes of memory or more. Btw, I noticed the manual says that the maximum message length is 65536 and not 65535 as most people would expect. That's exactly 2 to the 16th, but that would imply that a message can never have a zero length, is that correct?

No, the clients will remain in a steady connection and send and receive data every here and there. Are you sure ReceiveNetworkData() would block until more data is available for reception? I have never seen indications of such a program halt occurring.

My clients may send multiple messages in brief consecution, and I will modify my code to suit this. As I noiced, what was sent as one single message on the client side need not necessarily arrive as just one single message on the server, especially if there is considerable network latency (which there is, in my case). What exactly do you mean, send from multiple sources? Of course, there will be several clients, but I thought identifiying them using EventClient() would be sufficient.

I'll post my updated code in a moment.

Oh, as for my concept of using a single character as a message separator - is that a good idea or are there more elegant ways of handling this?

[merendo]

Code: Select all

    ; I'm assuming at this point that the server is already running - that much is easy.

    #SERVER_RECEPTION_LENGTH = 65535 ; How many bytes to receive per round
    #NETWORKMESSAGE_TIMEOUT = 5000 ; Timeout for non-finalised messages
    #NETWORKMESSAGE_SEPARATOR = Chr(254) ; Separator character between messages.

    Structure NetworkMessage ; Contains the received network messages.
       Sender.l ; Holds the value returned by EventClient()
       Content.s ; The actual content of the message
       Finalised.b ; #True if the message is fully received, #False if more data is waiting.
       Timeout.l ; When the message isn't yet Finalised, this will hold an ElapsedMilliseconds() timestamp. When a message hasn't been updated for #NETWORKMESSAGE_TIMEOUT milliseconds, it gets discarded.
    EndStructure

    Global NewList NetworkMessages.NetworkMessage()
    Global *server_reception_buffer = AllocateMemory(#SERVER_RECEPTION_LENGTH)

    Procedure Server_HandleNetworkEvents()
       Protected ServerEvent = NetworkServerEvent(), EventClient, messagematch.b, server_reception_length.w

       ; Delete old, non-finalised network messages.
       ForEach NetworkMessages()
          If NetworkMessages()\Finalised = #False And ElapsedMilliseconds()-NetworkMessages()\Timeout > #NETWORKMESSAGE_TIMEOUT
             DeleteElement(NetworkMessages())
          EndIf
       Next
       
       
       If ServerEvent
         EventClient = EventClient()
       
         Select ServerEvent
            Case #PB_NetworkEvent_Connect
               ; No code yet...
       
            Case #PB_NetworkEvent_Data
            
                  ; Check if a non-finalised message by this Client is already present.
                  messagematch = 0
                  ForEach NetworkMessages()
                     If NetworkMessages()\Sender = EventClient And NetworkMessages()\Finalised = #False
                        messagematch = #True
                        NetworkMessages()\Timeout = ElapsedMilliseconds()
                        Break
                     EndIf
                  Next
                  If messagematch = 0 ; If no non-finalised message by this Client is present, create a new one.
                     AddElement(NetworkMessages())
                     NetworkMessages()\Sender = EventClient
                     NetworkMessages()\Timeout = ElapsedMilliseconds()
                  EndIf
            
	        Repeat ; Keep looping until all data has been received
	          server_reception_length = ReceiveNetworkData(EventClient, *server_reception_buffer, #SERVER_RECEPTION_LENGTH)
						If server_reception_length > 0
							num_messages = CountString(PeekS(*server_reception_buffer, server_reception_length), #NETWORKMESSAGE_SEPARATOR)+1
						
							; The received message doesn't contain the separator character, so it only contains a fragment of a larger message.
							If num_messages = 1
								NetworkMessages()\Content + PeekS(*server_reception_buffer, server_reception_length)
							Else
								For x = 1 To num_messages ; The received message contains more than one message. Iterate every single one of them.
									NetworkMessages()\Content + StringField(PeekS(*server_reception_buffer, server_reception_length), x, #NETWORKMESSAGE_SEPARATOR) ; Append the content of this stringfield to the existing message
									
									If Not x = num_messages ; and finalise this message, then add a new message, if one more is to come.
										NetworkMessages()\Finalised = #True
					     			AddElement(NetworkMessages())
					     			NetworkMessages()\Sender = EventClient
					     			NetworkMessages()\Timeout = ElapsedMilliseconds()
					     		 Else
					     		 	If StringField(PeekS(*server_reception_buffer, server_reception_length), x, #NETWORKMESSAGE_SEPARATOR) = "" ; Message is also finalised.
					     		 		NetworkMessages()\Finalised = #True
					     		 	EndIf
									EndIf
								Next
							EndIf
							
						EndIf
	          
	          If server_reception_length = -1 ; Seems something went wrong...
	          	debug "Error in network message reception."
	          EndIf
	        Until Not server_reception_length = #SERVER_RECEPTION_LENGTH
            
            Case #PB_NetworkEvent_Disconnect
               ; No code yet...
            
         EndSelect
       EndIf
    EndProcedure

[RichAlgeni]

I'll try to answer your concerns in the order in which you wrote them. I don't want to scare you about threads, but it is much easier to move to threads once you have the basics down for a single thread. There's always an issue when talking numbers, since many instances start with 0, like dimensioning and array. In the case of receiving data from a socket, you will never 'receive' a 0 length packet. The number 65535 is stuck on my head, as in 0 - 65535, so that's what I use, but you are correct, you can use 65536.

I thought ReceiveNetworkData() would block, so I found from these forums a workaround:

Code: Select all

Procedure.i NetworkRead(connectNumber.i, *memoryLoc, memLength.i, *netSource, debugThis.i)

    Protected attemptCount.i = 0
    Protected amountRead.i   = 0
    Protected socketHandle.i
    Protected result.i
    Protected length.i

    socketHandle = ConnectionID(connectNumber)

; loop until we have received data, or our timeout has been exceeded

    Repeat
        result = ioctlsocket_(socketHandle, #FIONREAD, @length)
        If result < 0; socket error
            amountRead = result
            Break
        ElseIf result > 0; socket error
            amountRead = result * -1
            Break
        ElseIf length > 0; we have data in the receive buffer
            If  length > memLength
                length = memLength
            EndIf
            While length
                result     = ReceiveNetworkData(connectNumber, *memoryLoc + amountRead, length)
                amountRead = amountRead + result
                length     = length     - result
            Wend
            Break
        Else
            Delay(PortTimeout)
            AttemptCount = AttemptCount + 1
            If AttemptCount > MaxAttempts; if still nothing received, just get out
                Break
            EndIf
        EndIf
    ForEver

; send the data we have received to our debugger listing, if needed

    If debugThis
        DebugString(*memoryLoc, amountRead, *netSource, debugIn)
    EndIf

    ProcedureReturn amountRead

EndProcedure

If result from ioctlsocket_(socketHandle, #FIONREAD, @length) is not 0, then an error occurred. If length is not 0, then there is data to be read in the socket's read buffer. PortTimeout and AttemptCount are global variables that I use to control the read attempts and timeout. *netSource and debugIn are variables that I use when I send data to a debug listing. I would suggest that you create something like this so write the data you received to a file or listing, when the inevitable 'what the hell is that' moment occurs. If you use non-ascii characters like char(254), or character values less than 31, convert them to a human readable format, for instance char(10) = '<lf>'.

When I asked 'from multiple sources', I meant multiple different machines at different locations. Each location in this case would have it's own network latency issues, that's all.

Using a 1 character delimiter is fine, everyone has their own way of parsing data. There are low order byte characters such as <stx>, <etx>, <gs>, <fs>, <rs> and <us> that you could use as well. It's entirely up to you.

Other than that, it looks like you have a fairly good grasp on what you want to accomplish. As you get more familiar and comfortable with PB, try breaking out your code into small consistent procedures, like I have above. Once you know a procedure works correctly, you no longer have to worry about it when something goes wrong. Make it as generic and compact as possible. Group like procedures into 'pbi' include files. Then, the more code you write, the less code you have to write, as you will be able to just 'include' and call these existing procedures.