Multithreaded client server socket

Question

I have created this library mostly for a learning experience with sockets and threading.

For this review, focus on socket/threading. Let me know if I properly implemented both. If you want to comment on other part go ahead I'm open to any comment.

I have read this for the threading code.

I'm using lock() and ManualResetEvent to handle the threading part.

I am not using the SocketAsyncEventArgs pattern but I did read this to figure out how to play with socket.

On that page you can read this:

1. On the first receive op, receive less bytes than the length of the prefix.
2. After having the received part of the prefix on a previous receive op or ops, then receive another part of the prefix, but not all of it.
3. After having received part of the prefix on a previous receive op or ops, then receive the rest of the prefix, but nothing more.
4. After having received part of the prefix on a previous receive op or ops, we then receive the rest of it, plus part of the message.
5. After having received part of the prefix on a previous receive op or ops, we then receive the rest of it, plus all of the message.
6. Receive exactly the number of bytes that are in the prefix, but nothing more.
7. After having received exactly the number of bytes that are in the prefix on a previous receive op or ops, we then receive part of the message.
8. After having received exactly the number of bytes that are in the prefix on a previous receive op or ops, we then receive all of the message.
9. Receive the number of bytes for the prefix plus part of the message, but not all of the message.
10. After having received the prefix and part of the message on a previous receive op or ops, we then receive another part of the message, but not all of it.
11. After having received the prefix and part of the message on a previous receive op or ops, we then receive all the rest of the message.
12. Receive the number of bytes for the prefix plus all of the message on the first receive op.

I think I have completed all of them.

You can find the full code with a test project here (GitHub).

Server code:

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

namespace AsyncClientServer
{
    public sealed class Server
    {
        private Socket listener;
        private Stack<int> nextClientId;
        private int maxid;
        private bool isServerRunning;
        private Dictionary<int, Client> clients;

        private ManualResetEvent mreBeginAccept;

        public delegate void ConnectedHandler(int ClientId);
        public event ConnectedHandler Connected;
        internal void RaiseConnected(int ClientId)
        {
            var handler = Connected;
            if (handler != null)
            {
                handler(ClientId);
            }
        }

        public delegate void MessageReceivedHandler(int ClientId, byte[] msg, KindMessage kindOfSend);
        public event MessageReceivedHandler MessageReceived;
        internal void RaiseMessageReceived(int ClientId, byte[] msg, KindMessage kindOfSend)
        {
            if (kindOfSend == KindMessage.Message)
            {
                var handler = MessageReceived;
                if (handler != null)
                {
                    handler(ClientId, msg, kindOfSend);
                }
            }
            else if (kindOfSend == KindMessage.ListClientId)
            {
                GetClient(ClientId).SendBytes(clients.Keys.ToArrayOfByte(),KindMessage.ListClientId);
            }
        }

        public delegate void MessageSentHandler(int ClientId);
        public event MessageSentHandler MessageSent;
        internal void RaiseMessageSent(int ClientId)
        {
            var handler = MessageSent;
            if (handler != null)
            {
                handler(ClientId);
            }
        }

        public delegate void DisconnectedHandler(int ClientId);
        public event DisconnectedHandler Disconnected;
        internal void RaiseDisconnected(int ClientId)
        {
            var handler = Disconnected;
            if (handler != null)
            {
                handler(ClientId);
            }
        }

        public delegate void SocketErrorHandler(Client client, Exception e);
        public event SocketErrorHandler SocketError;
        internal void RaiseSocketError(Client client, Exception e)
        {
            var handler = SocketError;
            if (handler != null)
            {
                handler(client, e);
            }
        }

        public Server()
        {
            isServerRunning = false;
            clients = new Dictionary<int, Client>();
            nextClientId = new Stack<int>();
            nextClientId.Push(maxid);
        }

        public void StopServer()
        {
            if (isServerRunning)
            {
                isServerRunning = false;
                mreBeginAccept.SetIfNotNull();
                CloseAll();
            }
        }

