I'm writing a TCP server using tasks and I wanted to know if there's something I can improve and how I am doing.
ConnectionHandler.cs:
public class ConnectionHandler
{
private List<TClient> CLIENT_LIST;
private TcpListener LISTENER;
private const int PORT = 8080;
private const int ACCEPT_DELAY = 75;
public bool IsListening => LISTEN_FLAG;
private bool LISTEN_FLAG = false;
public ConnectionHandler()
{
LISTENER = new TcpListener(IPAddress.Any, PORT);
CLIENT_LIST = new List<TClient>();
}
public void StartListeningAsync()
=> Task.Run((Action)StartListening);
public void StartListening()
{
LISTEN_FLAG = true;
LISTENER.Start();
while (LISTEN_FLAG)
{
Task.Run(() => CreateConnectionAsync(LISTENER.AcceptSocket()));
Thread.Sleep(ACCEPT_DELAY);
]
LISTENER.Stop();
CLIENT_LIST.Clear();
}
public void StopListening()
=> LISTEN_FLAG = false;
private void CreateConnectionAsync(Socket connectionSocket)
=> Task.Run(() => CreateConnection(connectionSocket));
private void CreateConnection(Socket connectionSocket)
{
TClient CLIENT = new TClient(connectionSocket);
CLIENT.DisconnectEvent += new TClient.ClientEvent(DisconnectEventHanlder);
CLIENT_LIST.Add(CLIENT);
}
private void DisconnectEventHanlder(TClient sender)
=> CLIENT_LIST.Remove(sender);
}
Here I start to accept the incoming connections, then I create a TClientclass, add it to a List and add an event for the client disconnection.
TClient.cs:
public class TClient
{
public delegate void Event();
public delegate void DataEvent(IPacket data);
public delegate void ClientEvent(TClient client);
private int _SessionID;
private bool _IsAvailable = true;
private string _IPAdress;
private Socket _connectionSocket;
public event ClientEvent DisconnectEvent;
public event DataEvent ProcessDataEvent;
public event DataEvent SendDataEvent;
public Socket ConnectionSocket => _connectionSocket;
public string IPAdress => _IPAdress;
public int SessionID => _SessionID;
public bool IsAvailable => _IsAvailable;
private PacketHandler PacketHandler;
private PacketProcessor PacketProcessor;
private ConnectionMaintenance Maintenance;
public int ProcessedCorruptedPackets => PacketProcessor.CorruptedPackets;
public int ReceivedCorruptedPackets => PacketHandler.CorruptedPackets;
public int ReceivedData => PacketHandler.ReceivedData;
public int ReceivedPackets => PacketHandler.ReceivedPackets;
public TClient(Socket connection)
{
_SessionID = SessionFactory.GenerateUniqueSession();
_connectionSocket = connection;
_IPAdress = connection.RemoteEndPoint.ToString().Split(':')[0];
PacketHandler = PacketHandler.CreateHandler(this);
PacketProcessor = PacketProcessor.CreateProcessor(this);
Maintenance = ConnectionMaintenance.CreateConnectionMatinance(this);
}
public void Disconnect()
{
_connectionSocket.Close();
_IsAvailable = false;
DisconnectEvent(this);
}
public void ProcessData(IPacket data)
=> ProcessDataEvent(data);
public void SendData(IPacket data)
=> SendDataEvent(data);
public void KeepAlive() =>
Maintenance.AliveTime = 0;
}
This is were I store the socket and the other connection information, such as the IP address, session ID, other classes, and the events for sending data, and processing received data.
PacketHandler.cs:
public class PacketHandler
{
private const short MAX_DATA_LENGTH = 1024;
private const short LOOP_DELAY = 9;
public int ReceivedPackets => _receivedPackets;
public int ReceivedData => _receivedData;
public int CorruptedPackets => _corruptedPackets;
private int _corruptedPackets;
private int _receivedPackets;
private int _receivedData;
private TClient CLIENT;
public static PacketHandler CreateHandler(TClient client) => new PacketHandler(client);
private PacketHandler(TClient client)
{
CLIENT = client;
CLIENT.SendDataEvent += new TClient.DataEvent(SendPacket);
Task.Run(() => ReceiveDataLoop(CLIENT));
}
private void ReceiveDataLoop(TClient CLIENT)
{
while (CLIENT.IsAvailable)
{
try
{
ReceivePacket();
}
catch
{
CLIENT.Disconnect();
}
Thread.Sleep(LOOP_DELAY);
}
}
private void SendPacket(IPacket data)
{
CLIENT.ConnectionSocket.Send(BitConverter.GetBytes((short)data.Length));
CLIENT.ConnectionSocket.Send(data.PacketData);
}
private void ReceivePacket()
{
byte[] lengthHeader = new byte[2];
CLIENT.ConnectionSocket.Receive(lengthHeader);
ushort DataLength = BitConverter.ToUInt16(lengthHeader, 0);
byte[] PacketHeader = new byte[1];
CLIENT.ConnectionSocket.Receive(PacketHeader);
byte[] data = new byte[DataLength];
CLIENT.ConnectionSocket.Receive(data);
if (data.Length > MAX_DATA_LENGTH) _corruptedPackets++;
else
{
CLIENT.ProcessData(new IPacket(PacketHeader[0], data));
_receivedData += DataLength;
_receivedPackets++;
}
}
}
}
Here is were the data is received and sent. First I receive Ushort that indicates the length of the data received, then a byte that indicates what will the program have to when it's processed. This data is then passed to an IPacket.
public struct IPacket
{
private byte _packetHeader;
private byte[] _packetData;
public byte[] PacketData => _packetData;
public byte PacketHeader => _packetHeader;
public int Length => _packetData.Length;
public IPacket(byte _header, byte[] _data)
{
_packetData = _data;
_packetHeader = _header;
}
}
Then this is converted to an IMessage:
public enum Header
{
UNKNOWN = 0,
CORRUPTED = 1,
KEEP_ALIVE = 2
}
public struct IMessage
{
private IPacket basePacket;
private Header _header;
public string DataString => Encoding.UTF8.GetString(basePacket.PacketData);
public Header Header => GetHeader();
public IMessage(IPacket _basePacket)
{
basePacket = _basePacket;
_header = Header.UNKNOWN;
}
private Header GetHeader()
{
if (_header != Header.UNKNOWN)
return _header;
if (basePacket.PacketHeader == 0)
return Header.KEEP_ALIVE;
else return Header.CORRUPTED;
}
}
This is what will process the messages:
public class PacketProcessor
{
public int CorruptedPackets => _corruptedPackets;
private int _corruptedPackets;
private TClient CLIENT;
public static PacketProcessor CreateProcessor(TClient client) => new PacketProcessor(client);
private PacketProcessor(TClient client)
{
CLIENT = client;
CLIENT.ProcessDataEvent += new TClient.DataEvent(ProcessMessage);
}
private void ProcessMessage(IPacket Packet)
{
IMessage message = new IMessage(Packet);
if (message.Header == Header.KEEP_ALIVE)
CLIENT.KeepAlive();
else if (message.Header == Header.CORRUPTED)
_corruptedPackets++;
}
}
Then the connection is checked for exessive data flow, corrupted packets and no keep alive here:
public class ConnectionMaintenance
{
public int AliveTime { get; set; }
private const short LOOP_DELAY = 250;
private const short MAX_DATA_PER_TICK = 960;
private const short MAX_PACKETS_PER_TICK = 32;
private const short MAX_CORRUPTED_PACKETS = 50;
private const short MAX_ALIVE_MS = 15000;
private TClient CLIENT;
public static ConnectionMaintenance CreateConnectionMatinance(TClient client) => new ConnectionMaintenance(client);
private ConnectionMaintenance(TClient client)
{
CLIENT = client;
new Task(MaintenanceLoop).Start();
}
private void MaintenanceLoop()
{
int lastReceivedData = 0;
int lastReceivedPackets = 0;
while (CLIENT.IsAvailable)
{
if (CLIENT.ProcessedCorruptedPackets >= MAX_CORRUPTED_PACKETS
|| CLIENT.ReceivedCorruptedPackets >= MAX_CORRUPTED_PACKETS)
CLIENT.Disconnect();
else if ((CLIENT.ReceivedData - lastReceivedData) >= MAX_DATA_PER_TICK)
CLIENT.Disconnect();
else if ((CLIENT.ReceivedPackets - lastReceivedPackets) >= MAX_PACKETS_PER_TICK)
CLIENT.Disconnect();
else if (AliveTime >= MAX_ALIVE_MS)
CLIENT.Disconnect();
else
{
AliveTime += LOOP_DELAY;
lastReceivedData = CLIENT.ReceivedData;
lastReceivedPackets = CLIENT.ReceivedPackets;
}
Thread.Sleep(LOOP_DELAY);
}
}
}
I am worried about the structure of the program. Should I be using the async methods provided by the TCP classes?
I'm trying to make everything like "modules" so if I can update the code without affecting much each other "module". Then I'm using a lot of private variables with public get accesors. Is this worth it? Or should I just use public variables?
As I'm trying to keep things "modular", it doesn't feel right, since everything is dependent on TClient and the other classes are interconnected through it.
