Improvements to my code?

[tkblackbelt]

Hey guys,

So I started working on a private server a few days ago and have a super basic structure so far. If any of you get the chance I would appreciate it if you could take a quick scan over my code and see where I can improve or if you see any logic mistakes that I missed. I would primarily like my sockets and and worker threads to be looked at over the rest of it.

Here the link to the project

I also have a couple of questions.

Since I'm using several worker threads to process data would it be fine to have a seperate database connection in each thread?

Would you guys happen to know which people made the various cryptographic classes so I can give them credit.

-Tkblackbelt

[I don't have a username]

These are some old of mine.

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

namespace YukaServer.Network.Sockets
{
    public class EndResult
    {
        /// <summary>
        /// The server socket.
        /// </summary>
        private Socket Socket;

        /// <summary>
        /// The async result.
        /// </summary>
        private IAsyncResult asyncResult;

        /// <summary>
        /// Creates a new instance of EndResult.
        /// </summary>
        /// <param name="Socket">The socket.</param>
        /// <param name="asyncResult">The async result.</param>
        public EndResult(Socket Socket, IAsyncResult asyncResult)
        {
            this.Socket = Socket;
            this.asyncResult = asyncResult;
        }

        /// <summary>
        /// Gets the socket from the endaccept result.
        /// </summary>
        /// <returns>Returns null if there was no socket accepted.</returns>
        public Socket GetSocket()
        {
            try
            {
                return Socket.EndAccept(asyncResult);
            }
            catch
            {
                return null;
            }
        }
    }
}

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

namespace YukaServer.Network.Sockets
{
    public class SocketClient
    {
        /// <summary>
        /// The event invoked when a client disconnects.
        /// </summary>
        public SocketConnectionHandler OnDisconnection;

        /// <summary>
        /// The event invoked when the server receives data from a client.
        /// </summary>
        public SocketBufferHandler OnReceive;

        /// <summary>
        /// The actual client socket.
        /// </summary>
        private Socket clientSocket;

        /// <summary>
        /// The socket owner.
        /// </summary>
        public object Owner;

        /// <summary>
        /// Gets the client socket.
        /// </summary>
        public Socket ClientSocket
        {
            get
            {
                return clientSocket;
            }
        }

        /// <summary>
        /// The data-holding buffer.
        /// </summary>
        public byte[] Buffer;

        /// <summary>
        /// Gets the IPAddress of the EndPoint.
        /// </summary>
        public IPAddress Address
        {
            get
            {
                return (ClientSocket.RemoteEndPoint as IPEndPoint).Address;
            }
        }

        /// <summary>
        /// Gets the IPAddress of the EndPoint as a string.
        /// </summary>
        public string IP
        {
            get
            {
                return Address.ToString();
            }
        }

        /// <summary>
        /// Ends the accepting of the socket.
        /// </summary>
        /// <param name="endResult">The EndResult.</param>
        /// <returns>Returns true if the sockets connection was accepted.</returns>
        public bool EndAccept(EndResult endResult)
        {
            return (clientSocket = endResult.GetSocket()) != null;
        }

        /// <summary>
        /// The method handling all receive callbacks from the client socket.
        /// </summary>
        /// <param name="result">The async result.</param>
        private void Receive_Callback(IAsyncResult asyncResult)
        {
            try
            {
                if (ClientSocket.Connected)
                {
                    SocketError err;
                    int recSize = ClientSocket.EndReceive(asyncResult, out err);
                    if (err == SocketError.Success)
                    {
                        if (recSize >= 4 && recSize <= 1024)
                        {
                            byte[] recPacket = new byte[recSize];
                            Native.Msvcrt.MemoryCopy(recPacket, Buffer, recSize);
                            if (OnReceive != null)
                                OnReceive.Invoke(this, recPacket);
                            BeginReceive();
                        }
                    }
                    else
                        Disconnect();
                }
                else
                    Disconnect();
            }
            catch
            {
                Disconnect();
            }
        }

        /// <summary>
        /// Beginning to receive data from a client.
        /// </summary>
        public void BeginReceive()
        {
            Buffer = new byte[1024];
            ClientSocket.BeginReceive(Buffer, 0, 1024, SocketFlags.None, new AsyncCallback(Receive_Callback), null);
        }

        /// <summary>
        /// Sending data to the client.
        /// </summary>
        /// <param name="Buffer">The buffer to send.</param>
        public void Send(byte[] Buffer)
        {
            try
            {
                if (ClientSocket.Connected)
                {
                    ClientSocket.BeginSend(Buffer, 0, Buffer.Length, SocketFlags.None, new AsyncCallback(Send_Callback), null);
                }
                else
                    Disconnect();
            }
            catch
            {
                Disconnect();
            }
        }

        /// <summary>
        /// The method handling all send callbacks from the client socket.
        /// </summary>
        /// <param name="asyncResult">The async result.</param>
        private void Send_Callback(IAsyncResult asyncResult)
        {
            try
            {
                int send = ClientSocket.EndSend(asyncResult);
                if (send < 4)
                    Disconnect();
            }
            catch
            {
            }
        }

        /// <summary>
        /// This will disconnect a client, but this is also called when a disconnection happen.
        /// </summary>
        public void Disconnect()
        {
            try
            {
                ClientSocket.Disconnect(false);
            }
            catch { }

            if (OnDisconnection != null)
                OnDisconnection.Invoke(this);
        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace YukaServer.Network.Sockets
{
    public delegate void SocketConnectionHandler(SocketClient sClient);
    public delegate void SocketBufferHandler(SocketClient sClient, byte[] Buffer);
}

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

namespace YukaServer.Network.Sockets
{
    public class SocketServer
    {
        /// <summary>
        /// The server socket.
        /// </summary>
        public Socket ServerSocket;

        /// <summary>
        /// The port the server is bound to.
        /// </summary>
        private int port;

        /// <summary>
        /// Gets the port the server is bound to.
        /// </summary>
        public int Port
        {
            get
            {
                return port;
            }
        }

        /// <summary>
        /// If the server should use any ip for the endpoint.
        /// </summary>
        private bool anyIP;

        /// <summary>
        /// The event invoked when a client connects.
        /// </summary>
        public SocketConnectionHandler OnConnection;

        /// <summary>
        /// Creates a new instance of the socket server.
        /// </summary>
        /// <param name="Port">The port, which the server should be bound to.</param>
        public SocketServer(int Port, bool anyIP)
        {
            ServerSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            port = Port;
            this.anyIP = anyIP;
        }

        /// <summary>
        /// Binding the socket to the port assigned with the server.
        /// </summary>
        /// <param name="IP">The IP. Should be null, if the server is set to anyIP.</param>
        public void Bind(IPAddress IP)
        {
            IPAddress ip = (anyIP) ? IPAddress.Any : (IP == null) ? IPAddress.Any : IP;
            ServerSocket.Bind(new IPEndPoint(ip, Port));
            ServerSocket.Listen(100);
        }

        /// <summary>
        /// Calling System.Net.Sockets.Socket.BeginAccept() with the necessary parameters.
        /// </summary>
        public void BeginAccept()
        {
            ServerSocket.BeginAccept(new AsyncCallback(Accept_Callback), new SocketClient());
        }

        /// <summary>
        /// The method handling all accept callbacks from the client socket.
        /// </summary>
        /// <param name="asyncResult">The async result.</param>
        private void Accept_Callback(IAsyncResult asyncResult)
        {
            try
            {
                if (asyncResult.AsyncState != null)
                {
                    SocketClient sClient = asyncResult.AsyncState as SocketClient;
                    if (sClient.EndAccept(new EndResult(ServerSocket, asyncResult)))
                    {
                        if (OnConnection != null)
                            OnConnection.Invoke(sClient);
                    }
                }
            }
            catch
            {
            }

            BeginAccept();
        }
    }
}

You could compare yours to those and ermm you're always running a while loop around the beginaccept, which is not necessary as you just call the beginaccept in the accept callback and it should work just fine.

Oh and your ConsumerPool. Use

[tkblackbelt]

@I don't have a username

Thanks for the reply I will look over your code. I'm new to C# so I've been following Msdn a lot and there async socket code used a while loop so that's why I did. I assume though if I didn't use the while loop and just called beginAccept then I wouldn't need to create the separate thread in the ServerSocket class since it would be managed in a separate thread by the run-time. Haha didn't know they had there was a Task class I will go read up on the documentation and most likely replace my custom one.

[I don't have a username]

You're learning that's a step forward and my sockets may not be perfect or whatever, but every time I do a new socket system it gets improved, so you should try do so. Same thing goes for everything actually :P And about the task class. I didn't know either, I think it was Korvacs who referred it to me

[Korvacs]

Your socket system is pretty err poor...

[_DreadNought_]

Originally Posted by Korvacs

Your socket system is pretty err poor...

Should explain why..

[Korvacs]

His socket implementation requires manual splitting of packets instead of taking advantage of the nature of conquer packets which are sent sequentially. His socket implementation doesnt consider the fact that some packets may be bigger than 1024 bytes long (Conquer does implement packets longer than this). His socket implementation doesnt consider invalid data. His socket system doesnt implement any form of encryption or decryption. His socket system utilises async sending which will result in race conditions while handling packets which will mean that packets will arrive at the client in an incorrect order (Anyone wanting to challenge this can, i have however experienced this issue with the DH Key exchange resulting in clients logging in 50 - 60% of the time, and the rest of the time crashing).

This socket system reminds me of the example systems you find lying around on the internet.....it is no way near robust enough or tailored.

[I don't have a username]

Mine or his and mine wasn't written for Conquer tho. It was written for a custom game lol.

[Korvacs]

Which explains why its bad for Conquer and for the OP lol.

[I don't have a username]

It wouldn't take much effort to rewrite it tho lol.