        public void StartServer(string address, int port)
        {
            if (!isServerRunning)
            {
                isServerRunning = true;

                var ip = new IPAddress(address.Split('.').Select(x => Convert.ToByte(x)).ToArray());
                var endpoint = new IPEndPoint(ip, port);

                listener = MiscOperation.NewSocket();

                listener.Bind(endpoint);
                listener.Listen(Const.BackLogLimit);

                new Thread(this.BeginAccept).Start();
            }
        }

        private void BeginAccept()
        {
            using(mreBeginAccept = new ManualResetEvent(false))
            {
                while (isServerRunning)
                {
                    mreBeginAccept.Reset();
                    listener.BeginAccept(EndAccept, listener);
                    mreBeginAccept.WaitOne();
                }
            }

            listener.Close();
            listener.Dispose();
            listener = null;
        }

        private void EndAccept(IAsyncResult result)
        {
            if (!isServerRunning)
                return;

            var state = (Socket)result.AsyncState;

            var listener = state.EndAccept(result);

            HandleNewClient(listener);

            mreBeginAccept.SetIfNotNull();
        }

        private void HandleNewClient(Socket socket)
        {
            Client client;

            lock (clients)
            lock (nextClientId)
            {
                var id = nextClientId.Pop();

                if (nextClientId.Count == 0)
                {
                    nextClientId.Push(Interlocked.Increment(ref maxid));
                }

                client = new Client(id, true);
                clients.Add(id, client);
            }

            client.Connected += state_Connected;
            client.SocketError += client_SocketError;
            client.MessageReceived += state_MessageReceived;
            client.Disconnected += state_Disconnected;
            client.InitServer(socket);
        }

        private void state_Connected(Client client)
        {
            client.SendBytes(null, KindMessage.ServerReady);
            client.SendBytes(client.id.ToByte(), KindMessage.ClientId);
            client.SendBytes(clients.Keys.ToArrayOfByte(), KindMessage.ListClientId);

            RaiseConnected(client.Id);
        }

        private void state_Disconnected(Client client)
        {
            RemoveClient(client);
        }

        private void RemoveClient(Client client)
        {
            client.Connected -= state_Connected;
            client.SocketError -= client_SocketError;
            client.Disconnected -= state_Disconnected;
            client.MessageReceived -= state_MessageReceived;

            Close(client);

            lock (clients)
            lock (nextClientId)
            {
                clients.Remove(client.Id);
                nextClientId.Push(client.Id);
            }

            RaiseDisconnected(client.Id);
        }

        private void client_SocketError(Client client, Exception e)
        {
            RaiseSocketError(client, e);
        }

        private void client_MessageSent(int ClientId)
        {
            RaiseMessageSent(ClientId);
        }

        private void state_MessageReceived(Client client, byte[] msg, KindMessage kindOfSend)
        {
            RaiseMessageReceived(client.Id, msg, kindOfSend);
        }

        private Client GetClient(int id)
        {
            Client state;

            return clients.TryGetValue(id, out state) ? state : null;
        }

        public void SendAll(string message)
        {
            var keys = clients.Keys.OrderByDescending(o => o);
            var msg = Encoding.UTF8.GetBytes(message);

            foreach (var key in keys)
            {
                Send(key, msg);
            }
        }

        public void Send(int id, string message)
        {
            var msg = Encoding.UTF8.GetBytes(message);

            Send(id, msg);
        }

        private void Send(int id, byte[] message)
        {
            var client = GetClient(id);

            if (client != null)
            {
                client.Send(message);
            }
        }

        public void CloseAll()
        {
            var keys = clients.Keys.OrderByDescending(o => o);

            foreach (var key in keys)
            {
                Close(key);
            }
        }

        public void Close(int id)
        {
            var client = GetClient(id);

            Close(client);
        }

        private void Close(Client client)
        {
            if (client != null)
            {
                client.Close();
            }
        }
    }
}

Client code:

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

namespace AsyncClientServer
{
    public sealed class Client
    {
        internal string address;
        internal int port;

        internal bool isServerSocket;

        internal int id;
        internal bool closed;
        internal Socket socket;

        internal readonly byte[] socketBuffer;
        internal readonly List<byte> outBuffer;
        internal KindMessage KindOfMessage;
        internal int MessageLength;

