diff --git a/src/Aspire.Cli/Processes/ProcessShutdownService.cs b/src/Aspire.Cli/Processes/ProcessShutdownService.cs index 65064c3484d..d0089a68d54 100644 --- a/src/Aspire.Cli/Processes/ProcessShutdownService.cs +++ b/src/Aspire.Cli/Processes/ProcessShutdownService.cs @@ -46,14 +46,18 @@ public async Task StopAppHostAsync( return requestRpcStopAsync is not null && await TryRequestRpcStopAsync(requestRpcStopAsync, cancellationToken).ConfigureAwait(false); } - var processesToMonitor = new List { new(appHostInfo.ProcessId, appHostInfo.StartedAt) }; + var appHostProcess = new ProcessTarget(appHostInfo.ProcessId, appHostInfo.StartedAt); + var processesToForceKill = new List { appHostProcess }; if (appHostInfo.CliProcessId is int cliPid) { - processesToMonitor.Add(new ProcessTarget(cliPid, appHostInfo.CliStartedAt)); + // The CLI process is a shutdown handle, not the success condition. On Unix it can remain + // observable until its parent reaps it after the AppHost has already stopped. + processesToForceKill.Add(new ProcessTarget(cliPid, appHostInfo.CliStartedAt)); } return await StopProcessesAsync( - processesToMonitor, + processesToMonitor: [appHostProcess], + processesToForceKill, token => RequestAppHostGracefulShutdownAsync(appHostInfo, requestRpcStopAsync, token), cancellationToken).ConfigureAwait(false); } @@ -62,20 +66,55 @@ internal async Task StopProcessesAsync( IReadOnlyCollection processesToMonitorAndKill, Func> requestGracefulShutdownAsync, CancellationToken cancellationToken) + { + return await StopProcessesAsync( + processesToMonitorAndKill, + processesToMonitorAndKill, + requestGracefulShutdownAsync, + cancellationToken).ConfigureAwait(false); + } + + private async Task StopProcessesAsync( + IReadOnlyCollection processesToMonitor, + IReadOnlyCollection processesToForceKill, + Func> requestGracefulShutdownAsync, + CancellationToken cancellationToken) { var gracefulShutdownRequested = await TryRequestGracefulShutdownAsync(requestGracefulShutdownAsync, cancellationToken).ConfigureAwait(false); - if (gracefulShutdownRequested && await MonitorProcessesForTerminationAsync(processesToMonitorAndKill, cancellationToken).ConfigureAwait(false)) + if (gracefulShutdownRequested && await MonitorProcessesForTerminationAsync(processesToMonitor, cancellationToken).ConfigureAwait(false)) { + ForceKillRemainingProcesses(processesToForceKill.Except(processesToMonitor), afterTimeout: false); return true; } - foreach (var process in processesToMonitorAndKill.Distinct()) + ForceKillRemainingProcesses(processesToForceKill, afterTimeout: true); + + return await MonitorProcessesForTerminationAsync(processesToMonitor, cancellationToken).ConfigureAwait(false); + } + + private void ForceKillRemainingProcesses(IEnumerable processes, bool afterTimeout) + { + // On Unix the AppHost's process tree does not include DCP (it is launched in its own + // session/process group), so a tree kill of the AppHost is safe: DCP will detect the + // AppHost exiting and gracefully tear down its own children. The same applies to the + // launcher CLI handle - any leftover `dotnet run` / AppHost descendants get cleaned up. + // On Windows DCP is an in-tree descendant of the AppHost, so we must single-process-kill + // here and rely on the graceful DCP `stop-process-tree` path for orderly resource cleanup. + var killEntireProcessTree = !OperatingSystem.IsWindows(); + + foreach (var process in processes.Distinct()) { - logger.LogWarning("Process {Pid} did not stop gracefully within timeout. Forcing process to terminate.", process.Pid); - ProcessSignaler.ForceKill(process.Pid, process.StartTime, logger); - } + if (afterTimeout) + { + logger.LogWarning("Process {Pid} did not stop gracefully within timeout. Forcing process to terminate.", process.Pid); + } + else + { + logger.LogDebug("Forcing remaining shutdown handle process {Pid} to terminate.", process.Pid); + } - return await MonitorProcessesForTerminationAsync(processesToMonitorAndKill, cancellationToken).ConfigureAwait(false); + ProcessSignaler.ForceKill(process.Pid, process.StartTime, logger, killEntireProcessTree); + } } private async Task RequestAppHostGracefulShutdownAsync( @@ -83,6 +122,28 @@ private async Task RequestAppHostGracefulShutdownAsync( Func>? requestRpcStopAsync, CancellationToken cancellationToken) { + if (!OperatingSystem.IsWindows()) + { + // Signal the AppHost directly with SIGTERM. The AppHost catches SIGTERM via + // Microsoft.Extensions.Hosting and invokes IHostApplicationLifetime.StopApplication, + // which gives DCP and all in-process resources the orderly shutdown they expect. + // + // Routing the graceful signal through the launcher CLI (CliProcessId) cascades via + // `dotnet run`'s child kill. That walk depends on the AppHost being visible in /proc + // as a descendant of the `dotnet` process at the moment of the walk, and on the + // AppHost being reaped by its parent rather than orphaned. When either of those races + // misfires the AppHost is left running (or lingering as a zombie reparented to PID 1) + // and the StopCommand monitor then times out reporting "Failed to stop apphost". + // Targeting the AppHost PID directly avoids the cascade entirely. + logger.LogDebug("Sending graceful shutdown to AppHost PID {Pid}", appHostInfo.ProcessId); + return await RequestProcessTreeGracefulShutdownAsync(appHostInfo.ProcessId, appHostInfo.StartedAt, includeStartTimeForDcp: false, cancellationToken).ConfigureAwait(false); + } + + // On Windows DCP is an in-tree descendant of the AppHost, so we cannot tree-kill the + // AppHost without also taking DCP down. Instead, run the graceful shutdown against the + // launcher CLI's process tree (DCP performs `stop-process-tree --skip-descendants`), + // which signals the AppHost via DCP without disrupting the descendant cleanup DCP is + // responsible for. if (appHostInfo.CliProcessId is int cliPid) { logger.LogDebug("Requesting AppHost process tree shutdown via root CLI PID {Pid}", cliPid); diff --git a/src/Shared/ProcessSignaler.cs b/src/Shared/ProcessSignaler.cs index 8a8a3ad9919..0e3e7cb7ce6 100644 --- a/src/Shared/ProcessSignaler.cs +++ b/src/Shared/ProcessSignaler.cs @@ -30,15 +30,15 @@ public static void RequestGracefulShutdown(int pid, DateTimeOffset? expectedStar } } - public static void ForceKill(int pid, DateTimeOffset? expectedStartTime, ILogger logger) + public static void ForceKill(int pid, DateTimeOffset? expectedStartTime, ILogger logger, bool killEntireProcessTree = false) { using var process = TryGetRunningProcess(pid, expectedStartTime, logger); if (process is { }) { - logger.LogDebug("Killing process {Pid}...", pid); + logger.LogDebug("Killing process {Pid} (entireProcessTree={EntireProcessTree})...", pid, killEntireProcessTree); try { - process.Kill(entireProcessTree: false); + process.Kill(entireProcessTree: killEntireProcessTree); } catch (InvalidOperationException) { diff --git a/tests/Aspire.Cli.Tests/Processes/ProcessShutdownServiceTests.cs b/tests/Aspire.Cli.Tests/Processes/ProcessShutdownServiceTests.cs index da8d43e5310..bb0879e0d1d 100644 --- a/tests/Aspire.Cli.Tests/Processes/ProcessShutdownServiceTests.cs +++ b/tests/Aspire.Cli.Tests/Processes/ProcessShutdownServiceTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Globalization; +using Aspire.Cli.Backchannel; using Aspire.Cli.Bundles; using Aspire.Cli.Layout; using Aspire.Cli.Processes; @@ -10,6 +11,7 @@ using Aspire.Cli.Tests.Utils; using Aspire.Shared; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Time.Testing; namespace Aspire.Cli.Tests.Processes; @@ -102,11 +104,51 @@ public async Task TryStopProcessTreeWithDcpAsync_UsesLeasedBundleDcpPathWhenAvai Assert.Equal(leasedDcpPath, executionFactory.LastFileName); } + [Fact] + public async Task StopAppHostAsync_CleansUpCliProcessWithoutWaitingForItAsSuccessCondition() + { + Assert.SkipWhen(OperatingSystem.IsWindows(), "The signal-ignoring shell process is Unix-specific."); + + using var workspace = TemporaryWorkspace.Create(outputHelper); + var dcpDirectory = workspace.WorkspaceRoot.CreateSubdirectory("dcp"); + File.WriteAllText(BundleDiscovery.GetDcpExecutablePath(dcpDirectory.FullName), string.Empty); + + using var cliProcess = StartSignalIgnoringShellProcess(); + try + { + var signaler = CreateService( + workspace, + dcpDirectory.FullName, + new TestProcessExecutionFactory(), + timeProvider: new FakeTimeProvider()); + + var result = await signaler.StopAppHostAsync( + new AppHostInformation + { + AppHostPath = Path.Combine(workspace.WorkspaceRoot.FullName, "apphost.cs"), + ProcessId = int.MaxValue, + StartedAt = null, + CliProcessId = cliProcess.Id, + CliStartedAt = new DateTimeOffset(cliProcess.StartTime) + }, + requestRpcStopAsync: null, + CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(2)); + + Assert.True(result); + await cliProcess.WaitForExitAsync().WaitAsync(TimeSpan.FromSeconds(2)); + } + finally + { + await StopProcessAsync(cliProcess); + } + } + private static ProcessShutdownService CreateService( TemporaryWorkspace workspace, string dcpDirectory, TestProcessExecutionFactory executionFactory, - IBundleService? bundleService = null) + IBundleService? bundleService = null, + TimeProvider? timeProvider = null) { var executionContext = new CliExecutionContext( workspace.WorkspaceRoot, @@ -122,7 +164,41 @@ private static ProcessShutdownService CreateService( new LayoutProcessRunner(executionFactory), executionContext, NullLogger.Instance, - TimeProvider.System); + timeProvider ?? TimeProvider.System); + } + + private static Process StartSignalIgnoringShellProcess() + { + var startInfo = new ProcessStartInfo("/bin/sh") + { + RedirectStandardError = true, + RedirectStandardOutput = true, + UseShellExecute = false + }; + startInfo.ArgumentList.Add("-c"); + startInfo.ArgumentList.Add("trap '' TERM; exec sleep 60"); + + var process = Process.Start(startInfo); + Assert.NotNull(process); + return process; + } + + private static async Task StopProcessAsync(Process process) + { + if (process.HasExited) + { + return; + } + + try + { + process.Kill(entireProcessTree: true); + await process.WaitForExitAsync().WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (InvalidOperationException) + { + // The process exited between the HasExited check and Kill/WaitForExitAsync. + } } private sealed class FixedLayoutDiscovery(string dcpDirectory) : ILayoutDiscovery