SEBWIN-226: Improved client startup algorithm to immediately abort if client instance terminates unexpectedly. Thus also re-integrated message to user for session start failure.

SafeExamBrowser.Contracts/I18n/TextKey.cs

@@ -48,6 +48,8 @@ namespace SafeExamBrowser.Contracts.I18n
 		MessageBox_ReconfigurationQuestionTitle,
 		MessageBox_ReloadConfirmation,
 		MessageBox_ReloadConfirmationTitle,
+		MessageBox_SessionStartError,
+		MessageBox_SessionStartErrorTitle,
 		MessageBox_ShutdownError,
 		MessageBox_ShutdownErrorTitle,
 		MessageBox_StartupError,

SafeExamBrowser.I18n/Text.xml

@@ -102,6 +102,12 @@
   <Entry key="MessageBox_ReloadConfirmationTitle">
     Reload?
   </Entry>
+  <Entry key="MessageBox_SessionStartError">
+    The application failed to start a new session! Please consult the application log for more information...
+  </Entry>
+  <Entry key="MessageBox_SessionStartErrorTitle">
+    Session Start Error
+  </Entry>
   <Entry key="MessageBox_ShutdownError">
     An unexpected error occurred during the shutdown procedure! Please consult the application log for more information...
   </Entry>

SafeExamBrowser.Runtime.UnitTests/Operations/ClientOperationTests.cs

@@ -7,6 +7,7 @@
  */
 
 using System;