        internal readonly Queue<byte[]> sendMsg;

        internal ManualResetEvent mreInit;
        internal ManualResetEvent mreMonitorDisconnect;
        internal ManualResetEvent mreIsConnected;

        internal ManualResetEvent mreBeginConnect;
        public delegate void ConnectedHandler(Client client);
        public event ConnectedHandler Connected;
        internal void RaiseConnected()
        {
            var handler = Connected;
            if (handler != null)
            {
                handler(this);
            }
        }

        internal ManualResetEvent mreBeginReceive;
        public delegate void MessageReceivedHandler(Client client, byte[] msg, KindMessage kindOfSend);
        public event MessageReceivedHandler MessageReceived;
        internal void RaiseMessageReceived(byte[] msg, KindMessage kindOfSend)
        {
            if (KindOfMessage == KindMessage.Message)
            {
                var handler = MessageReceived;
                if (handler != null)
                {
                    handler(this, msg, kindOfSend);
                }
            }
            else if (KindOfMessage == KindMessage.ListClientId)
            {
                if(isServerSocket)
                {
                    var handler = MessageReceived;
                    if (handler != null)
                    {
                        handler(this, msg, kindOfSend);
                    }
                }
                else
                {
                    this.RaiseReceivedListClientId(msg.ToListOfInt());
                }
            }
            else if (KindOfMessage == KindMessage.ServerReady)
            {

            }
            else if (KindOfMessage == KindMessage.ClientId)
            {
                this.RaiseReceivedClientId(msg.ToInt());
            }
        }

        internal ManualResetEvent mreBeginSend;
        internal ManualResetEvent mreEndSend;
        public delegate void MessageSentHandler(Client client, int size);
        public event MessageSentHandler MessageSent;
        internal void RaiseMessageSent(int size)
        {
            var handler = MessageSent;
            if (handler != null)
            {
                handler(this, size);
            }
        }

        internal ManualResetEvent mreBeginDisconnect;
        public delegate void DisconnectedHandler(Client client);
        public event DisconnectedHandler Disconnected;
        internal void RaiseDisconnected()
        {
            var handler = Disconnected;
            if (handler != null)
            {
                handler(this);
            }
        }

        public delegate void SocketErrorHandler(Client client, Exception e);
        public event SocketErrorHandler SocketError;
        internal void RaiseSocketError(Exception e)
        {
            var handler = SocketError;
            if (handler != null)
            {
                handler(this, e);
            }
        }

        public delegate void ReceivedClientIdHandler(Client client);
        public event ReceivedClientIdHandler ReceivedClientId;
        internal void RaiseReceivedClientId(int Id)
        {
            this.id = Id;

            var handler = ReceivedClientId;
            if (handler != null)
            {
                handler(this);
            }
        }

        public delegate void ReceivedListClientIdHandler(Client client, IEnumerable<int> ids);
        public event ReceivedListClientIdHandler ReceivedListClientId;
        internal void RaiseReceivedListClientId(IEnumerable<int> ids)
        {
            var handler = ReceivedListClientId;
            if (handler != null)
            {
                handler(this, ids);
            }
        }

        public Client()
            : this(-1, false)
        { }

        internal Client(int ClientId, bool serverSocket)
        {
            socketBuffer = new byte[Const.BufferSize];
            outBuffer = new List<byte>(Const.BufferSize);
            sendMsg = new Queue<byte[]>();
            mreIsConnected = new ManualResetEvent(true);

            isServerSocket = serverSocket;
            closed = true;
            id = ClientId;
        }

        public int Id { get { return id; } }

        public bool IsConnected()
        {
            return this.IsClientConnected();
        }

        public void Connect(string address, int port)
        {
            this.InitClient(address, port);
        }
        public void Close()
        {
            this.Disconnect();
        }

        public void Send(string message)
        {
            var msg = Encoding.UTF8.GetBytes(message);

            this.SendBytes(msg, KindMessage.Message);
        }

        public void Send(byte[] msg)
        {
            this.SendBytes(msg, KindMessage.Message);
        }

