diff --git a/src/Tgstation.Server.Host/System/Process.cs b/src/Tgstation.Server.Host/System/Process.cs index 32cca93336..56ab181e58 100644 --- a/src/Tgstation.Server.Host/System/Process.cs +++ b/src/Tgstation.Server.Host/System/Process.cs @@ -85,7 +85,7 @@ namespace Tgstation.Server.Host.System { handle.WaitForInputIdle(); } - catch (InvalidOperationException ex) + catch (Exception ex) { logger.LogDebug(ex, "Error on WaitForInputIdle()!"); } @@ -102,7 +102,6 @@ namespace Tgstation.Server.Host.System async Task WrapLifetimeTask(Task lifetimeTask) { - // relevant: https://stackoverflow.com/a/26722542 var exitCode = await lifetimeTask.ConfigureAwait(false); logger.LogTrace("PID {0} exited with code {1}", Id, exitCode); return exitCode; @@ -136,18 +135,22 @@ namespace Tgstation.Server.Host.System /// public void Terminate() { - if (Lifetime.IsCompleted) + if (handle.HasExited) + { + logger.LogTrace("PID {0} already exited", Id); return; + } + try { logger.LogTrace("Terminating PID {0}...", Id); handle.Kill(); - - // DO NOT USE WaitForExit! https://stackoverflow.com/a/26722542 + if (!handle.WaitForExit(5000)) + logger.LogWarning("WaitForExit() on PID {0} timed out!", Id); } catch (Exception e) { - logger.LogDebug(e, "Process termination exception!"); + logger.LogDebug(e, "PID {0} termination exception!", Id); } } @@ -195,7 +198,7 @@ namespace Tgstation.Server.Host.System } } - /// + /// public async Task GetExecutingUsername(CancellationToken cancellationToken) { var result = await processFeatures.GetExecutingUsername(handle, cancellationToken).ConfigureAwait(false); diff --git a/src/Tgstation.Server.Host/System/ProcessExecutor.cs b/src/Tgstation.Server.Host/System/ProcessExecutor.cs index 4c0bc5f8c8..04329dfd35 100644 --- a/src/Tgstation.Server.Host/System/ProcessExecutor.cs +++ b/src/Tgstation.Server.Host/System/ProcessExecutor.cs @@ -24,22 +24,27 @@ namespace Tgstation.Server.Host.System /// readonly ILoggerFactory loggerFactory; - /// - /// Create a resulting in the exit code of a given - /// - /// The to attach the for - /// A new resulting in the exit code of - Task AttachExitHandler(global::System.Diagnostics.Process handle) + async Task> AttachExitHandlerBeforeLaunch(global::System.Diagnostics.Process handle, Task startupTask) + { + var id = -1; + var result = AttachExitHandler(handle, () => id); + await startupTask.ConfigureAwait(false); + return result; + } + + Task AttachExitHandler(global::System.Diagnostics.Process handle, Func idProvider) { handle.EnableRaisingEvents = true; + var tcs = new TaskCompletionSource(); - handle.Exited += (a, b) => + void ExitHandler(object sender, EventArgs args) { + var id = idProvider(); try { if (tcs.Task.IsCompleted) { - logger.LogTrace("Skipping process exit handler as the TaskCompletionSource is already set"); + logger.LogTrace("Skipping PID {0} exit handler as the TaskCompletionSource is already set", id); return; } @@ -49,23 +54,25 @@ namespace Tgstation.Server.Host.System // Try because this can be invoked twice for weird reasons if (tcs.TrySetResult(exitCode)) - logger.LogTrace("Process termination event completed"); + logger.LogTrace("PID {0} termination event completed", id); else - logger.LogTrace("Ignoring duplicate process termination event"); + logger.LogTrace("Ignoring duplicate PID {0} termination event", id); } catch (InvalidOperationException ex) { if (!tcs.Task.IsCompleted) throw; - logger.LogTrace(ex, "Ignoring expected exception!"); + logger.LogTrace(ex, "Ignoring expected PID {0} exit handler exception!", id); } } - catch(Exception ex) + catch (Exception ex) { - logger.LogError(ex, "Process exit handler exception!"); + logger.LogError(ex, "PID {0} exit handler exception!", id); } - }; + } + + handle.Exited += ExitHandler; return tcs.Task; } @@ -150,11 +157,10 @@ namespace Tgstation.Server.Host.System Task outputTask = null; Task errorTask = null; - TaskCompletionSource processStartTcs = null; + var processStartTcs = new TaskCompletionSource(); if (readOutput || readError) { combinedStringBuilder = new StringBuilder(); - processStartTcs = new TaskCompletionSource(); async Task ConsumeReader(Func readerFunc) { @@ -188,30 +194,30 @@ namespace Tgstation.Server.Host.System } } - var lifetimeTask = AttachExitHandler(handle); + var lifetimeTaskTask = AttachExitHandlerBeforeLaunch(handle, processStartTcs.Task); try { handle.Start(); - var process = new Process( - processFeatures, - handle, - lifetimeTask, - outputTask, - errorTask, - combinedStringBuilder, - loggerFactory.CreateLogger(), false); - - processStartTcs?.SetResult(null); - - return process; + processStartTcs.SetResult(null); } catch (Exception ex) { - processStartTcs?.SetException(ex); + processStartTcs.SetException(ex); throw; } + + var process = new Process( + processFeatures, + handle, + lifetimeTaskTask.GetAwaiter().GetResult(), // won't block + outputTask, + errorTask, + combinedStringBuilder, + loggerFactory.CreateLogger(), false); + + return process; } catch { @@ -250,10 +256,11 @@ namespace Tgstation.Server.Host.System { try { + var pid = handle.Id; return new Process( processFeatures, handle, - AttachExitHandler(handle), + AttachExitHandler(handle, () => pid), null, null, null,