+using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 using SafeExamBrowser.Contracts.Communication.Data;
@@ -68,7 +69,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustStartClientWhenPerforming()
+		public void Perform_MustStartClient()
 		{
 			var result = default(OperationResult);
 			var response = new AuthenticationResponse { ProcessId = 1234 };
@@ -87,19 +88,42 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustFailStartupIfClientNotStartedWithinTimeout()
+		public void Perform_MustFailStartupIfClientNotStartedWithinTimeout()
 		{
 			var result = default(OperationResult);
 
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Returns(process.Object);
+
 			result = sut.Perform();
 
-			Assert.IsNull(sessionContext.ClientProcess);
 			Assert.IsNull(sessionContext.ClientProxy);
+			Assert.AreEqual(process.Object, sessionContext.ClientProcess);
 			Assert.AreEqual(OperationResult.Failed, result);
 		}
 
 		[TestMethod]
-		public void MustFailStartupIfConnectionToClientNotEstablished()
+		public void Perform_MustFailStartupImmediatelyIfClientTerminates()
+		{
+			const int ONE_SECOND = 1000;
+
+			var after = default(DateTime);
+			var before = default(DateTime);
+			var result = default(OperationResult);
+			var terminateClient = new Action(() => Task.Delay(100).ContinueWith(_ => process.Raise(p => p.Terminated += null, 0)));
+
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Returns(process.Object).Callback(terminateClient);
+			sut = new ClientOperation(logger.Object, processFactory.Object, proxyFactory.Object, runtimeHost.Object, sessionContext, ONE_SECOND);
+
+			before = DateTime.Now;
+			result = sut.Perform();
+			after = DateTime.Now;
+
+			Assert.IsTrue(after - before < new TimeSpan(0, 0, ONE_SECOND));
+			Assert.AreEqual(OperationResult.Failed, result);
+		}
+
+		[TestMethod]
+		public void Perform_MustFailStartupIfConnectionToClientNotEstablished()
 		{
 			var result = default(OperationResult);
 
@@ -114,7 +138,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustFailStartupIfAuthenticationNotSuccessful()
+		public void Perform_MustFailStartupIfAuthenticationNotSuccessful()
 		{
 			var result = default(OperationResult);
 			var response = new AuthenticationResponse { ProcessId = -1 };
@@ -133,7 +157,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustStartClientWhenRepeating()
+		public void Repeat_MustStartClient()
 		{
 			var result = default(OperationResult);
 			var response = new AuthenticationResponse { ProcessId = 1234 };
@@ -152,7 +176,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustStopClientWhenReverting()
+		public void Revert_MustStopClient()
 		{
 			proxy.Setup(p => p.Disconnect()).Callback(terminated);
 
@@ -168,7 +192,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustKillClientIfStoppingFailed()
+		public void Revert_MustKillClientIfStoppingFailed()
 		{
 			process.Setup(p => p.Kill()).Callback(() => process.SetupGet(p => p.HasTerminated).Returns(true));
 
@@ -182,7 +206,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustAttemptToKillFiveTimesThenAbort()
+		public void Revert_MustAttemptToKillFiveTimesThenAbort()
 		{
 			PerformNormally();
 			sut.Revert();
@@ -194,7 +218,7 @@ namespace SafeExamBrowser.Runtime.UnitTests.Operations
 		}
 
 		[TestMethod]
-		public void MustNotStopClientOnRevertIfAlreadyTerminated()
+		public void Revert_MustNotStopClientIfAlreadyTerminated()
 		{
 			process.SetupGet(p => p.HasTerminated).Returns(true);
 

SafeExamBrowser.Runtime/Operations/ClientOperation.cs

@@ -96,10 +96,6 @@ namespace SafeExamBrowser.Runtime.Operations
 
 		private bool TryStartClient()
 		{
-			var clientReady = false;
-			var clientReadyEvent = new AutoResetEvent(false);
-			var clientReadyEventHandler = new CommunicationEventHandler(() => clientReadyEvent.Set());
-
 			var clientExecutable = Context.Next.AppConfig.ClientExecutablePath;
 			var clientLogFile = $"{'"' + Context.Next.AppConfig.ClientLogFile + '"'}";
 			var clientLogLevel = Context.Next.Settings.LogLevel.ToString();
@@ -107,48 +103,75 @@ namespace SafeExamBrowser.Runtime.Operations
 			var startupToken = Context.Next.StartupToken.ToString("D");
 			var uiMode = Context.Next.Settings.UserInterfaceMode.ToString();
 
+			var clientReady = false;
+			var clientReadyEvent = new AutoResetEvent(false);
+			var clientReadyEventHandler = new CommunicationEventHandler(() => clientReadyEvent.Set());
+
+			var clientTerminated = false;
+			var clientTerminatedEventHandler = new ProcessTerminatedEventHandler(_ => { clientTerminated = true; clientReadyEvent.Set(); });
+
 			logger.Info("Starting new client process...");
 			runtimeHost.AllowConnection = true;
 			runtimeHost.ClientReady += clientReadyEventHandler;
 			ClientProcess = processFactory.StartNew(clientExecutable, clientLogFile, clientLogLevel, runtimeHostUri, startupToken, uiMode);
+			ClientProcess.Terminated += clientTerminatedEventHandler;
 
 			logger.Info("Waiting for client to complete initialization...");
 			clientReady = clientReadyEvent.WaitOne(timeout_ms);
-			runtimeHost.ClientReady -= clientReadyEventHandler;
+
 			runtimeHost.AllowConnection = false;
+			runtimeHost.ClientReady -= clientReadyEventHandler;
+			ClientProcess.Terminated -= clientTerminatedEventHandler;
+
+			if (clientReady && !clientTerminated)
+			{
+				return TryStartCommunication();
+			}
 
 			if (!clientReady)
 			{
 				logger.Error($"Failed to start client within {timeout_ms / 1000} seconds!");
-
-				return false;
 			}
 
+			if (clientTerminated)
+			{
+				logger.Error("Client instance terminated unexpectedly during initialization!");
+			}
+
+			return false;
+		}
+
+		private bool TryStartCommunication()
+		{
+			var success = false;
+
 			logger.Info("Client has been successfully started and initialized. Creating communication proxy for client host...");
 			ClientProxy = proxyFactory.CreateClientProxy(Context.Next.AppConfig.ClientAddress);
 
-			if (!ClientProxy.Connect(Context.Next.StartupToken))
+			if (ClientProxy.Connect(Context.Next.StartupToken))
+			{
+				logger.Info("Connection with client has been established. Requesting authentication...");
+
+				var communication = ClientProxy.RequestAuthentication();
+				var response = communication.Value;
+
+				success = communication.Success && ClientProcess.Id == response?.ProcessId;
+
+				if (success)
+				{
+					logger.Info("Authentication of client has been successful, client is ready to operate.");
+				}
+				else
+				{
+					logger.Error("Failed to verify client integrity!");
+				}
+			}
+			else
 			{
 				logger.Error("Failed to connect to client!");
-
-				return false;
 			}
 
-			logger.Info("Connection with client has been established. Requesting authentication...");
+			return success;
-
-			var communication = ClientProxy.RequestAuthentication();
-			var response = communication.Value;
-
-			if (!communication.Success || ClientProcess.Id != response?.ProcessId)
-			{
-				logger.Error("Failed to verify client integrity!");
-
-				return false;
-			}
-
-			logger.Info("Authentication of client has been successful, client is ready to operate.");
-
-			return true;
 		}
 
 		private bool TryStopClient()

SafeExamBrowser.Runtime/RuntimeController.cs

@@ -207,9 +207,15 @@ namespace SafeExamBrowser.Runtime
 			{
 				StopSession();
 
+				messageBox.Show(TextKey.MessageBox_SessionStartError, TextKey.MessageBox_SessionStartErrorTitle, icon: MessageBoxIcon.Error, parent: runtimeWindow);
+
 				logger.Info("Terminating application...");
 				shutdown.Invoke();
 			}
+			else
+			{
+				messageBox.Show(TextKey.MessageBox_SessionStartError, TextKey.MessageBox_SessionStartErrorTitle, icon: MessageBoxIcon.Error, parent: runtimeWindow);
+			}
 		}
 
 		private void HandleSessionStartAbortion()
@@ -301,7 +307,6 @@ namespace SafeExamBrowser.Runtime
 			}
 
 			messageBox.Show(TextKey.MessageBox_ApplicationError, TextKey.MessageBox_ApplicationErrorTitle, icon: MessageBoxIcon.Error);
-
 			shutdown.Invoke();
 		}
 
@@ -315,7 +320,6 @@ namespace SafeExamBrowser.Runtime
 			}
 
 			messageBox.Show(TextKey.MessageBox_ApplicationError, TextKey.MessageBox_ApplicationErrorTitle, icon: MessageBoxIcon.Error);
-
 			shutdown.Invoke();
 		}