Enhance orphan detection in Aspire AppHost to be robust against PID reuse (#10673)

Copilot · web-flow · commit b167ea5cc7f1 · 2025-07-29T17:48:16.000+10:00
diff --git a/src/Aspire.Cli/DotNet/DotNetCliRunner.cs b/src/Aspire.Cli/DotNet/DotNetCliRunner.cs
@@ -8,6 +8,7 @@
 using System.Text;
 using System.Text.Json;
 using Aspire.Cli.Backchannel;
+using Aspire.Cli.Configuration;
 using Aspire.Cli.Resources;
 using Aspire.Cli.Telemetry;
 using Aspire.Hosting;
@@ -41,11 +42,17 @@ internal sealed class DotNetCliRunnerInvocationOptions
     public bool NoLaunchProfile { get; set; }
 }
 
-internal class DotNetCliRunner(ILogger<DotNetCliRunner> logger, IServiceProvider serviceProvider, AspireCliTelemetry telemetry, IConfiguration configuration) : IDotNetCliRunner
+internal class DotNetCliRunner(ILogger<DotNetCliRunner> logger, IServiceProvider serviceProvider, AspireCliTelemetry telemetry, IConfiguration configuration, IFeatures features) : IDotNetCliRunner
 {
 
     internal Func<int> GetCurrentProcessId { get; set; } = () => Environment.ProcessId;
 
+    internal Func<long> GetCurrentProcessStartTime { get; set; } = () =>
+    {
+        var startTime = Process.GetCurrentProcess().StartTime;
+        return ((DateTimeOffset)startTime).ToUnixTimeSeconds();
+    };
+
     private string GetMsBuildServerValue()
     {
         return configuration["DOTNET_CLI_USE_MSBUILD_SERVER"] ?? "true";
@@ -437,6 +444,13 @@ public virtual async Task<int> ExecuteAsync(string[] args, IDictionary<string, s
         // not exist the orphan detector will exit.
         startInfo.EnvironmentVariables[KnownConfigNames.CliProcessId] = GetCurrentProcessId().ToString(CultureInfo.InvariantCulture);
 
+        // Set the CLI process start time for robust orphan detection to prevent PID reuse issues.
+        // The AppHost will verify both PID and start time to ensure it's monitoring the correct process.
+        if (features.IsFeatureEnabled(KnownFeatures.OrphanDetectionWithTimestampEnabled, true))
+        {
+            startInfo.EnvironmentVariables[KnownConfigNames.CliProcessStarted] = GetCurrentProcessStartTime().ToString(CultureInfo.InvariantCulture);
+        }
+
         // Always set MSBUILDTERMINALLOGGER=false for all dotnet command executions to ensure consistent terminal logger behavior
         startInfo.EnvironmentVariables[KnownConfigNames.MsBuildTerminalLogger] = "false";
 
diff --git a/src/Aspire.Cli/KnownFeatures.cs b/src/Aspire.Cli/KnownFeatures.cs
@@ -10,4 +10,5 @@ internal static class KnownFeatures
     public static string UpdateNotificationsEnabled => "updateNotificationsEnabled";
     public static string MinimumSdkCheckEnabled => "minimumSdkCheckEnabled";
     public static string ExecCommandEnabled => "execCommandEnabled";
+    public static string OrphanDetectionWithTimestampEnabled => "orphanDetectionWithTimestampEnabled";
 }
diff --git a/src/Aspire.Hosting/Cli/CliOrphanDetector.cs b/src/Aspire.Hosting/Cli/CliOrphanDetector.cs
@@ -22,6 +22,28 @@ internal sealed class CliOrphanDetector(IConfiguration configuration, IHostAppli
         }
     };
 
+    internal Func<int, long, bool> IsProcessRunningWithStartTime { get; set; } = (int pid, long expectedStartTimeUnix) =>
+    {
+        try
+        {
+            var process = Process.GetProcessById(pid);
+            if (process.HasExited)
+            {
+                return false;
+            }
+            
+            // Check if the process start time matches the expected start time exactly.
+            var actualStartTimeUnix = ((DateTimeOffset)process.StartTime).ToUnixTimeSeconds();
+            return actualStartTimeUnix == expectedStartTimeUnix;
+        }
+        catch
+        {
+            // If we can't get the process and/or can't get the start time, 
+            // then we interpret both exceptions as the process not being there.
+            return false;
+        }
+    };
+
     protected override async Task ExecuteAsync(CancellationToken stoppingToken)
     {
         try
@@ -33,11 +55,32 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
                 return;
             }
 
+            // Try to get the CLI process start time for robust orphan detection
+            long? expectedStartTimeUnix = null;
+            if (configuration[KnownConfigNames.CliProcessStarted] is { } startTimeString && 
+                long.TryParse(startTimeString, out var startTimeUnix))
+            {
+                expectedStartTimeUnix = startTimeUnix;
+            }
+
             using var periodic = new PeriodicTimer(TimeSpan.FromSeconds(1), timeProvider);
 
             do
             {
-                if (!IsProcessRunning(pid))
+                bool isProcessStillRunning;
+                
+                if (expectedStartTimeUnix.HasValue)
+                {
+                    // Use robust process checking with start time verification
+                    isProcessStillRunning = IsProcessRunningWithStartTime(pid, expectedStartTimeUnix.Value);
+                }
+                else
+                {
+                    // Fall back to PID-only logic for backwards compatibility
+                    isProcessStillRunning = IsProcessRunning(pid);
+                }
+
+                if (!isProcessStillRunning)
                 {
                     lifetime.StopApplication();
                     return;
diff --git a/src/Shared/KnownConfigNames.cs b/src/Shared/KnownConfigNames.cs
@@ -27,6 +27,7 @@ internal static class KnownConfigNames
     public const string WaitForDebuggerTimeout = "ASPIRE_DEBUGGER_TIMEOUT";
     public const string UnixSocketPath = "ASPIRE_BACKCHANNEL_PATH";
     public const string CliProcessId = "ASPIRE_CLI_PID";
+    public const string CliProcessStarted = "ASPIRE_CLI_STARTED";
     public const string ForceRichConsole = "ASPIRE_FORCE_RICH_CONSOLE";
     public const string TestingDisableHttpClient = "ASPIRE_TESTING_DISABLE_HTTP_CLIENT";
 
diff --git a/tests/Aspire.Cli.Tests/DotNet/DotNetCliRunnerTests.cs b/tests/Aspire.Cli.Tests/DotNet/DotNetCliRunnerTests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Aspire.Cli.Backchannel;
+using Aspire.Cli.Configuration;
 using Aspire.Cli.DotNet;
 using Aspire.Cli.Telemetry;
 using Aspire.Cli.Tests.Utils;
@@ -34,6 +35,7 @@ public async Task DotNetCliCorrectlyAppliesNoLaunchProfileArgumentWhenSpecifiedI
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, _, _, _, _) => Assert.Contains(args, arg => arg == "--no-launch-profile"),
             42
             );
@@ -74,6 +76,7 @@ public async Task BuildAsyncAlwaysInjectsDotnetCliUseMsBuildServerEnvironmentVar
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) => 
             {
                 Assert.NotNull(env);
@@ -116,6 +119,7 @@ public async Task BuildAsyncUsesConfigurationValueForDotnetCliUseMsBuildServer()
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) => 
             {
                 Assert.NotNull(env);
@@ -148,6 +152,7 @@ public async Task RunAsyncInjectsDotnetCliUseMsBuildServerWhenNoBuildIsFalse()
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) => 
             {
                 Assert.NotNull(env);
@@ -189,6 +194,7 @@ public async Task RunAsyncDoesNotInjectDotnetCliUseMsBuildServerWhenNoBuildIsTru
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) => 
             {
                 // When noBuild is true, the original env should be passed through unchanged
@@ -233,6 +239,7 @@ public async Task RunAsyncPreservesExistingEnvironmentVariables()
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) => 
             {
                 Assert.NotNull(env);
@@ -279,6 +286,7 @@ public async Task NewProjectAsyncReturnsExitCode73WhenProjectAlreadyExists()
             provider,
             new AspireCliTelemetry(),
             provider.GetRequiredService<IConfiguration>(),
+            provider.GetRequiredService<IFeatures>(),
             (args, env, _, _, _) =>
             {
                 // Verify the arguments are correct for dotnet new
@@ -305,9 +313,10 @@ internal sealed class AssertingDotNetCliRunner(
     IServiceProvider serviceProvider,
     AspireCliTelemetry telemetry,
     IConfiguration configuration,
+    IFeatures features,
     Action<string[], IDictionary<string, string>?, DirectoryInfo, TaskCompletionSource<IAppHostBackchannel>?, DotNetCliRunnerInvocationOptions> assertionCallback,
     int exitCode
-    ) : DotNetCliRunner(logger, serviceProvider, telemetry, configuration)
+    ) : DotNetCliRunner(logger, serviceProvider, telemetry, configuration, features)
 {
     public override Task<int> ExecuteAsync(string[] args, IDictionary<string, string>? env, DirectoryInfo workingDirectory, TaskCompletionSource<IAppHostBackchannel>? backchannelCompletionSource, DotNetCliRunnerInvocationOptions options, CancellationToken cancellationToken)
     {
diff --git a/tests/Aspire.Cli.Tests/Hosting/CliOrphanDetectorTests.cs b/tests/Aspire.Cli.Tests/Hosting/CliOrphanDetectorTests.cs
@@ -48,6 +48,122 @@ public async Task CliOrphanDetectorCallsStopIfEnvironmentVariablePresentAndProce
         Assert.True(await stopSignalChannel.Reader.WaitToReadAsync());
     }
 
+    [Fact]
+    public async Task CliOrphanDetectorUsesTimestampDetectionWhenStartTimeProvided()
+    {
+        var expectedStartTime = DateTime.Now.AddMinutes(-5);
+        var expectedStartTimeUnixSeconds = ((DateTimeOffset)expectedStartTime).ToUnixTimeSeconds();
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> 
+            { 
+                { "ASPIRE_CLI_PID", "1111" },
+                { "ASPIRE_CLI_STARTED", expectedStartTimeUnixSeconds.ToString() }
+            })    
+            .Build();
+
+        var stopSignalChannel = Channel.CreateUnbounded<bool>();
+        var lifetime = new HostLifetimeStub(() => stopSignalChannel.Writer.TryWrite(true));
+
+        var detector = new CliOrphanDetector(configuration, lifetime, TimeProvider.System);
+        detector.IsProcessRunningWithStartTime = (pid, startTime) => false;
+
+        await detector.StartAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5));
+        Assert.True(await stopSignalChannel.Reader.WaitToReadAsync());
+    }
+
+    [Fact]
+    public async Task CliOrphanDetectorFallsBackToPidOnlyWhenStartTimeInvalid()
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> 
+            { 
+                { "ASPIRE_CLI_PID", "1111" },
+                { "ASPIRE_CLI_STARTED", "invalid_start_time" }
+            })    
+            .Build();
+
+        var stopSignalChannel = Channel.CreateUnbounded<bool>();
+        var lifetime = new HostLifetimeStub(() => stopSignalChannel.Writer.TryWrite(true));
+
+        var detector = new CliOrphanDetector(configuration, lifetime, TimeProvider.System);
+        detector.IsProcessRunning = _ => false;
+
+        await detector.StartAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5));
+        Assert.True(await stopSignalChannel.Reader.WaitToReadAsync());
+    }
+
+    [Fact]
+    public async Task CliOrphanDetectorContinuesRunningWhenProcessAliveWithCorrectStartTime()
+    {
+        var expectedStartTime = DateTime.Now.AddMinutes(-5);
+        var expectedStartTimeUnix = ((DateTimeOffset)expectedStartTime).ToUnixTimeSeconds();
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> 
+            { 
+                { "ASPIRE_CLI_PID", "1111" },
+                { "ASPIRE_CLI_STARTED", expectedStartTimeUnix.ToString() }
+            })    
+            .Build();
+        var fakeTimeProvider = new FakeTimeProvider(DateTimeOffset.Now);
+
+        var stopSignalChannel = Channel.CreateUnbounded<bool>();
+        var processRunningChannel = Channel.CreateUnbounded<int>();
+
+        var lifetime = new HostLifetimeStub(() => stopSignalChannel.Writer.TryWrite(true));
+        var detector = new CliOrphanDetector(configuration, lifetime, fakeTimeProvider);
+        
+        var processRunningCallCounter = 0;
+        detector.IsProcessRunningWithStartTime = (pid, startTime) => {
+            Assert.True(processRunningChannel.Writer.TryWrite(++processRunningCallCounter));
+            return processRunningCallCounter < 3; // Process dies after 3 checks
+        };
+
+        await detector.StartAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5));
+
+        // Verify process is checked first time
+        Assert.True(await processRunningChannel.Reader.WaitToReadAsync());
+        fakeTimeProvider.Advance(TimeSpan.FromSeconds(1));
+
+        // Second check
+        Assert.True(await processRunningChannel.Reader.WaitToReadAsync());
+        fakeTimeProvider.Advance(TimeSpan.FromSeconds(1));
+
+        // Third check (process dies)
+        Assert.True(await processRunningChannel.Reader.WaitToReadAsync());
+        Assert.Equal(3, processRunningCallCounter);
+
+        // Should stop the application
+        Assert.True(await stopSignalChannel.Reader.WaitToReadAsync());
+    }
+
+    [Fact]
+    public async Task CliOrphanDetectorStopsWhenProcessHasDifferentStartTime()
+    {
+        var expectedStartTime = DateTime.Now.AddMinutes(-5);
+        var expectedStartTimeUnixString = ((DateTimeOffset)expectedStartTime).ToUnixTimeSeconds().ToString();
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> 
+            { 
+                { "ASPIRE_CLI_PID", "1111" },
+                { "ASPIRE_CLI_STARTED", expectedStartTimeUnixString }
+            })    
+            .Build();
+
+        var stopSignalChannel = Channel.CreateUnbounded<bool>();
+        var lifetime = new HostLifetimeStub(() => stopSignalChannel.Writer.TryWrite(true));
+
+        var detector = new CliOrphanDetector(configuration, lifetime, TimeProvider.System);
+        
+        // Simulate process with different start time (PID reuse scenario)
+        detector.IsProcessRunningWithStartTime = (pid, startTime) => {
+            // Process exists but has different start time - indicates PID reuse
+            return false;
+        };
+
+        await detector.StartAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5));
+        Assert.True(await stopSignalChannel.Reader.WaitToReadAsync());
+    }
+
     [Fact]
     public async Task CliOrphanDetectorAfterTheProcessWasRunningForAWhileThenStops()
     {
diff --git a/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs b/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs
@@ -203,7 +203,8 @@ public IProjectLocator CreateDefaultProjectLocatorFactory(IServiceProvider servi
         var logger = serviceProvider.GetRequiredService<ILogger<DotNetCliRunner>>();
         var telemetry = serviceProvider.GetRequiredService<AspireCliTelemetry>();
         var configuration = serviceProvider.GetRequiredService<IConfiguration>();
-        return new DotNetCliRunner(logger, serviceProvider, telemetry, configuration);
+        var features = serviceProvider.GetRequiredService<IFeatures>();
+        return new DotNetCliRunner(logger, serviceProvider, telemetry, configuration, features);
     };
 
     public Func<IServiceProvider, IDotNetSdkInstaller> DotNetSdkInstallerFactory { get; set; } = (IServiceProvider serviceProvider) =>

Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,5 @@ internal static class KnownFeatures`
`10`	`10`	`public static string UpdateNotificationsEnabled => "updateNotificationsEnabled";`
`11`	`11`	`public static string MinimumSdkCheckEnabled => "minimumSdkCheckEnabled";`
`12`	`12`	`public static string ExecCommandEnabled => "execCommandEnabled";`
	`13`	`+ public static string OrphanDetectionWithTimestampEnabled => "orphanDetectionWithTimestampEnabled";`
`13`	`14`	`}`