        internal void Send(byte[] msg, KindMessage kind)
        {
            this.SendBytes(msg, kind);
        }

        public void RequestListOfConnectedCliendId()
        {
            this.SendBytes(null, KindMessage.ListClientId);
        }
    }
}

Helper / extension code:

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

namespace AsyncClientServer
{
    //supported message (kind of protocol)
    public enum KindMessage : byte
    {
        Unknown = 0,
        ServerReady = 10,
        ClientId = 20,
        ListClientId = 21,
        //AddClientId = 22, //not implemented yet
        //RemClientId = 23, //not implemented yet
        Message = 50,
    }
    public static class Const
    {
        public const int BufferSize = 1; //small buffer to test multi part message
        public const int SizeOfEnvelopeKind = sizeof(KindMessage);
        public const int SizeOfEnvelopeLength = sizeof(int);
        public const int TotalSizeOfEnvelope = SizeOfEnvelopeKind + SizeOfEnvelopeLength;
        public const int MonitorDisconnectCycle = 1000; //every X millisecond check if the connection is still open
        public const int BackLogLimit = 20; // backlog for the listener
    }

    internal static class MiscOperation
    {
        internal static void SetIfNotNull(this ManualResetEvent mre)
        {
            if (mre != null && !mre.SafeWaitHandle.IsClosed)
            {
                mre.Set();
            }
        }

        internal static void HandleError(this Client client, Exception e)
        {
            if (!client.closed)
            {
                client.Disconnect();
                client.RaiseSocketError(e);
            }
        }

        public static byte[] ToArrayOfByte(this IEnumerable<int> OriginalList)
        {
            int index = 0;
            var ArrayOfByte = new byte[OriginalList.Count() * 4];

            foreach (var item in OriginalList)
            {
                ArrayOfByte[index++] = (byte)item;
                ArrayOfByte[index++] = (byte)(item >> 8);
                ArrayOfByte[index++] = (byte)(item >> 16);
                ArrayOfByte[index++] = (byte)(item >> 24);
            }

            return ArrayOfByte;
        }

        public static IEnumerable<int> ToListOfInt(this byte[] ArrayOfByte)
        {
            int index = 0;
            int length = ArrayOfByte.Length;

            var CopyOfList = new List<int>(length / 4);

            while (index < length)
            {
                CopyOfList.Add(ArrayOfByte[index++] | (ArrayOfByte[index++] << 8) | (ArrayOfByte[index++] << 16) | (ArrayOfByte[index++] << 24));
            }

            return CopyOfList;
        }

        public static byte[] ToByte(this int value)
        {
            var ArrayOfByte = new byte[4];

            ArrayOfByte[0] = (byte)value;
            ArrayOfByte[1] = (byte)(value >> 8);
            ArrayOfByte[2] = (byte)(value >> 16);
            ArrayOfByte[3] = (byte)(value >> 24);

            return ArrayOfByte;
        }

        public static int ToInt(this byte[] value)
        {
            return value[0] | (value[1] << 8) | (value[2] << 16) | (value[3] << 24);
        }

        public static Socket NewSocket()
        {
            LingerOption lo = new LingerOption(true, 10);
            var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            socket.NoDelay = true;
            socket.LingerState = lo;
            socket.ReceiveBufferSize = Const.BufferSize;
            socket.SendBufferSize = Const.BufferSize;

            return socket;
        }
    }
}

I have decided to split these into multiple files (let me know if this is a good idea).

Connect code:

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

namespace AsyncClientServer
{
    public static class ConnectOperation
    {
        internal static bool IsClientConnected(this Client client)
        {
            //this code is verbose because i had issue at some point, it should be done in one line but i will keep it verbose for now.
            bool connected = false;
            bool connected1 = false;
            bool connected2 = false;
            bool connected3 = false;
            bool poll1 = false;
            bool poll2 = false;

            client.mreIsConnected.WaitOne();
            client.mreIsConnected.Reset();

            try
            {
                if (!client.closed && client.socket != null)
                {
                    lock (client.socket)
                    {
                        connected1 = !client.closed && client.socket != null;
                        poll1 = client.socket.Poll(1000, SelectMode.SelectRead);
                        poll2 = client.socket.Available == 0;
                        connected2 = !(poll1 && poll2);
                        connected3 = client.socket.Connected;

                        connected = connected1 && connected2 && connected3;
                    }
                }
            }
            catch (Exception e)
            {
                client.HandleError(e);
                connected = false;
            }

            client.mreIsConnected.SetIfNotNull();

            return connected;
        }

