From 78cd7818f04d679598a3c721eaa7c225f33ce52f Mon Sep 17 00:00:00 2001
From: Jordan Brown <Cyberboss@users.noreply.github.com>
Date: Thu, 13 Sep 2018 18:40:42 -0400
Subject: [PATCH] Swallow and log all process termination exceptions. Avoid
 where possible

---
 src/Tgstation.Server.Host/Core/Process.cs     | 23 ++++++++++++++++---
 .../Core/ProcessExecutor.cs                   | 14 +++++++----
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/Tgstation.Server.Host/Core/Process.cs b/src/Tgstation.Server.Host/Core/Process.cs
index 15296ebb2f..513b0bbe71 100644
--- a/src/Tgstation.Server.Host/Core/Process.cs
+++ b/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) { }
 		}
diff --git a/src/Tgstation.Server.Host/Core/ProcessExecutor.cs b/src/Tgstation.Server.Host/Core/ProcessExecutor.cs
index 6b8a97490a..5aad8a9e8b 100644
--- a/src/Tgstation.Server.Host/Core/ProcessExecutor.cs
+++ b/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
 			{