Separate startup from initialization (#23)

Procedures to be run on initialization
Fix concurrency issues with listeners

MTSC.UnitTests/E2ETests.cs

@@ -35,7 +35,7 @@ namespace MTSC.UnitTests
         public TestContext TestContext { get; set; }
         static ServerSide.Server Server { get; set; }
 
-        [AssemblyInitialize]
+        [ClassInitialize]
         public static void InitializeServer(TestContext testContext)
         {
             ServicePointManager.ServerCertificateValidationCallback += (s, e, o, p) => true;

MTSC.UnitTests/Models/ServiceOnInitialization.cs

@@ -0,0 +1,36 @@
+using MTSC.ServerSide;
+using MTSC.ServerSide.Handlers;
+using System;
+
+namespace MTSC.UnitTests.Models;
+
+public sealed class ServiceOnInitialization : IHandler, IRunOnInitialization
+{
+    public bool RanOnInitialization { get; private set; }
+
+    public void OnInitialization(Server server)
+    {
+        if (this.RanOnInitialization)
+        {
+            throw new InvalidOperationException("Initialization procedure already ran");
+        }
+
+        this.RanOnInitialization = true;
+    }
+
+    public void ClientRemoved(Server server, ClientData client)
+    {
+    }
+
+    public bool HandleClient(Server server, ClientData client) => true;
+
+    public bool HandleReceivedMessage(Server server, ClientData client, Message message) => false;
+
+    public bool HandleSendMessage(Server server, ClientData client, ref Message message) => false;
+
+    public bool PreHandleReceivedMessage(Server server, ClientData client, ref Message message) => false;
+
+    public void Tick(Server server)
+    {
+    }
+}

MTSC.UnitTests/Models/ServiceOnStartup.cs

@@ -0,0 +1,30 @@
+using MTSC.ServerSide;
+using MTSC.ServerSide.Handlers;
+
+namespace MTSC.UnitTests.Models;
+
+public sealed class ServiceOnStartup : IHandler, IRunOnStartup
+{
+    public int RanOnStartup { get; set; } = 0;
+
+    public void OnStartup(Server server)
+    {
+        this.RanOnStartup += 1;
+    }
+
+    public void ClientRemoved(Server server, ClientData client)
+    {
+    }
+
+    public bool HandleClient(Server server, ClientData client) => true;
+
+    public bool HandleReceivedMessage(Server server, ClientData client, Message message) => false;
+
+    public bool HandleSendMessage(Server server, ClientData client, ref Message message) => false;
+
+    public bool PreHandleReceivedMessage(Server server, ClientData client, ref Message message) => false;
+
+    public void Tick(Server server)
+    {
+    }
+}

MTSC.UnitTests/ServerTests.cs

@@ -1,7 +1,11 @@
 using FluentAssertions;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using MTSC.ServerSide;
+using MTSC.UnitTests.Models;
+using System;
 using System.Threading;
+using System.Threading.Tasks;
 
 namespace MTSC.UnitTests
 {
@@ -34,5 +38,51 @@ namespace MTSC.UnitTests
             runningTask.Wait(1000);
             runningTask.IsCompleted.Should().BeTrue();
         }
+
+        [TestMethod]
+        public async Task Stop_Start_RunsInitializationOnlyOnce()
+        {
+            var serviceOnInitialization = new ServiceOnInitialization();
+
+            var server = new Server(256)
+                .AddHandler(serviceOnInitialization);
+            server.RunAsync();
+            var cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(5));
+            while(cancellationToken.IsCancellationRequested is false)
+            {
+                if (server.Running)
+                {
+                    break;
+                }
+            }
+
+            server.Stop();
+            server.RunAsync();
+            while (cancellationToken.IsCancellationRequested is false)
+            {
+                if (server.Running)
+                {
+                    break;
+                }
+            }
+
+            serviceOnInitialization.RanOnInitialization.Should().BeTrue();
+        }
+
+        [TestMethod]
+        public async Task Stop_Start_RunsOnStartupEveryTime()
+        {
+            var serviceOnStartup = new ServiceOnStartup();
+
+            var server = new Server(256)
+                .AddHandler(serviceOnStartup);
+            var cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(2));
+            await server.RunAsync(cancellationToken.Token);
+
+            cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(2));
+            await server.RunAsync(cancellationToken.Token);
+
+            serviceOnStartup.RanOnStartup.Should().Be(2);
+        }
     }
 }

MTSC/MTSC.csproj

@@ -5,13 +5,13 @@
     <TargetFrameworks>netstandard2.0</TargetFrameworks>
     <ApplicationIcon />
     <StartupObject />
-    <Version>5.5</Version>
+    <Version>5.5.1</Version>
     <LangVersion>latest</LangVersion>
     <Authors>Alexandru-Victor Macocian</Authors>
     <Product>MTSC</Product>
     <Description>Modular TCP Server and Client</Description>
