HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Implementation of an asynchronous TCP/UDP server

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asynchronousudptcpserverimplementation

Problem

I am trying to implement a TCP/UDP server so all I have to do is something like this:

var server = new Server(Type.UDP, "127.0.0.1", 8888);
server.OnDataRecieved += Datahandler;
server.Start();


I have tried to make it perform as fast as possible by using Asynchronous calls where possible.

I basically would like to know if there is anything missing/any changes that people would recommend (and why). It is not finished yet as I need to handle exceptions better etc...

TODO: I need to complete the signature of the events to make them more meaningful, etc.

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace SBlackler.Networking
{
public sealed class HighPerformanceServer
{
private Int32 _currentConnections = 0;
Socket listener;
EndPoint ipeSender;

#region "Properties"

public Int32 Port { get; set; }
public Int32 CurrentConnections { get { return _currentConnections; } }
public Int32 MaxQueuedConnections { get; set; }
public IPEndPoint Endpoint { get; set; }
public ServerType Type { get; set; }

#endregion

#region "Constructors"

private HighPerformanceServer()
{
// do nothing
}

public HighPerformanceServer(ServerType type, String IpAddress)
{
Init(type, IpAddress, 28930);
}

public HighPerformanceServer(ServerType type, String IpAddress, Int32 Port)
{
Init(type, IpAddress, Port);
}

private void Init(ServerType server, String IpAddress, Int32 Port)
{
IPAddress ip;
// Check the IpAddress to make sure that it is valid
if (!String.IsNullOrEmpty(IpAddress) && IPAddress.TryParse(IpAddress, out ip))
{
this.Endpoint = new IPEndPoint(ip, Port);
// Make sure that the port is greater than 100 as not to conflict with any other programs
if (Port OnServerStart;
public ev

Solution

At a quick glance, there are a couple minor things I noticed regarding how you handle your events:

-
You are passing null event args. I would instead use EventArgs.Empty, as callers will typically assume the EventArgs object they get from the event handler will be non-null.

-
You are using Interlocked.Increment on your connection counter, suggesting you are going to be using this in multi-threaded code.

As such, you should note that

if (OnClientConnected!= null)
{
   OnClientConnected (this, null);
}


is not thread-safe. Instead, you will want to do something more like the following:

var  evt = OnClientConnected;
if (evt != null)
{
   evt (this, EventArgs.Empty);
}


I would suggest converting all your internal members to private, unless there is a specific need for other classes to access them, which seems unlikely, given their content.

Additionally, if SocketConnectionInfo.BufferSize is >= 0, then

if (bytesRead == 0 || (bytesRead > 0 && bytesRead < SocketConnectionInfo.BufferSize))


can be converted to

if (bytesRead < SocketConnectionInfo.BufferSize)

Code Snippets

if (OnClientConnected!= null)
{
   OnClientConnected (this, null);
}
var  evt = OnClientConnected;
if (evt != null)
{
   evt (this, EventArgs.Empty);
}
if (bytesRead == 0 || (bytesRead > 0 && bytesRead < SocketConnectionInfo.BufferSize))
if (bytesRead < SocketConnectionInfo.BufferSize)

Context

StackExchange Code Review Q#7226, answer score: 8

Revisions (0)

No revisions yet.