Merge pull request #639 from Cyberboss/FixProcessIssue

Swallow and log all process termination exceptions. Avoid where possible

src/Tgstation.Server.Host/Core/Process.cs

@@ -1,4 +1,5 @@
-using System;
+using Microsoft.Extensions.Logging;
+using System;
 using System.Text;
 using System.Threading.Tasks;
 
@@ -22,7 +23,12 @@ namespace Tgstation.Server.Host.Core
 		readonly StringBuilder errorStringBuilder;
 		readonly StringBuilder combinedStringBuilder;
 
-		public Process(System.Diagnostics.Process handle, Task<int> lifetime, StringBuilder outputStringBuilder, StringBuilder errorStringBuilder, StringBuilder combinedStringBuilder)
+		/// <summary>
+		/// The <see cref="ILogger"/> for the <see cref="Process"/>
+		/// </summary>
+		readonly ILogger<Process> logger;
+
+		public Process(System.Diagnostics.Process handle, Task<int> lifetime, StringBuilder outputStringBuilder, StringBuilder errorStringBuilder, StringBuilder combinedStringBuilder, ILogger<Process> logger)
 		{
 			this.handle = handle ?? throw new ArgumentNullException(nameof(handle));
 			Lifetime = lifetime ?? throw new ArgumentNullException(nameof(lifetime));
@@ -31,6 +37,8 @@ namespace Tgstation.Server.Host.Core
 			this.errorStringBuilder = errorStringBuilder;
 			this.combinedStringBuilder = combinedStringBuilder;
 
+			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
+
 			Id = handle.Id;
 			Startup = Task.Factory.StartNew(() =>
 			{
@@ -40,6 +48,8 @@ namespace Tgstation.Server.Host.Core
 				}
 				catch (InvalidOperationException) { }
 			}, default, TaskCreationOptions.LongRunning, TaskScheduler.Current);
+
+			logger.LogTrace("Created proces ID: {0}", Id);
 		}
 
 		/// <inheritdoc />
@@ -72,12 +82,18 @@ namespace Tgstation.Server.Host.Core
 		/// <inheritdoc />
 		public void Terminate()
 		{
+			if (handle.HasExited)
+				return;
 			try
 			{
+				logger.LogTrace("Terminating process...");
 				handle.Kill();
 				handle.WaitForExit();
 			}
-			catch (InvalidOperationException) { }
+			catch (Exception e)
+			{
+				logger.LogDebug("Process termination exception: {0}", e);
+			}
 		}
 
 		public void SetHighPriority()
@@ -85,6 +101,7 @@ namespace Tgstation.Server.Host.Core
 			try
 			{
 				handle.PriorityClass = System.Diagnostics.ProcessPriorityClass.AboveNormal;
+				logger.LogTrace("Set to above normal priority", handle.Id);
 			}
 			catch (InvalidOperationException) { }
 		}

src/Tgstation.Server.Host/Core/ProcessExecutor.cs

@@ -1,6 +1,5 @@
 using Microsoft.Extensions.Logging;
 using System;
-using System.Diagnostics;
 using System.Text;
 using System.Threading.Tasks;
 
@@ -14,6 +13,11 @@ namespace Tgstation.Server.Host.Core
 		/// </summary>
 		readonly ILogger<ProcessExecutor> logger;
 
+		/// <summary>
+		/// The <see cref="ILoggerFactory"/> for the <see cref="ProcessExecutor"/>
+		/// </summary>
+		readonly ILoggerFactory loggerFactory;
+
 		/// <summary>
 		/// Create a <see cref="Task{TResult}"/> resulting in the exit code of a given <paramref name="handle"/>
 		/// </summary>
@@ -43,9 +47,11 @@ namespace Tgstation.Server.Host.Core
 		/// Construct a <see cref="ProcessExecutor"/>
 		/// </summary>
 		/// <param name="logger">The value of <see cref="logger"/></param>
-		public ProcessExecutor(ILogger<ProcessExecutor> logger)
+		/// <param name="loggerFactory">The value of <see cref="loggerFactory"/></param>
+		public ProcessExecutor(ILogger<ProcessExecutor> logger, ILoggerFactory loggerFactory)
 		{
 			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
+			this.loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory));
 		}
 
 		/// <inheritdoc />
@@ -55,7 +61,7 @@ namespace Tgstation.Server.Host.Core
 			var handle = System.Diagnostics.Process.GetProcessById(id);
 			try
 			{
-				return new Process(handle, AttachExitHandler(handle), null, null, null);
+				return new Process(handle, AttachExitHandler(handle), null, null, null, loggerFactory.CreateLogger<Process>());
 			}
 			catch
 			{
@@ -123,7 +129,7 @@ namespace Tgstation.Server.Host.Core
 				}
 				catch (InvalidOperationException) { }
 
-				return new Process(handle, lifetimeTask, outputStringBuilder, errorStringBuilder, combinedStringBuilder);
+				return new Process(handle, lifetimeTask, outputStringBuilder, errorStringBuilder, combinedStringBuilder, loggerFactory.CreateLogger<Process>());
 			}
 			catch
 			{