        internal static void BeginConnect(this Client client)
        {
            client.closed = true;
            client.socket = MiscOperation.NewSocket();

            try
            {
                client.socket.BeginConnect(client.address, client.port, EndConnect, client);
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }
        }

        private static void EndConnect(IAsyncResult result)
        {
            var client = (Client)result.AsyncState;

            try
            {
                client.socket.EndConnect(result);

                client.closed = false;
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }

            client.mreBeginConnect.SetIfNotNull();
        }
    }
}

Disconnect code:

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

namespace AsyncClientServer
{
    internal static class DisconnectOperation
    {
        internal static void Disconnect(this Client client)
        {
            try
            {
                if (!client.closed)
                {
                    client.closed = true;

                    client.mreIsConnected.SetIfNotNull();
                    client.mreBeginReceive.SetIfNotNull();
                    client.mreBeginSend.SetIfNotNull();
                    client.mreEndSend.SetIfNotNull();
                    client.mreMonitorDisconnect.SetIfNotNull();

                    if (client.socket != null)
                    {
                        lock (client.socket)
                        {
                            if (client.socket != null)
                            {
                                client.socket.Shutdown(SocketShutdown.Both);
                                BeginDisconnect(client);

                                client.socket.Close();
                                client.socket.Dispose();
                                client.socket = null;

                                client.RaiseDisconnected();
                            }
                        }
                    }
                }
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }
        }

        internal static void MonitorDisconnect(this Client client)
        {
            using(client.mreMonitorDisconnect = new ManualResetEvent(false))
            {
                while (!client.closed)
                {
                    if (!client.IsClientConnected())
                    {
                        Disconnect(client);
                    }
                    client.mreInit.SetIfNotNull();
                    client.mreMonitorDisconnect.WaitOne(Const.MonitorDisconnectCycle);
                    client.mreIsConnected.WaitOne();
                }
            }
        }

        private static void BeginDisconnect(Client client)
        {
            using(client.mreBeginDisconnect = new ManualResetEvent(false))
            {
                try
                {
                    client.socket.BeginDisconnect(false, EndDisconnect, client);
                }
                catch (Exception e)
                {
                    client.HandleError(e);
                }

                client.mreBeginDisconnect.WaitOne(5000);
            }
        }

        private static void EndDisconnect(IAsyncResult result)
        {
            var client = (Client)result.AsyncState;

            try
            {
                client.socket.EndDisconnect(result);
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }

            client.mreBeginDisconnect.SetIfNotNull();
        }
    }
}

Init code:

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

namespace AsyncClientServer
{
    internal static class InitOperation
    {
        internal static void InitServer(this Client client, Socket listener)
        {
            if (client.socket == null)
            {
                client.address = null;
                client.port = 0;
                client.socket = listener;
                client.closed = false;

                client.InitFinal();

                client.RaiseConnected();
            }
        }

        internal static void InitClient(this Client client, string address, int port)
        {
            if (client.socket == null)
            {
                client.id = -1;
                client.address = address;
                client.port = port;

                using (client.mreBeginConnect = new ManualResetEvent(false))
                {
                    client.mreBeginConnect.Reset();
                    client.BeginConnect();
                    client.mreBeginConnect.WaitOne();
                }

                if(!client.closed)
                {
                    client.InitFinal();

                    client.RaiseConnected();
                }
                else
                {
                    client.RaiseDisconnected();
                }
            }
        }

