From e4d6c5d4876d2df89b6da192d69c0152c4a597f7 Mon Sep 17 00:00:00 2001
From: Jordan Dominion <Cyberboss@users.noreply.github.com>
Date: Mon, 27 Nov 2023 12:49:55 -0500
Subject: [PATCH] Fix Process dispose handling

Fix read timeout workaround unnecessarily disposing too much
---
 src/Tgstation.Server.Host/System/Process.cs | 30 ++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/Tgstation.Server.Host/System/Process.cs b/src/Tgstation.Server.Host/System/Process.cs
index da67c8f0e8..210a86db8c 100644
--- a/src/Tgstation.Server.Host/System/Process.cs
+++ b/src/Tgstation.Server.Host/System/Process.cs
@@ -128,7 +128,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public async ValueTask DisposeAsync()
 		{
-			if (Interlocked.Exchange(ref disposed, 1) == 1)
+			if (Interlocked.Exchange(ref disposed, 1) != 0)
 				return;
 
 			logger.LogTrace("Disposing PID {pid}...", Id);
@@ -146,6 +146,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public async ValueTask<string> GetCombinedOutput(CancellationToken cancellationToken)
 		{
+			CheckDisposed();
 			if (readTask == null)
 				throw new InvalidOperationException("Output/Error stream reading was not enabled!");
 
@@ -158,7 +159,7 @@ namespace Tgstation.Server.Host.System
 				if (!readTask.IsCompleted)
 				{
 					logger.LogWarning("Detected process output read hang on PID {pid}! Closing handle as a workaround...", Id);
-					await DisposeAsync();
+					cancellationTokenSource.Cancel();
 				}
 			}
 
@@ -168,6 +169,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public void Terminate()
 		{
+			CheckDisposed();
 			if (handle.HasExited)
 			{
 				logger.LogTrace("PID {pid} already exited", Id);
@@ -190,6 +192,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public void AdjustPriority(bool higher)
 		{
+			CheckDisposed();
 			var targetPriority = higher ? ProcessPriorityClass.AboveNormal : ProcessPriorityClass.BelowNormal;
 			try
 			{
@@ -205,6 +208,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public void Suspend()
 		{
+			CheckDisposed();
 			try
 			{
 				processFeatures.SuspendProcess(handle);
@@ -220,6 +224,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public void Resume()
 		{
+			CheckDisposed();
 			try
 			{
 				processFeatures.ResumeProcess(handle);
@@ -235,6 +240,7 @@ namespace Tgstation.Server.Host.System
 		/// <inheritdoc />
 		public string GetExecutingUsername()
 		{
+			CheckDisposed();
 			var result = processFeatures.GetExecutingUsername(handle);
 			logger.LogTrace("PID {pid} Username: {username}", Id, result);
 			return result;
@@ -244,6 +250,7 @@ namespace Tgstation.Server.Host.System
 		public ValueTask CreateDump(string outputFile, CancellationToken cancellationToken)
 		{
 			ArgumentNullException.ThrowIfNull(outputFile);
+			CheckDisposed();
 
 			logger.LogTrace("Dumping PID {pid} to {dumpFilePath}...", Id, outputFile);
 			return processFeatures.CreateDump(handle, outputFile, cancellationToken);
@@ -255,18 +262,29 @@ namespace Tgstation.Server.Host.System
 		/// <returns>A <see cref="Task{TResult}"/> resulting in the <see cref="global::System.Diagnostics.Process.ExitCode"/> or <see langword="null"/> if the process was detached.</returns>
 		async Task<int?> WrapLifetimeTask()
 		{
+			bool hasExited;
 			try
 			{
 				await handle.WaitForExitAsync(cancellationTokenSource.Token);
-				var exitCode = handle.ExitCode;
-				logger.LogTrace("PID {pid} exited with code {exitCode}", Id, exitCode);
-				return exitCode;
+				hasExited = true;
 			}
 			catch (OperationCanceledException ex)
 			{
 				logger.LogTrace(ex, "Process lifetime task cancelled!");
-				return null;
+				hasExited = handle.HasExited;
 			}
+
+			if (!hasExited)
+				return null;
+
+			var exitCode = handle.ExitCode;
+			logger.LogTrace("PID {pid} exited with code {exitCode}", Id, exitCode);
+			return exitCode;
 		}
+
+		/// <summary>
+		/// Throws an <see cref="ObjectDisposedException"/> if a method of the <see cref="Process"/> was called after <see cref="DisposeAsync"/>.
+		/// </summary>
+		void CheckDisposed() => ObjectDisposedException.ThrowIf(disposed != 0, this);
 	}
 }