diff --git a/src/Tgstation.Server.Host.Watchdog/Watchdog.cs b/src/Tgstation.Server.Host.Watchdog/Watchdog.cs index fc86f1f271..4f67185223 100644 --- a/src/Tgstation.Server.Host.Watchdog/Watchdog.cs +++ b/src/Tgstation.Server.Host.Watchdog/Watchdog.cs @@ -143,17 +143,10 @@ namespace Tgstation.Server.Host.Watchdog process.StartInfo.UseShellExecute = false; // runs in the same console - var tcs = new TaskCompletionSource(); - process.Exited += (a, b) => - { - tcs.TrySetResult(null); - }; - process.EnableRaisingEvents = true; - var killedHostProcess = false; try { - var processTask = tcs.Task; + Task processTask = null; (int, Task) StartProcess(string additionalArg) { if (additionalArg != null) @@ -162,11 +155,10 @@ namespace Tgstation.Server.Host.Watchdog logger.LogInformation("Launching host with arguments: {arguments}", process.StartInfo.Arguments); process.Start(); - return (process.Id, processTask); + return (process.Id, processTask = process.WaitForExitAsync(cancellationToken)); } using (var processCts = new CancellationTokenSource()) - using (processCts.Token.Register(() => tcs.TrySetResult(null))) using (cancellationToken.Register(() => { if (!Directory.Exists(updateDirectory)) diff --git a/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs b/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs index 7471d0ee21..ce23a737bc 100644 --- a/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs @@ -206,7 +206,7 @@ namespace Tgstation.Server.Host.Components.Byond try { // noShellExecute because we aren't doing runas shennanigans - await using var directXInstaller = await processExecutor.LaunchProcess( + await using var directXInstaller = processExecutor.LaunchProcess( IOManager.ConcatPath(rbdx, "DXSETUP.exe"), rbdx, "/silent", @@ -214,7 +214,7 @@ namespace Tgstation.Server.Host.Components.Byond int exitCode; using (cancellationToken.Register(() => directXInstaller.Terminate())) - exitCode = await directXInstaller.Lifetime; + exitCode = (await directXInstaller.Lifetime).Value; cancellationToken.ThrowIfCancellationRequested(); if (exitCode != 0) @@ -251,7 +251,7 @@ namespace Tgstation.Server.Host.Components.Byond // 1. It'd make IByondInstaller need to be transient per-instance and WindowsByondInstaller relys on being a singleton for its DX installer call // 2. The instance could be renamed, so it'd have to be an unfriendly ID anyway. var arguments = $"advfirewall firewall add rule name=\"TGS DreamDaemon {version}\" program=\"{dreamDaemonPath}\" protocol=tcp dir=in enable=yes action=allow"; - await using var netshProcess = await processExecutor.LaunchProcess( + await using var netshProcess = processExecutor.LaunchProcess( "netsh.exe", IOManager.ResolvePath(), arguments, @@ -260,7 +260,7 @@ namespace Tgstation.Server.Host.Components.Byond int exitCode; using (cancellationToken.Register(() => netshProcess.Terminate())) - exitCode = await netshProcess.Lifetime; + exitCode = (await netshProcess.Lifetime).Value; cancellationToken.ThrowIfCancellationRequested(); Logger.LogDebug( diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index 46f2857883..ff584a4ab1 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -18,7 +18,6 @@ using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Components.Session; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Database; -using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; @@ -860,7 +859,7 @@ namespace Tgstation.Server.Host.Components.Deployment /// A representing the running operation. async Task RunDreamMaker(string dreamMakerPath, Models.CompileJob job, CancellationToken cancellationToken) { - await using var dm = await processExecutor.LaunchProcess( + await using var dm = processExecutor.LaunchProcess( dreamMakerPath, ioManager.ResolvePath( job.DirectoryName.ToString()), @@ -873,7 +872,7 @@ namespace Tgstation.Server.Host.Components.Deployment int exitCode; using (cancellationToken.Register(() => dm.Terminate())) - exitCode = await dm.Lifetime; + exitCode = (await dm.Lifetime).Value; cancellationToken.ThrowIfCancellationRequested(); logger.LogDebug("DreamMaker exit code: {exitCode}", exitCode); diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index 6acd29dd49..653eb443fb 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -67,7 +67,7 @@ namespace Tgstation.Server.Host.Components.Session public Task LaunchResult { get; } /// - public Task Lifetime { get; } + public Task Lifetime { get; } /// public Task OnStartup => startupTcs.Task; @@ -296,7 +296,7 @@ namespace Tgstation.Server.Host.Components.Session ? "no" : $"incompatible ({reattachInformation.Dmb.CompileJob.DMApiVersion})"); - async Task WrapLifetime() + async Task WrapLifetime() { var exitCode = await process.Lifetime; await postLifetimeCallback(); diff --git a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs index 60931b9050..80ae371cd2 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs @@ -523,7 +523,7 @@ namespace Tgstation.Server.Host.Components.Session : String.Empty, parameters); - var process = await processExecutor.LaunchProcess( + var process = processExecutor.LaunchProcess( byondLock.DreamDaemonPath, dmbProvider.Directory, arguments, diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs index d4d5c27b6f..4c66a9136a 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs @@ -629,7 +629,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles foreach (var scriptFile in scriptFiles) { logger.LogTrace("Running event script {scriptFile}...", scriptFile); - await using (var script = await processExecutor.LaunchProcess( + await using (var script = processExecutor.LaunchProcess( ioManager.ConcatPath(resolvedScriptsDir, scriptFile), resolvedScriptsDir, String.Join( diff --git a/src/Tgstation.Server.Host/System/IProcessBase.cs b/src/Tgstation.Server.Host/System/IProcessBase.cs index 612c86447b..472b1cf63a 100644 --- a/src/Tgstation.Server.Host/System/IProcessBase.cs +++ b/src/Tgstation.Server.Host/System/IProcessBase.cs @@ -9,9 +9,9 @@ namespace Tgstation.Server.Host.System interface IProcessBase { /// - /// The resulting in the exit code of the process. + /// The resulting in the exit code of the process or if the process was detached. /// - Task Lifetime { get; } + Task Lifetime { get; } /// /// Set's the owned to a non-normal value. diff --git a/src/Tgstation.Server.Host/System/IProcessExecutor.cs b/src/Tgstation.Server.Host/System/IProcessExecutor.cs index e862a5ec87..81593dbe37 100644 --- a/src/Tgstation.Server.Host/System/IProcessExecutor.cs +++ b/src/Tgstation.Server.Host/System/IProcessExecutor.cs @@ -1,6 +1,4 @@ -using System.Threading.Tasks; - -namespace Tgstation.Server.Host.System +namespace Tgstation.Server.Host.System { /// /// For launching '. @@ -16,8 +14,8 @@ namespace Tgstation.Server.Host.System /// File to write process output and error streams to. Requires to be . /// If the process output and error streams should be read. /// If shell execute should not be used. Must be set if is set. - /// A resulting in the new . - Task LaunchProcess( + /// The new . + IProcess LaunchProcess( string fileName, string workingDirectory, string arguments = null, diff --git a/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs b/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs index 73401a4ba1..1332203311 100644 --- a/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs +++ b/src/Tgstation.Server.Host/System/PosixProcessFeatures.cs @@ -88,7 +88,7 @@ namespace Tgstation.Server.Host.System string output; int exitCode; - await using (var gcoreProc = await lazyLoadedProcessExecutor.Value.LaunchProcess( + await using (var gcoreProc = lazyLoadedProcessExecutor.Value.LaunchProcess( GCorePath, Environment.CurrentDirectory, $"-o {outputFile} {process.Id}", @@ -96,7 +96,7 @@ namespace Tgstation.Server.Host.System noShellExecute: true)) { using (cancellationToken.Register(() => gcoreProc.Terminate())) - exitCode = await gcoreProc.Lifetime; + exitCode = (await gcoreProc.Lifetime).Value; output = await gcoreProc.GetCombinedOutput(cancellationToken); logger.LogDebug("gcore output:{0}{1}", Environment.NewLine, output); diff --git a/src/Tgstation.Server.Host/System/Process.cs b/src/Tgstation.Server.Host/System/Process.cs index 883c9e9946..ac122ce0ff 100644 --- a/src/Tgstation.Server.Host/System/Process.cs +++ b/src/Tgstation.Server.Host/System/Process.cs @@ -20,7 +20,7 @@ namespace Tgstation.Server.Host.System public Task Startup { get; } /// - public Task Lifetime { get; } + public Task Lifetime { get; } /// /// The for the . @@ -38,9 +38,9 @@ namespace Tgstation.Server.Host.System readonly global::System.Diagnostics.Process handle; /// - /// The used to shutdown the . + /// The used to shutdown the and . /// - readonly CancellationTokenSource readerCts; + readonly CancellationTokenSource cancellationTokenSource; /// /// The . @@ -58,8 +58,7 @@ namespace Tgstation.Server.Host.System /// /// The value of . /// The value of . - /// The value of . - /// The value of . + /// The override value of . /// The value of . /// The value of . /// If was NOT just created. @@ -67,7 +66,6 @@ namespace Tgstation.Server.Host.System IProcessFeatures processFeatures, global::System.Diagnostics.Process handle, CancellationTokenSource readerCts, - Task lifetime, Task readTask, ILogger logger, bool preExisting) @@ -78,7 +76,7 @@ namespace Tgstation.Server.Host.System safeHandle = handle.SafeHandle; Id = handle.Id; - this.readerCts = readerCts; + cancellationTokenSource = readerCts ?? new CancellationTokenSource(); this.processFeatures = processFeatures ?? throw new ArgumentNullException(nameof(processFeatures)); @@ -86,7 +84,7 @@ namespace Tgstation.Server.Host.System this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); - Lifetime = WrapLifetimeTask(lifetime ?? throw new ArgumentNullException(nameof(lifetime))); + Lifetime = WrapLifetimeTask(); if (preExisting) { @@ -117,11 +115,13 @@ namespace Tgstation.Server.Host.System public async ValueTask DisposeAsync() { logger.LogTrace("Disposing PID {pid}...", Id); - readerCts?.Cancel(); - readerCts?.Dispose(); + cancellationTokenSource.Cancel(); + cancellationTokenSource.Dispose(); if (readTask != null) await readTask; + await Lifetime; + safeHandle.Dispose(); handle.Dispose(); } @@ -221,13 +221,21 @@ namespace Tgstation.Server.Host.System /// /// Attaches a log message to the process' exit event. /// - /// The original lifetime . - /// A functionally identical to . - async Task WrapLifetimeTask(Task lifetimeTask) + /// A resulting in the or if the process was detached. + async Task WrapLifetimeTask() { - var exitCode = await lifetimeTask; - logger.LogTrace("PID {pid} exited with code {exitCode}", Id, exitCode); - return exitCode; + try + { + await handle.WaitForExitAsync(cancellationTokenSource.Token); + var exitCode = handle.ExitCode; + logger.LogTrace("PID {pid} exited with code {exitCode}", Id, exitCode); + return exitCode; + } + catch (OperationCanceledException ex) + { + logger.LogTrace(ex, "Process lifetime task cancelled!"); + return null; + } } } } diff --git a/src/Tgstation.Server.Host/System/ProcessExecutor.cs b/src/Tgstation.Server.Host/System/ProcessExecutor.cs index af4f01c421..1e5ea2e45a 100644 --- a/src/Tgstation.Server.Host/System/ProcessExecutor.cs +++ b/src/Tgstation.Server.Host/System/ProcessExecutor.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; -using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.IO; namespace Tgstation.Server.Host.System @@ -80,7 +79,7 @@ namespace Tgstation.Server.Host.System } /// - public async Task LaunchProcess( + public IProcess LaunchProcess( string fileName, string workingDirectory, string arguments, @@ -111,41 +110,47 @@ namespace Tgstation.Server.Host.System handle.StartInfo.UseShellExecute = !noShellExecute; var processStartTcs = new TaskCompletionSource(); - var lifetimeTaskTask = AttachExitHandlerBeforeLaunch(handle, processStartTcs.Task); Task readTask = null; CancellationTokenSource disposeCts = null; - if (readStandardHandles) - { - handle.StartInfo.RedirectStandardOutput = true; - handle.StartInfo.RedirectStandardError = true; - - disposeCts = new CancellationTokenSource(); - readTask = ConsumeReaders(handle, processStartTcs.Task, fileRedirect, disposeCts.Token); - } - try { - handle.Start(); + if (readStandardHandles) + { + handle.StartInfo.RedirectStandardOutput = true; + handle.StartInfo.RedirectStandardError = true; - processStartTcs.SetResult(); + disposeCts = new CancellationTokenSource(); + readTask = ConsumeReaders(handle, processStartTcs.Task, fileRedirect, disposeCts.Token); + } + + try + { + handle.Start(); + + processStartTcs.SetResult(); + } + catch (Exception ex) + { + processStartTcs.SetException(ex); + throw; + } + + var process = new Process( + processFeatures, + handle, + disposeCts, + readTask, + loggerFactory.CreateLogger(), + false); + + return process; } - catch (Exception ex) + catch { - processStartTcs.SetException(ex); + disposeCts?.Dispose(); throw; } - - var process = new Process( - processFeatures, - handle, - disposeCts, - await lifetimeTaskTask, // won't block - readTask, - loggerFactory.CreateLogger(), - false); - - return process; } catch { @@ -175,66 +180,6 @@ namespace Tgstation.Server.Host.System return CreateFromExistingHandle(handle); } - /// - /// Wrapper for to safely provide the process ID. - /// - /// The to attach an exit handler to. - /// A that completes once the process represented by launches. - /// The result of the call to . - async Task> AttachExitHandlerBeforeLaunch(global::System.Diagnostics.Process handle, Task startupTask) - { - var id = -1; - var result = AttachExitHandler(handle, () => id); - await startupTask; - id = handle.Id; - return result; - } - - /// - /// Attach an asychronous exit handler to a given process . - /// - /// The to attach an exit handler to. - /// A that can be called to get the safely. - /// A that completes with the exit code of the process represented by . - Task AttachExitHandler(global::System.Diagnostics.Process handle, Func idProvider) - { - handle.EnableRaisingEvents = true; - - var tcs = new TaskCompletionSource(); - void ExitHandler(object sender, EventArgs args) - { - var id = idProvider(); - try - { - try - { - var exitCode = handle.ExitCode; - - // Try because this can be invoked twice for weird reasons - if (tcs.TrySetResult(exitCode)) - logger.LogTrace("PID {pid} termination event completed", id); - else - logger.LogTrace("Ignoring duplicate PID {pid} termination event", id); - } - catch (InvalidOperationException ex) - { - if (!tcs.Task.IsCompleted) - throw; - - logger.LogTrace(ex, "Ignoring expected PID {pid} exit handler exception!", id); - } - } - catch (Exception ex) - { - logger.LogError(ex, "PID {pid} exit handler exception!", id); - } - } - - handle.Exited += ExitHandler; - - return tcs.Task; - } - /// /// Consume the stdout/stderr streams into a . /// @@ -327,7 +272,6 @@ namespace Tgstation.Server.Host.System processFeatures, handle, null, - AttachExitHandler(handle, () => pid), null, loggerFactory.CreateLogger(), true); diff --git a/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs b/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs index 1ea6c67d38..d70881cdab 100644 --- a/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs +++ b/tests/Tgstation.Server.Host.Tests/System/TestPosixSignalHandler.cs @@ -63,7 +63,7 @@ namespace Tgstation.Server.Host.System.Tests Mock.Of(), loggerFactory.CreateLogger(), loggerFactory); - await using var subProc = await processExecutor + await using var subProc = processExecutor .LaunchProcess( "dotnet", pathToSignalTestApp, diff --git a/tests/Tgstation.Server.Tests/TestSystemInteraction.cs b/tests/Tgstation.Server.Tests/TestSystemInteraction.cs index a7b41aba76..92e7aaee00 100644 --- a/tests/Tgstation.Server.Tests/TestSystemInteraction.cs +++ b/tests/Tgstation.Server.Tests/TestSystemInteraction.cs @@ -27,7 +27,7 @@ namespace Tgstation.Server.Tests Mock.Of>(), loggerFactory); - await using var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, true, true); + await using var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, true, true); using var cts = new CancellationTokenSource(); cts.CancelAfter(3000); var exitCode = await process.Lifetime.WaitAsync(cts.Token); @@ -55,7 +55,7 @@ namespace Tgstation.Server.Tests File.Delete(tempFile); try { - await using (var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, tempFile, true, true)) + await using (var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, tempFile, true, true)) { using var cts = new CancellationTokenSource(); cts.CancelAfter(3000); diff --git a/tests/Tgstation.Server.Tests/TestVersions.cs b/tests/Tgstation.Server.Tests/TestVersions.cs index 6b04fe6b95..c5d29f21f7 100644 --- a/tests/Tgstation.Server.Tests/TestVersions.cs +++ b/tests/Tgstation.Server.Tests/TestVersions.cs @@ -410,7 +410,7 @@ namespace Tgstation.Server.Tests Assert.IsTrue(supportsCli); - await using var process = await processExecutor.LaunchProcess( + await using var process = processExecutor.LaunchProcess( ddPath, Environment.CurrentDirectory, "fake.dmb -map-threads 3 -close",