I have created a class which wraps the TcpClient. It's greatly simplified everything when developing.
It takes an interface for me to run callbacks where I can control what I want to do based on the network message and/or based on disconnections and connections, this makes it less hardcoded for a particular project which has been helpful when using it in other projects.
I am new to networking, and plan to try my hand at using Sockets which is a bit lower level, but wanted to start on TcpClient since it was easier.
I want to know if I have any design flaws or non robust setup which could be problematic in future.
public abstract class SocketManager
{
// call back interface [see below class for example]
private readonly iNetworkEvents _netEvent;
// for sending keep alives, use seperate array
// this to avoid conflicts with actual messages
protected readonly byte[] Empty = new byte[1];
private readonly byte[] _readBuffer;
public readonly byte[] WriteBuffer;
public bool IsConnected { protected set; get; }
protected TcpClient TcpClient;
protected int KeepAliveRateMilliseconds;
protected Stream Stream;
private bool lockPackets;
protected bool secure;
protected SocketManager(int bufferSize, iNetworkEvents callback)
{
_readBuffer = new byte[bufferSize];
WriteBuffer = new byte[bufferSize];
_netEvent = callback;
}
protected async Task<bool> Connect(string address, int port)
{
if (IsConnected) return true;
// have to create new every time since we have to dispose when we disconnect
TcpClient = new TcpClient();
try
{
_netEvent.Connected(secure);
await TcpClient.ConnectAsync(address, port);
IsConnected = true;
return true;
}
catch (ObjectDisposedException e)
{
Disconnect();
return false;
}
catch (SocketException)
{
Disconnect();
return false;
}
}
protected async Task RunKeepAlives()
{
while (IsConnected && lockPackets == false)
{
Empty[0] = 0;
await Send(Empty,1);
await Task.Delay(KeepAliveRateMilliseconds);
}
}
public async Task<bool> Send()
{
if (!IsConnected)
return false;
return await Send(WriteBuffer, WriteBuffer[0] + 1);
}
private async Task<bool> Send(byte[] msg, int length)
{
if (!IsConnected)
{
return false;
}
if (msg == null) return false;
try
{
await Stream.WriteAsync(msg, 0, length);
return true;
}
catch (ObjectDisposedException e)
{
Disconnect();
return false;
}
}
private async Task<int> Read(byte[] readBuffer)
{
if (!IsConnected)
{
return 0;
}
try
{
await Stream.ReadAsync(readBuffer, 0, 1);
return await Stream.ReadAsync(readBuffer, 1, readBuffer[0]);
}
catch (ObjectDisposedException e)
{
Disconnect();
return 0;
}
}
protected async Task _RunReader()
{
var totalBytes = await Read(_readBuffer);
if (totalBytes > 0)
{
HandleReader();
}
}
private void HandleReader()
{
if (Enum.IsDefined(typeof(NetworkMessage), _readBuffer[1])) // valid network message was recieved
{
switch (_readBuffer[1])
{
case (byte)NetworkMessage.Disconnect:
Disconnect();
_netEvent.Disconnect(secure); // run the callback for custom callbacks
return;
default:
_netEvent.MessageRecieved((NetworkMessage)_readBuffer[1], _readBuffer, 2);
break;
}
}
_RunReader(); //start reading the next message
}
public async Task Disconnect(bool graceful = false)
{
if (IsConnected && graceful)
{
Empty[0] = (byte)NetworkMessage.Disconnect;
await Send(Empty,1);
}
// we lock the keep alive packets to prevent one being sent after disposal of stream in rare cases
lockPackets = true;
Stream?.Dispose();
TcpClient.Close();
IsConnected = false;
_netEvent.Disconnect(secure);
}
}
public class TcpClientSecureSocket : SocketManager
{
private readonly X509CertificateCollection _collection;
public TcpClientSecureSocket(int bufferSize, X509CertificateCollection collection, iNetworkEvents callback) : base(bufferSize,callback)
{
secure = true;
_collection = collection;
}
public async Task<bool> Connect(string address, int port, int keepAliveRate)
{
bool result = await Connect(address, port);
if (result)
{
var networkStream = TcpClient.GetStream();
Stream = new SslStream(networkStream,false,ValidateCert);
SslStream stream = (SslStream) Stream;
await stream.AuthenticateAsClientAsync(address, _collection, SslProtocols.Tls12, false);
_RunReader();
if (keepAliveRate > 0)
{
KeepAliveRateMilliseconds = keepAliveRate;
RunKeepAlives();
}
}
return result;
}
private bool ValidateCert(object sender, X509Certificate x509Certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
{
return true; //todo get CA
}
}
public class TCPClientSocket : SocketManager
{
public TCPClientSocket(int bufferSize, iNetworkEvents callback) : base(bufferSize, callback){}
public async Task<bool> Connect(string address, int port, int keepAliveRate)
{
bool result = await Connect(address, port);
if (result)
{
Stream = TcpClient.GetStream();
_RunReader();
if (keepAliveRate > 0)
{
KeepAliveRateMilliseconds = keepAliveRate;
RunKeepAlives();
}
}
return result;
}
}
}
Here is usage of the class for initialisation:
var certificate = X509Certificate.CreateFromCertFile(certificateDir);
certificateCollection = new X509CertificateCollection(new[] { certificate });
ClientNonSecure = new TCPClientSocket(255, netEvents);
ClientTcpClientSecureSocket = new TcpClientSecureSocket(255, certificateCollection, netEvents);
With an example of the netEvents looking like this:
public interface iNetworkEvents
{
void MessageRecieved(NetworkMessage networkMessage, byte[] message, int startIndex);
void Disconnect(bool secure);
void Connected(bool secure);
}
public class NetEvents : iNetworkEvents
{
public void MessageRecieved(NetworkMessage networkMessage, byte[] message, int startIndex)
{
switch (networkMessage)
{
// example message
case NetworkMessage.LoginSuccess:
break;
default:
break;
}
}
public void Disconnect(bool secure)
{
}
public void Connected(bool secure)
{
}
}
If the code is looking good I'll next move on to using the Sockets class so I can support udp as well but want to make sure I have robust code first.
IDisposable-implementing type (TcpClient), it needs to also implementIDisposable. \$\endgroup\$IDisposableobject, it should implement theIDisposableinterface itself to ensure proper, deterministic disposal of those types. This has been framework design guidelines since 2003. stackoverflow.com/a/898867/3312 might help. \$\endgroup\$.Dispose()calls in theDisconnect()method are always reached for every call toConnect(), then you don't need to do that. Otherwise, implement the interface to allow your callers to dispose the object. \$\endgroup\$