Clean up socket file after stopping running AppHost instance (#17587)

src/Aspire.Cli/Commands/StopCommand.cs

@@ -206,6 +206,12 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
 
         if (stopped)
         {
+            // ProcessShutdownService only reports success once it has confirmed the AppHost process has
+            // terminated, so the socket's owner is gone and the file is safe to remove by exact path. Doing
+            // it here is the primary guard against a stale socket tripping up later commands: the AppHost's own
+            // cleanup is skipped if it crashes hard, and the orphan-pruning backstop misfires on Windows when the
+            // dead PID is reused (https://github.com/microsoft/aspire/issues/17587).
+            AppHostHelper.TryDeleteSocketFile(connection.SocketPath, _logger);
             InteractionService.DisplaySuccess(string.Format(CultureInfo.CurrentCulture, StopCommandStrings.AppHostStoppedSuccessfully, appHostIdentifier));
             return CompleteStopActivity(activity, CliExitCodes.Success);
         }

src/Aspire.Cli/Projects/RunningInstanceManager.cs

@@ -7,6 +7,7 @@
 using Aspire.Cli.Interaction;
 using Aspire.Cli.Resources;
 using Aspire.Cli.Telemetry;
+using Aspire.Cli.Utils;
 using Microsoft.Extensions.Logging;
 
 namespace Aspire.Cli.Projects;
@@ -69,6 +70,9 @@ public async Task<bool> StopRunningInstanceAsync(string socketPath, Cancellation
             if (stopped)
             {
                 _interactionService.DisplaySuccess(RunCommandStrings.RunningInstanceStopped);
+                // Clean up the socket file now that the instance has been stopped so a later command does not
+                // rediscover it and try to connect to a dead process (https://github.com/microsoft/aspire/issues/17587).
+                AppHostHelper.TryDeleteSocketFile(socketPath, _logger);
             }
             else
             {

src/Aspire.Cli/Utils/AppHostHelper.cs

@@ -144,7 +144,7 @@ internal static string[] FindMatchingNonOrphanedSockets(
         // off the physical path (for example "/private/tmp/..." on macOS).
         var resolvedPath = PathNormalizer.ResolveSymlinks(appHostPath);
         var matchingSockets = BackchannelConstants.FindMatchingSockets(resolvedPath, homeDirectory);
-        var remainingSockets = PruneOrphanedSockets(matchingSockets, currentPid, out var deletedCount);
+        var remainingSockets = PruneOrphanedSockets(matchingSockets, currentPid, logger, out var deletedCount);
         if (deletedCount > 0)
         {
             logger.LogDebug("Cleaned up {Count} orphaned AppHost socket(s).", deletedCount);
@@ -153,10 +153,50 @@ internal static string[] FindMatchingNonOrphanedSockets(
         return remainingSockets;
     }
 
+    /// <summary>
+    /// Best-effort deletion of an auxiliary backchannel socket file whose owning AppHost instance is no longer running.
+    /// </summary>
+    /// <remarks>
+    /// This is the single CLI-side socket-cleanup path. It is used both at stop time (<c>aspire stop</c> and
+    /// <see cref="Projects.RunningInstanceManager"/>, once the process is confirmed terminated) and by the orphan-pruning
+    /// backstop in <see cref="PruneOrphanedSockets"/> (once the owning PID is observed to be dead). Leaving the socket
+    /// behind causes a later command (for example <c>aspire add</c> or <c>aspire stop</c>) to rediscover it via
+    /// <see cref="FindMatchingNonOrphanedSockets"/> and attempt to connect to a now-dead process. This is most visible on
+    /// Windows, where the dead AppHost's PID can be reused so the orphan-pruning heuristic still believes the process is
+    /// alive. Deleting by exact path at stop time sidesteps that PID heuristic entirely. The caller must only invoke this
+    /// once it knows the owning process has terminated. The AppHost-side socket cleanup in <c>Aspire.Hosting</c>
+    /// (<c>BackchannelService</c>/<c>AuxiliaryBackchannelService</c>) deliberately does not share this method: it lives in a
+    /// different assembly and deletes a socket the AppHost itself owns, so it is not exposed to the PID-reuse problem. See
+    /// https://github.com/microsoft/aspire/issues/17587.
+    /// </remarks>
+    /// <param name="socketPath">The path to the auxiliary backchannel socket file.</param>
+    /// <param name="logger">Logger used for diagnostic output.</param>
+    /// <returns><see langword="true"/> if the socket file was deleted; otherwise <see langword="false"/>.</returns>
+    internal static bool TryDeleteSocketFile(string socketPath, ILogger logger)
+    {
+        try
+        {
+            if (File.Exists(socketPath))
+            {
+                File.Delete(socketPath);
+                logger.LogDebug("Cleaned up backchannel socket file: {SocketPath}", socketPath);
+                return true;
+            }
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        {
+            // A failed delete is non-fatal: the next command's orphan-pruning pass will attempt cleanup again. We swallow
+            // the same exception types as the other socket-cleanup sites for consistency.
+            logger.LogDebug(ex, "Failed to clean up backchannel socket file (this may be safe to ignore): {SocketPath}", socketPath);
+        }
+
+        return false;
+    }
+
     /// <summary>
     /// Deletes PID-qualified socket files whose owning process has exited and returns sockets that should still be probed.
     /// </summary>
-    private static string[] PruneOrphanedSockets(string[] socketPaths, int currentPid, out int deletedCount)
+    private static string[] PruneOrphanedSockets(string[] socketPaths, int currentPid, ILogger logger, out int deletedCount)
     {
         deletedCount = 0;
         var remainingSockets = new List<string>(socketPaths.Length);
@@ -173,14 +213,10 @@ private static string[] PruneOrphanedSockets(string[] socketPaths, int currentPi
                     //   auxi.sock.{hash}.{instanceHash}.{pid}
                     // After a crash or reboot these files can remain, and connecting to them
                     // reports "connection refused" even though there is no AppHost to stop.
-                    try
+                    if (TryDeleteSocketFile(socketPath, logger))
                     {
-                        File.Delete(socketPath);
                         deletedCount++;
                     }
-                    catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
-                    {
-                    }
 
                     continue;
                 }

tests/Aspire.Cli.Tests/Commands/StopCommandTests.cs

@@ -319,6 +319,44 @@ public async Task StopCommand_NoRunningAppHosts_ReturnsSuccess(string commandLin
         Assert.Equal(SharedCommandStrings.AppHostNotRunning, displayedMessage.Message);
     }
 
+    [Fact]
+    public async Task StopCommand_DeletesSocketFile_AfterSuccessfulStop()
+    {
+        // Regression test for https://github.com/microsoft/aspire/issues/17587: 'aspire stop' is the command
+        // that leaks the socket, so it must delete the socket file once the AppHost has been confirmed stopped.
+        // Otherwise a later command rediscovers the stale socket and tries to connect to a dead process.
+        using var workspace = TemporaryWorkspace.Create(outputHelper);
+        var interactionService = new TestInteractionService();
+
+        // The AppHost is reported with a process id that does not exist, so ProcessShutdownService observes
+        // termination immediately and the stop reaches the socket-cleanup branch.
+        var appHostPath = Path.Combine(workspace.WorkspaceRoot.FullName, "App1", "App1.AppHost.csproj");
+        var socketPath = Path.Combine(workspace.WorkspaceRoot.FullName, "a.sock");
+        File.WriteAllText(socketPath, "");
+        Assert.True(File.Exists(socketPath));
+
+        var connection = CreateConnection(appHostPath, int.MaxValue - 9);
+        connection.SocketPath = socketPath;
+
+        var monitor = new TestAuxiliaryBackchannelMonitor();
+        monitor.AddConnection("hash1", socketPath, connection);
+
+        var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options =>
+        {
+            options.InteractionServiceFactory = _ => interactionService;
+            options.AuxiliaryBackchannelMonitorFactory = _ => monitor;
+        });
+        using var provider = services.BuildServiceProvider();
+
+        var command = provider.GetRequiredService<RootCommand>();
+        var result = command.Parse("stop");
+
+        var exitCode = await result.InvokeAsync().DefaultTimeout();
+
+        Assert.Equal(CliExitCodes.Success, exitCode);
+        Assert.False(File.Exists(socketPath));
+    }
+
     private static TestAppHostAuxiliaryBackchannel CreateConnection(string appHostPath, int processId, bool isInScope = true)
     {
         return new TestAppHostAuxiliaryBackchannel

tests/Aspire.Cli.Tests/Projects/RunningInstanceManagerTests.cs

@@ -0,0 +1,163 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net.Sockets;
+using Aspire.Cli.Backchannel;
+using Aspire.Cli.Projects;
+using Aspire.Cli.Telemetry;
+using Aspire.Cli.Tests.TestServices;
+using Microsoft.AspNetCore.InternalTesting;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Logging.Abstractions;
+using StreamJsonRpc;
+
+namespace Aspire.Cli.Tests.Projects;
+
+public class RunningInstanceManagerTests
+{
+    [Fact]
+    public async Task StopRunningInstanceAsync_DeletesSocketFile_WhenStopSucceeds()
+    {
+        // Regression test for https://github.com/microsoft/aspire/issues/17587.
+        //
+        // After a successful stop, the CLI must delete the auxiliary backchannel socket file. If the
+        // file is left behind, a later command (e.g. 'aspire add' or 'aspire stop') rediscovers it via
+        // FindMatchingNonOrphanedSockets and tries to connect to a now-defunct process, failing with
+        // "Unable to stop one or more running Aspire AppHost instances". This is most visible on Windows
+        // where the dead AppHost's PID can be reused (so the orphan-pruning heuristic believes the
+        // process is still alive), which is why a unit test is the deterministic, cross-platform guard.
+
+        // The AppHost process reported by the fake backchannel must be one MonitorProcessesForTerminationAsync
+        // observes as already terminated so StopRunningInstanceAsync reaches the socket-cleanup branch. A fabricated,
+        // guaranteed-nonexistent PID makes Process.GetProcessById throw ArgumentException, which the monitor treats as
+        // "terminated". We deliberately do NOT start a real process and reuse its exited PID: the OS can recycle that
+        // exact PID for an unrelated live process before the monitor checks, which would make the monitor loop for the
+        // full timeout, return stopped == false, and flake the cleanup assertion.
+        const int exitedProcessId = int.MaxValue - 9;
+
+        var socketDirectory = Directory.CreateTempSubdirectory("aspire-rim-");
+        try
+        {
+            // Keep the socket path short: a UnixDomainSocketEndPoint is limited to ~104 bytes on macOS.
+            var socketPath = Path.Combine(socketDirectory.FullName, "a.sock");
+            using var server = TestAuxiliaryBackchannelUdsServer.Start(socketPath, exitedProcessId);
+
+            // Binding the server creates the socket file on disk.
+            Assert.True(File.Exists(socketPath));
+
+            var manager = new RunningInstanceManager(NullLogger.Instance, new TestInteractionService(), TimeProvider.System, new ProfilingTelemetry(new ConfigurationBuilder().Build()));
+
+            var stopped = await manager.StopRunningInstanceAsync(socketPath, CancellationToken.None).DefaultTimeout();
+
+            Assert.True(stopped);
+            Assert.True(server.Target.StopRequested);
+            // The fix: the socket file must be removed once the instance has been stopped.
+            Assert.False(File.Exists(socketPath));
+        }
+        finally
+        {
+            socketDirectory.Delete(recursive: true);
+        }
+    }
+
+    private sealed class TestAuxiliaryBackchannelUdsServer : IDisposable
+    {
+        private readonly Socket _listener;
+        private readonly CancellationTokenSource _cts = new();
+        private readonly List<IDisposable> _disposables = [];
+
+        private TestAuxiliaryBackchannelUdsServer(string socketPath, int appHostProcessId)
+        {
+            Target = new StoppableAppHostRpcTarget(appHostProcessId);
+            _listener = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified);
+            _listener.Bind(new UnixDomainSocketEndPoint(socketPath));
+            _listener.Listen(1);
+        }
+
+        public StoppableAppHostRpcTarget Target { get; }
+
+        public static TestAuxiliaryBackchannelUdsServer Start(string socketPath, int appHostProcessId)
+        {
+            var server = new TestAuxiliaryBackchannelUdsServer(socketPath, appHostProcessId);
+            _ = server.AcceptConnectionAsync();
+            return server;
+        }
+
+        private async Task AcceptConnectionAsync()
+        {
+            try
+            {
+                var serverSocket = await _listener.AcceptAsync(_cts.Token).ConfigureAwait(false);
+                var serverStream = new NetworkStream(serverSocket, ownsSocket: true);
+                var messageHandler = new HeaderDelimitedMessageHandler(serverStream, serverStream, BackchannelJsonSerializerContext.CreateRpcMessageFormatter());
+                var rpc = new JsonRpc(messageHandler, Target);
+                rpc.StartListening();
+                _disposables.Add(rpc);
+                _disposables.Add(messageHandler);
+                _disposables.Add(serverStream);
+            }
+            catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException)
+            {
+                // Expected when the server is disposed before/while a connection is accepted.
+            }
+        }
+
+        public void Dispose()
+        {
+            _cts.Cancel();
+            foreach (var disposable in _disposables)
+            {
+                disposable.Dispose();
+            }
+
+            _listener.Dispose();
+            _cts.Dispose();
+        }
+    }
+
+    private sealed class StoppableAppHostRpcTarget
+    {
+        private readonly int _processId;
+        private readonly string[] _capabilities =
+        [
+            AuxiliaryBackchannelCapabilities.V1,
+            AuxiliaryBackchannelCapabilities.V2
+        ];
+
+        public StoppableAppHostRpcTarget(int processId)
+        {
+            _processId = processId;
+        }
+
+        public bool StopRequested { get; private set; }
+
+        public Task<AppHostInformation> GetAppHostInformationAsync(CancellationToken cancellationToken = default)
+        {
+            _ = cancellationToken;
+
+            return Task.FromResult(new AppHostInformation
+            {
+                AppHostPath = "/path/to/AppHost.csproj",
+                ProcessId = _processId
+            });
+        }
+
+        public Task<GetCapabilitiesResponse> GetCapabilitiesAsync(GetCapabilitiesRequest? request = null, CancellationToken cancellationToken = default)
+        {
+            _ = request;
+            _ = cancellationToken;
+
+            return Task.FromResult(new GetCapabilitiesResponse
+            {
+                Capabilities = _capabilities
+            });
+        }
+
+        public Task StopAppHostAsync(CancellationToken cancellationToken = default)
+        {
+            _ = cancellationToken;
+            StopRequested = true;
+            return Task.CompletedTask;
+        }
+    }
+}