-    <AssemblyVersion>5.5.0.0</AssemblyVersion>
-    <FileVersion>5.5.0.0</FileVersion>
+    <AssemblyVersion>5.5.1.0</AssemblyVersion>
+    <FileVersion>5.5.1.0</FileVersion>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <Platforms>AnyCPU;x64</Platforms>
     <PackageProjectUrl>https://github.com/AlexMacocian/MTSC</PackageProjectUrl>

MTSC/ServerSide/Handlers/HttpRoutingHandler.cs

@@ -16,7 +16,7 @@ using static MTSC.Common.Http.HttpMessage;
 
 namespace MTSC.ServerSide.Handlers
 {
-    public sealed class HttpRoutingHandler : IHandler, IRunOnStartup
+    public sealed class HttpRoutingHandler : IHandler, IRunOnInitialization
     {
         private class FragmentedMessage
         {
@@ -188,7 +188,7 @@ namespace MTSC.ServerSide.Handlers
             }
         }
 
-        void IRunOnStartup.OnStartup(Server server)
+        void IRunOnInitialization.OnInitialization(Server server)
         {
             foreach (var routes in this.moduleDictionary.Values)
             {

MTSC/ServerSide/Handlers/IRunOnInitialization.cs

@@ -0,0 +1,9 @@
+namespace MTSC.ServerSide.Handlers;
+
+/// <summary>
+/// Implement this interface in handlers that need to run a procedure on server initialization.
+/// </summary>
+public interface IRunOnInitialization
+{
+    void OnInitialization(Server server);
+}

MTSC/ServerSide/Handlers/IRunOnStartup.cs

@@ -1,10 +1,9 @@
-namespace MTSC.ServerSide.Handlers
+namespace MTSC.ServerSide.Handlers;
+
+/// <summary>
+/// Implement this interface in handlers that need to run a procedure on server startup.
+/// </summary>
+public interface IRunOnStartup
 {
-    /// <summary>
-    /// Implement this interface in handlers that need to run a procedure on server startup.
-    /// </summary>
-    public interface IRunOnStartup
-    {
-        void OnStartup(Server server);
-    }
+    void OnStartup(Server server);
 }

MTSC/ServerSide/Handlers/WebsocketRoutingHandler.cs

@@ -10,7 +10,7 @@ using System.Text;
 
 namespace MTSC.ServerSide.Handlers
 {
-    public class WebsocketRoutingHandler : IHandler, IRunOnStartup
+    public class WebsocketRoutingHandler : IHandler, IRunOnInitialization
     {
         private const string WebsocketHeaderAcceptKey = "Sec-WebSocket-Accept";
         private const string WebsocketHeaderKey = "Sec-WebSocket-Key";
@@ -302,7 +302,7 @@ namespace MTSC.ServerSide.Handlers
             }
         }
 
-        void IRunOnStartup.OnStartup(Server server)
+        void IRunOnInitialization.OnInitialization(Server server)
         {
             foreach((var routeType, _) in this.moduleDictionary.Values)
             {

MTSC/ServerSide/Listeners/TcpSocketListener.cs

@@ -10,17 +10,20 @@ namespace MTSC.ServerSide.Listeners
     {
         private readonly ConcurrentQueue<Socket> acceptedSockets = new();
 
-        private Socket socket;
+        private volatile Socket socket;
 
         public bool Active => this.socket is not null;
         public EndPoint LocalEndpoint { get => this.socket?.LocalEndPoint; }
 
         public void Initialize(int port, IPAddress ipAddress)
         {
-            this.socket?.Close();
-            this.socket?.Dispose();
-            this.socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
-            this.socket.Bind(new IPEndPoint(ipAddress, port));
+            lock (this)
+            {
+                this.socket?.Close();
+                this.socket?.Dispose();
+                this.socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
+                this.socket.Bind(new IPEndPoint(ipAddress, port));
+            }
         }
 
         public Socket AcceptSocket()
@@ -40,24 +43,32 @@ namespace MTSC.ServerSide.Listeners
 
         public void Start()
         {
-            this.socket.Listen(50);
-            var socket = this.socket;
-            Task.Run(async() =>
+            lock (this)
             {
-                while(socket is not null)
+                while(this.socket is not Socket)
                 {
-                    var clientSocket = await this.socket.AcceptAsync();
-                    this.acceptedSockets.Enqueue(clientSocket);
                 }
-            });
-            
+
+                this.socket.Listen(50);
+                Task.Run(async () =>
+                {
+                    while (this.socket is not null)
+                    {
+                        var clientSocket = await this.socket.AcceptAsync();
+                        this.acceptedSockets.Enqueue(clientSocket);
+                    }
+                });
+            }
         }
 
         public void Stop()
         {
-            this.socket?.Close();
-            this.socket?.Dispose();
-            this.socket = null;
+            lock (this)
+            {
+                this.socket?.Close();
+                this.socket?.Dispose();
+                this.socket = null;
+            }
         }
     }
 }

MTSC/ServerSide/Server.cs

@@ -30,7 +30,7 @@ namespace MTSC.ServerSide
     {
         #region Fields
         private CancellationToken cancellationToken;
-        private bool running;
+        private bool initialized;
         private X509Certificate2 certificate;
         private readonly BackgroundServicesHolder backgroundServicesHolder;
         private readonly ProducerConsumerQueue<ClientData> addQueue = new();
@@ -106,11 +106,11 @@ namespace MTSC.ServerSide
         /// </summary>
         public IReadOnlyCollection<ClientData> Clients { get => this.clients.AsReadOnly(); }
         /// <summary>
-        /// <see cref="IServiceManager"/> for configuring and retrieving services. Will be initialized at server startup from the <see cref="ServiceCollection"/>.
+        /// <see cref="IServiceManager"/> for configuring and retrieving services. Will be populated at server startup from the <see cref="ServiceCollection"/>.
         /// </summary>
-        public IServiceManager ServiceManager { get; private set; }
+        public IServiceManager ServiceManager { get; private set; } = new ServiceManager();
         /// <summary>
-        /// <see cref="IServiceCollection"/> used to create the <see cref="IServiceManager"/> at server startup.
+        /// <see cref="IServiceCollection"/> used to populate the <see cref="IServiceManager"/> at server startup.
         /// </summary>
         public IServiceCollection ServiceCollection { get; } = new ServiceCollection();
         #endregion
@@ -423,50 +423,62 @@ namespace MTSC.ServerSide
         /// <param name="cancellationToken">Cancellation token used to cancel the server.</param>
         public void Run(CancellationToken cancellationToken = default)
         {
-            if (this.running)
+            lock (this)
             {
-                return;
-            }
-
-            this.ServiceManager?.Dispose();
-            this.ServiceManager = this.ServiceCollection.BuildSlimServiceProvider() as IServiceManager;
-            this.Listener?.Stop();
-            if (this.logger is null)
-            {
-                // Try to get a logger, in case it exists. If not, keep it null.
-                try
+                if (this.Running)
                 {
-                    this.logger = this.ServiceManager.GetService<ILogger<Server>>();
+                    return;
                 }
-                catch
+
+                /*
+                 * Run only once when the server is first initializing.
+                 * This gives the handlers an opportunity to schedule tasks, register services, etc.
+                 */
+                if (this.initialized is false)
                 {
+                    this.initialized = true;
+                    this.ServiceManager = this.ServiceCollection.BuildSlimServiceProvider(this.ServiceManager) as IServiceManager;
+
+                    foreach (var toBeRunOnInitialization in this.handlers.OfType<IRunOnInitialization>())
+                    {
+                        toBeRunOnInitialization.OnInitialization(this);
+                    }
+
+                    // Try to get a logger, in case it exists. If not, keep it null.
                     try
                     {
-                        this.logger = this.ServiceManager.GetService<ILogger>();
+                        this.logger = this.ServiceManager.GetService<ILogger<Server>>();
                     }
                     catch
                     {
+                        try
+                        {
+                            this.logger = this.ServiceManager.GetService<ILogger>();
+                        }
+                        catch
+                        {
+                        }
                     }
                 }
-            }
 
-            this.Listener.Initialize(this.Port, this.IPAddress);
-            this.Listener.Start();
-            this.cancellationToken = cancellationToken;
-            this.running = true;
-            this.Log("Server started on: " + this.Listener.LocalEndpoint.ToString());
-            foreach(var toBeRunOnStartup in this.handlers.OfType<IRunOnStartup>())
-            {
-                toBeRunOnStartup.OnStartup(this);
-            }
+                this.Listener?.Stop();
+                this.Listener.Initialize(this.Port, this.IPAddress);
+                this.Listener.Start();
+                this.cancellationToken = cancellationToken;
+                this.Log("Server starting on: " + this.Listener.LocalEndpoint.ToString());
+                foreach (var toBeRunOnStartup in this.handlers.OfType<IRunOnStartup>())
+                {
+                    toBeRunOnStartup.OnStartup(this);
+                }
 
-            /*
-             * Initialize background services.
-             */
-            this.backgroundServicesHolder.Initialize();
+                /*
+                 * Initialize background services.
+                 */
+                this.backgroundServicesHolder.Initialize();
+            }
 
             DateTime startLoopTime;
-            while (this.running)
+            while (this.Listener?.Active is true)
             {
                 startLoopTime = DateTime.Now;
                 
@@ -476,7 +488,6 @@ namespace MTSC.ServerSide
                  */
                 if (this.cancellationToken.IsCancellationRequested)
                 {
-                    this.running = false;
                     break;
                 }
 
@@ -586,7 +597,7 @@ namespace MTSC.ServerSide
                 }
             }
 
-            this.Listener.Stop();
+            this.Listener?.Stop();
             foreach (var client in this.Clients)
             {
                 try
@@ -598,8 +609,6 @@ namespace MTSC.ServerSide
                     this.HandleException(e);
                 }
             }
-
-            this.Listener = null;
         }
         /// <summary>
         /// Runs the server async.
@@ -613,10 +622,7 @@ namespace MTSC.ServerSide
         /// </summary>
         public void Stop()
         {
-            if (this.running)
-            {
-                this.running = false;
-            }
+            this.Listener?.Stop();
         }
         #endregion
         #region Private Methods