        internal static void InitFinal(this Client client)
        {
            if (!client.closed)
            {
                using(client.mreInit = new ManualResetEvent(false))
                {
                    client.mreInit.Reset();
                    new Thread(client.BeginReceive).Start();
                    client.mreInit.WaitOne();

                    client.mreInit.Reset();
                    new Thread(client.BeginSend).Start();
                    client.mreInit.WaitOne();

                    client.mreInit.Reset();
                    new Thread(client.MonitorDisconnect).Start();
                    client.mreInit.WaitOne();
                }
            }
        }
    }
}

Receive code:

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

namespace AsyncClientServer
{
    public static class ReceiveOperation
    {
        internal static void BeginReceive(this Client client)
        {
            client.outBuffer.Clear();
            client.KindOfMessage = KindMessage.Unknown;
            client.MessageLength = int.MaxValue;

            using (client.mreBeginReceive = new ManualResetEvent(false))
            {
                try
                {
                    while (!client.closed)
                    {
                        client.mreBeginReceive.Reset();
                        client.socket.BeginReceive(client.socketBuffer, 0, Const.BufferSize, SocketFlags.None, EndReceive, client);
                        client.mreInit.SetIfNotNull();
                        client.mreBeginReceive.WaitOne();
                        client.mreIsConnected.WaitOne();
                    }
                }
                catch (Exception e)
                {
                    client.HandleError(e);
                }
            }
        }

        private static void EndReceive(IAsyncResult result)
        {
            var client = (Client)result.AsyncState;

            if (client.closed)
            {
                return;
            }

            try
            {
                var receive = client.socket.EndReceive(result);

                if (receive == 0)
                {
                    client.Disconnect();
                    return;
                }

                client.ProcessNewData(receive);
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }

            client.mreBeginReceive.SetIfNotNull();
        }

        internal static void ProcessNewData(this Client client, int receive)
        {
            lock (client.outBuffer)
            {
                client.outBuffer.AddRange(client.socketBuffer.Take(receive));

                do
                {
                    client.EnvelopeRead();

                    if (client.outBuffer.Count >= client.MessageLength)
                    {
                        var msg = client.outBuffer.GetRange(0, client.MessageLength).ToArray();
                        client.outBuffer.RemoveRange(0, client.MessageLength);

                        client.RaiseMessageReceived(msg, client.KindOfMessage);

                        client.KindOfMessage = KindMessage.Unknown;
                        client.MessageLength = client.outBuffer.Count >= Const.TotalSizeOfEnvelope ? 0 :  int.MaxValue;
                    }
                } while (client.outBuffer.Count >= client.MessageLength);
            }
        }

        private static void EnvelopeRead(this Client client)
        {
            if (client.KindOfMessage == KindMessage.Unknown && client.outBuffer.Count >= Const.TotalSizeOfEnvelope)
            {
                client.KindOfMessage = (KindMessage)client.outBuffer[0];

                if (!Enum.IsDefined(typeof(KindMessage), client.KindOfMessage))
                {
                    client.KindOfMessage = KindMessage.Unknown;
                    throw new FormatException("Doesn't understand the envelope!");
                }

                byte[] length = client.outBuffer.GetRange(Const.SizeOfEnvelopeKind, Const.SizeOfEnvelopeLength).ToArray();

                client.MessageLength = length.ToInt();

                if (client.MessageLength > ushort.MaxValue || client.MessageLength < 0)
                {
                    throw new ArgumentOutOfRangeException();
                }

                client.outBuffer.RemoveRange(0, Const.TotalSizeOfEnvelope);
            }
        }
    }
}

Send code:

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

namespace AsyncClientServer
{
    internal static class SendOperation
    {
        internal static void SendBytes(this Client client, byte[] msg, KindMessage kind)
        {
            try
            {
                if(msg == null)
                {
                    msg = new byte[0];
                }

                var length = msg.Length;

                if (length > ushort.MaxValue)
                {
                    throw new ArgumentOutOfRangeException();
                }

                byte[] outMsg;
                EnvelopeSend(msg, kind, length, out outMsg);

                lock (client.sendMsg)
                {
                    client.sendMsg.Enqueue(outMsg);
                }

                client.mreBeginSend.SetIfNotNull();
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }
        }

