patterncsharpMinor
Implementation of an asynchronous TCP/UDP server
Viewed 0 times
asynchronousudptcpserverimplementation
Problem
I am trying to implement a TCP/UDP server so all I have to do is something like this:
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
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
is not thread-safe. Instead, you will want to do something more like the following:
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
can be converted to
-
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.