Do not kill instance setup to minimize the risk for a corrupted database

src/ServiceControl.sln

@@ -183,6 +183,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ServiceControl.Transports.P
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ServiceControl.Transports.PostgreSql.Tests", "ServiceControl.Transports.PostgreSql.Tests\ServiceControl.Transports.PostgreSql.Tests.csproj", "{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SetupProcessFake", "SetupProcessFake\SetupProcessFake.csproj", "{36D53BA0-C1E1-4D74-81AE-C33B40C84958}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -997,6 +999,18 @@ Global
 		{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x64.Build.0 = Release|Any CPU
 		{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x86.ActiveCfg = Release|Any CPU
 		{18DBEEF5-42EE-4C1D-A05B-87B21C067D53}.Release|x86.Build.0 = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x64.Build.0 = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Debug|x86.Build.0 = Debug|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|Any CPU.Build.0 = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x64.ActiveCfg = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x64.Build.0 = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x86.ActiveCfg = Release|Any CPU
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -1080,6 +1094,7 @@ Global
 		{51F5504E-E915-40EC-B96E-CA700A57982C} = {80C55E70-4B7A-4EF2-BB9E-C42F8DB0495D}
 		{448CBDCF-718D-4BC7-8F7C-099C9A362B59} = {A21A1A89-0B07-4E87-8E3C-41D9C280DCB8}
 		{18DBEEF5-42EE-4C1D-A05B-87B21C067D53} = {E0E45F22-35E3-4AD8-B09E-EFEA5A2F18EE}
+		{36D53BA0-C1E1-4D74-81AE-C33B40C84958} = {927A078A-E271-4878-A153-86D71AE510E2}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {3B9E5B72-F580-465A-A22C-2D2148AF4EB4}

src/ServiceControlInstaller.Engine.UnitTests/ServiceControlInstaller.Engine.UnitTests.csproj

@@ -6,6 +6,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\ServiceControlInstaller.Engine\ServiceControlInstaller.Engine.csproj" />
+    <ProjectReference Include="..\SetupProcessFake\SetupProcessFake.csproj" />
     <ProjectReference Include="..\TestHelper\TestHelper.csproj" />
   </ItemGroup>
 

src/ServiceControlInstaller.Engine.UnitTests/Setup/SetupInstanceTests.cs

@@ -0,0 +1,41 @@
+namespace ServiceControlInstaller.Engine.UnitTests.Setup;
+
+using System;
+using System.Threading;
+using Engine.Setup;
+using NUnit.Framework;
+
+[TestFixture]
+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));
+
+    [Test]
+    public void Should_capture_and_rethrow_failures()
+    {
+        var ex = Assert.Throws<Exception>(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "fail", Timeout.Infinite));
+
+        Assert.That(ex.Message, Does.Contain("Fake exception"));
+    }
+
+    [Test]
+    public void Should_capture_and_rethrow_non_zero_exit_codes()
+    {
+        var ex = Assert.Throws<Exception>(() => InstanceSetup.Run(TestContext.CurrentContext.WorkDirectory, "SetupProcessFake.exe", "test", "non-zero-exit-code", Timeout.Infinite));
+
+        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();
+    }
+}

src/ServiceControlInstaller.Engine/Setup/InstanceSetup.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Diagnostics;
 using System.IO;
+using System.Threading;
 using Instances;
 
 static class InstanceSetup
@@ -34,6 +35,13 @@ 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);
+    }
+
+    internal static Process Run(string installPath, string exeName, string instanceName, string args, int timeout)
+    {
         var processStartupInfo = new ProcessStartInfo
         {
             CreateNoWindow = true,
@@ -46,25 +54,17 @@ static void Run(string installPath, string exeName, string instanceName, bool sk
 
         processStartupInfo.EnvironmentVariables.Add("INSTANCE_NAME", instanceName);
 
-        var p = Process.Start(processStartupInfo);
-        if (p != null)
-        {
-            var error = p.StandardError.ReadToEnd();
-            p.WaitForExit((int)TimeSpan.FromMinutes(1).TotalMilliseconds);
-            if (!p.HasExited)
-            {
-                p.Kill();
-                throw new TimeoutException($"Timed out waiting for {exeName} to perform setup. This usually indicates a configuration error.");
-            }
+        var p = Process.Start(processStartupInfo) ?? throw new Exception($"Attempt to launch {exeName} failed.");
 
-            if (p.ExitCode != 0)
-            {
-                throw new Exception($"{exeName} threw an error when performing setup. This typically indicates a configuration error. The error output from {exeName} was:\r\n {error}");
-            }
-        }
-        else
+        p.WaitForExit(timeout);
+
+        if (!p.HasExited || p.ExitCode == 0)
         {
-            throw new Exception($"Attempt to launch {exeName} failed.");
+            return p;
         }
+
+        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}");
     }
 }

src/SetupProcessFake/Program.cs

@@ -0,0 +1,20 @@
+if (args.Any(a => a == "fail"))
+{
+    throw new Exception("Fake exception");
+}
+
+if (args.Any(a => a == "non-zero-exit-code"))
+{
+    Console.Error.WriteLine("Fake non zero exit code message");
+
+    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;

src/SetupProcessFake/SetupProcessFake.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+    <PropertyGroup>
+        <OutputType>Exe</OutputType>
+        <TargetFramework>net8.0</TargetFramework>
+        <ImplicitUsings>enable</ImplicitUsings>
+        <Nullable>enable</Nullable>
+    </PropertyGroup>
+
+</Project>