        internal static void EnvelopeSend(byte[] response, KindMessage kindOfSend, int length, out byte[] outMsg)
        {
            outMsg = new byte[length + Const.TotalSizeOfEnvelope];

            outMsg[0] = (byte)kindOfSend;
            Array.Copy(length.ToByte(), 0, outMsg, Const.SizeOfEnvelopeKind, Const.SizeOfEnvelopeLength);
            Array.Copy(response, 0, outMsg, Const.TotalSizeOfEnvelope, length);
        }

        internal static void BeginSend(this Client client)
        {
            byte[] msg = null;

            client.sendMsg.Clear();

            using(client.mreBeginSend = new ManualResetEvent(false))
            using(client.mreEndSend = new ManualResetEvent(false))
            {
                try
                {
                    while (!client.closed)
                    {
                        client.mreBeginSend.Reset();
                        if (client.sendMsg.Count > 0)
                        {
                            lock (client.sendMsg)
                            {
                                if (client.sendMsg.Count > 0)
                                {
                                    msg = client.sendMsg.Dequeue();
                                }
                            }

                            if (msg != null)
                            {
                                client.mreEndSend.Reset();
                                client.socket.BeginSend(msg, 0, msg.Length, SocketFlags.None, EndSend, client);
                                client.mreEndSend.WaitOne();
                                msg = null;
                            }
                        }
                        client.mreInit.SetIfNotNull();
                        client.mreBeginSend.WaitOne();
                        client.mreIsConnected.WaitOne();
                    }
                }
                catch (Exception e)
                {
                    client.HandleError(e);
                }
            }
        }

        private static void EndSend(IAsyncResult result)
        {
            var client = (Client)result.AsyncState;

            if (client.closed)
            {
                return;
            } 

            try
            {
                int size = client.socket.EndSend(result);

                client.RaiseMessageSent(size - Const.TotalSizeOfEnvelope);
            }
            catch (Exception e)
            {
                client.HandleError(e);
            }

            client.mreBeginSend.SetIfNotNull();
            client.mreEndSend.SetIfNotNull();
        }
    }
}

Answer 1

Observations as I'm reading down the code:

Server

