diff --git a/src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs b/src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs index fce86dab29..ada58aceaa 100644 --- a/src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs +++ b/src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs @@ -9,12 +9,12 @@ public class SetupInstanceTests { [Test] - public void Should_not_throw_on_0_exit_code() => Assert.DoesNotThrow(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "", Timeout.Infinite)); + public void Should_not_throw_on_0_exit_code() => Assert.DoesNotThrow(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "")); [Test] public void Should_capture_and_rethrow_failures() { - var ex = Assert.Throws(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "fail", Timeout.Infinite)); + var ex = Assert.Throws(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "fail")); Assert.That(ex.Message, Does.Contain("Fake exception")); } @@ -22,20 +22,9 @@ public void Should_capture_and_rethrow_failures() [Test] public void Should_capture_and_rethrow_non_zero_exit_codes() { - var ex = Assert.Throws(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "non-zero-exit-code", Timeout.Infinite)); + var ex = Assert.Throws(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "non-zero-exit-code")); Assert.That(ex.Message, Does.Contain("returned a non-zero exit code: 3")); Assert.That(ex.Message, Does.Contain("Fake non zero exit code message")); } - - [Test] - public void Should_not_kill_the_process_if_wait_time_is_execeeded() - { - var process = InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "delay", 10); - - Assert.That(process.HasExited, Is.False); - - process.Kill(); - process.WaitForExit(); - } } \ No newline at end of file diff --git a/src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs b/src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs index 6ea0fc3898..749e7068fe 100644 --- a/src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs +++ b/src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs @@ -35,12 +35,10 @@ static void Run(string installPath, string exeName, string instanceName, bool sk args += " --skip-queue-creation"; } - // we will wait "forever" since that is the most safe action right not. We will leave it up to the setup code in the instances to make sure it won't run forever. - // If/when provide better UI experience we might revisit this and potentially find a way for the installer to control the timeout. - Run(installPath, exeName, instanceName, args, Timeout.Infinite); + Run(installPath, exeName, instanceName, args); } - internal static Process Run(string installPath, string exeName, string instanceName, string args, int timeout) + internal static void Run(string installPath, string exeName, string instanceName, string args) { var processStartupInfo = new ProcessStartInfo { @@ -56,15 +54,20 @@ internal static Process Run(string installPath, string exeName, string instanceN var p = Process.Start(processStartupInfo) ?? throw new Exception($"Attempt to launch {exeName} failed."); - p.WaitForExit(timeout); + // Reading std err needs to happen before waiting to avoid the risk of a deadlock. + // See https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.standardoutput?view=net-9.0&redirectedfrom=MSDN#remarks for more details. + // Note that this will wait forever, should we want to avoid that the async ProcessStartInfo.ErrorDataReceived API needs to be used. + var error = p.StandardError.ReadToEnd(); + + // we will wait "forever" since that is the most safe action right not. We will leave it up to the setup code in the instances to make sure it won't run forever. + // If/when provide better UI experience we might revisit this and potentially find a way for the installer to control the timeout. + p.WaitForExit(); if (!p.HasExited || p.ExitCode == 0) { - return p; + return; } - var error = p.StandardError.ReadToEnd(); - throw new Exception($"{exeName} returned a non-zero exit code: {p.ExitCode}. This typically indicates a configuration error. The error output was:\r\n {error}"); } } \ No newline at end of file diff --git a/src/SetupProcessFake/Program.cs b/src/SetupProcessFake/Program.cs index a4232846d8..6495e82dcd 100644 --- a/src/SetupProcessFake/Program.cs +++ b/src/SetupProcessFake/Program.cs @@ -10,11 +10,4 @@ return 3; } -if (args.Any(a => a == "delay")) -{ -#pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task - await Task.Delay(TimeSpan.FromSeconds(5)); -#pragma warning restore CA2007 // Consider calling ConfigureAwait on the awaited task -} - return 0;