From f8cfbffcd43b0636f9c33fd76d294b0013c74afa Mon Sep 17 00:00:00 2001 From: dbuechel Date: Fri, 22 Mar 2019 15:41:25 +0100 Subject: [PATCH] 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 | 2 + SafeExamBrowser.I18n/Text.xml | 6 ++ .../Operations/ClientOperationTests.cs | 44 ++++++++--- .../Operations/ClientOperation.cs | 73 ++++++++++++------- SafeExamBrowser.Runtime/RuntimeController.cs | 8 +- 5 files changed, 96 insertions(+), 37 deletions(-) diff --git a/SafeExamBrowser.Contracts/I18n/TextKey.cs b/SafeExamBrowser.Contracts/I18n/TextKey.cs index 80b96e9f..7ce8cd78 100644 --- a/SafeExamBrowser.Contracts/I18n/TextKey.cs +++ b/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, diff --git a/SafeExamBrowser.I18n/Text.xml b/SafeExamBrowser.I18n/Text.xml index bf68947e..d12a3a21 100644 --- a/SafeExamBrowser.I18n/Text.xml +++ b/SafeExamBrowser.I18n/Text.xml @@ -102,6 +102,12 @@ Reload? + + The application failed to start a new session! Please consult the application log for more information... + + + Session Start Error + An unexpected error occurred during the shutdown procedure! Please consult the application log for more information... diff --git a/SafeExamBrowser.Runtime.UnitTests/Operations/ClientOperationTests.cs b/SafeExamBrowser.Runtime.UnitTests/Operations/ClientOperationTests.cs index d8594ff2..6dd03762 100644 --- a/SafeExamBrowser.Runtime.UnitTests/Operations/ClientOperationTests.cs +++ b/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(), It.IsAny())).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(), It.IsAny())).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); diff --git a/SafeExamBrowser.Runtime/Operations/ClientOperation.cs b/SafeExamBrowser.Runtime/Operations/ClientOperation.cs index a56921e8..997ed020 100644 --- a/SafeExamBrowser.Runtime/Operations/ClientOperation.cs +++ b/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..."); - - 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; + return success; } private bool TryStopClient() diff --git a/SafeExamBrowser.Runtime/RuntimeController.cs b/SafeExamBrowser.Runtime/RuntimeController.cs index c1d4767f..a0d87a5a 100644 --- a/SafeExamBrowser.Runtime/RuntimeController.cs +++ b/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(); }