• Class doesn't implement an interface. If I'm using your library, at least one of my classes is going to have a dependency on the Server class. If I use an IoC container, I need to pick one that can work with concrete types (that's usually not a problem though). If I want to write unit tests for that class of mine however, I'm stuck. I need to write a wrapper around your Server class, and inject that instead of using the Server directly: by not implementing an interface, you're creating work for your client code.

• Class is sealed. That's usually not a good idea. General design guidelines say:

  DO NOT seal classes without having a good reason to do so.

  If I wanted to take your library and, I don't know, extend the Server class with a decorator that logs everything coming in and everything going out... well I'm out of luck, I need to think of another way. And a sealed class can't be mocked, so the combination of not implementing an interface and sealing the type makes for either tightly-coupled client code, or client code that needs to work harder than it should to decouple the components.

• None of the private fields are readonly. Fields that can be initialized statically, or in the constructor, and that aren't assigned a new reference, should be made readonly to better convey the idea that their reference shouldn't be reassigned throughout the lifetime of the class instance.

• clients would probably be better off as a ConcurrentDictionary.

• Event delegates aren't standard. Client code will expect event delegates to be compatible with System.EventHandler. The convention is to declare events using an EventHandler delegate for events without arguments, and to use the generic EventHandler<T> for events with arguments - and encapsulate the arguments in a class derived from System.EventArgs. That way if your arguments ever need to change, you're not changing the delegate's signature and you're not breaking any existing client code.

• Naming convention for event-raising procedures isn't standard either. RaiseConnected would be OnConnected, and RaiseMessageReceived would be OnMessageReceived, for example.

• Naming convention for event-handling procedures isn't consistent. Take this snippet:

      client.Connected += state_Connected;
      client.SocketError += client_SocketError;
      client.MessageReceived += state_MessageReceived;
      client.Disconnected += state_Disconnected;
      client.InitServer(socket);
  

  Typically when the event source is called client, event handlers would be named client_NameOfEvent.

• KindMessage reads funny. I'd reverse the words to get MessageKind. And it would make KindMessage kindOfSend read like MessageKind messageKind.

• msg has no reason not to be spelled out: message gets picked up just as well by IntelliSense/auto-complete.

• There's a possible NullReferenceException right here - GetClient can return null, so you need a local variable and a null-check here:

  GetClient(ClientId).SendBytes(clients.Keys.ToArrayOfByte(),KindMessage.ListClientId);`
  

• msg would be more descriptive as messageBytes in the Send method.

• This boilerplate repeats a lot:

  var handler = Disconnected;
  if (handler != null)
  {
      handler(ClientId);
  }
  

  You could make a general-purpose event-raising method... if your event delegates were EventHandler delegates, and if your arguments were encapsulated in an EventArgs class:

  private void RaiseEventInternal(EventHandler raised, EventArgs args)
  {
      var handler = raised;
      if (handler != null)
      {
          handler(this, args);
      }
  }
  

  Alternatively, you can declare your events with a no-op delegate:

  public event ConnectedHandler Connected = delegate { };
  

  That way the handler will never be null on any thread, and you can safely do away with the thread-local copy and null-check.

---

I strongly recommend you standardize your events, and if you're going to keep the class sealed, at least make it implement some IServer interface:

public interface IServer
{
    /// <summary>
    /// An event that is raised when a client successfully connects.
    /// </summary>
    EventHandler<ClientEventArgs> Connected;
    /// <summary>
    /// An event that is raised when a client successfully disconnects.
    /// </summary>
    EventHandler<ClientEventArgs> Disconnected;
    /// <summary>
    /// An event that is raised when a client successfully receives a message.
    /// </summary>
    EventHandler<ClientMessageEventArgs> MessageReceived;
    /// <summary>
    /// An event that is raised when a client successfully sends a message.
    /// </summary>
    EventHandler<ClientEventArgs> MessageSent;
    /// <summary>
    /// An event that is raised when a client throws an exception.
    /// </summary>
    EventHandler<ClientExceptionEventArgs> SocketError;

    /// <summary>
    /// Starts the server.
    /// </summary>
    void Start(string address, int port);
    /// <summary>
    /// Stops the server.
    /// </summary>
    void Stop();

    /// <summary>
    /// Sends the specified message to all connected clients.
    /// </summary>
    void Send(string message);
    /// <summary>
    /// Sends the specified message to the specified client.
    /// </summary>
    void Send(string message, int clientId);

    /// <summary>
    /// Closes all client connections.
    /// </summary>
    void Close();
    /// <summary>
    /// Closes connection with specified client.
    /// </summary>
    void Close(int clientId);
}

Notice:

• EventHandler<T> event delegates.
• Stop and Start methods; "Server" is redundant in the name.
• Method overloading for Send and Close.
• XML comments. Documenting parameters and generic type parameters would be even nicer.

Now, interfaces shouldn't be designed to change, and the Interface Segregation Principle says the fewer members, the better - and quickly glancing at the Client code, it seems a number of things are common between the client and the server, so I'd consider making the two types share an interface for the things they have in common. This has the added benefit that a test that's only calling Connect can mock some IConnectable interface that merely exposes Connect and Close methods and the related events. At the end of the day you might end up with an IServer interface looking something like:

public interface IServer : IConnectable, IMessageSender
{
    void Start(string address, int port);
    void Stop();
    EventHandler<ClientExceptionEventArgs> SocketError;
}

---

I'm not convinced by this:

lock (clients)
lock (nextClientId)

That's not how I understand locking works: you're not locking access to/from the specified variable - the scope under a lock statement is the protected code that only one thread can ever execute. It's good that you're using an instance field, because all threads will see that field and will be able to determine whether they can obtain a lock or not... but the two fields are both created in the same thread, so one of them is redundant IMO. But I don't write much code at that level of abstraction so I might be wrong if there's a tricky little race condition this double lock is preventing, in which case a little comment there would have been nice to explain why we need to lock with two handles... in any case, you wouldn't need to acquire the first lock, if you used a ConcurrentDictionary instead of a regular one.