Skip to content

Commit 9f9ccbb

Browse files
authored
Fix issue 15806 ("aspire stop" is not cleaning up application containers) (#16006)
* Fix issue 15806 * Change default process start time check tolerance to 1 second (to match the start time granularity used by the backchannel) * Address code review feedback from Mitch * Add ExecuteCommandUntilOutputAsync test helper * Ensure containers are cleaned up by "aspire stop" command * Improved check for container cleanup
1 parent ca405f9 commit 9f9ccbb

File tree

11 files changed

+372
-69
lines changed

11 files changed

+372
-69
lines changed

src/Aspire.Cli/Aspire.Cli.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
<Compile Include="$(SharedDir)ConsoleLogs\SharedAIHelpers.cs" Link="ConsoleLogs\SharedAIHelpers.cs" />
130130
<Compile Include="$(SharedDir)ConsoleLogs\TimestampParser.cs" Link="ConsoleLogs\TimestampParser.cs" />
131131
<Compile Include="$(SharedDir)ConsoleLogs\UrlParser.cs" Link="ConsoleLogs\UrlParser.cs" />
132+
<Compile Include="$(SharedDir)ProcessSignaler.cs" Link="Utils\ProcessSignaler.cs" />
132133
<Compile Include="$(SharedDir)NpmVersionHelper.cs" Link="Utils\NpmVersionHelper.cs" />
133134
</ItemGroup>
134135

src/Aspire.Cli/Commands/StopCommand.cs

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.CommandLine;
5-
using System.Diagnostics;
65
using System.Globalization;
76
using Aspire.Cli.Backchannel;
87
using Aspire.Cli.Configuration;
@@ -193,16 +192,12 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
193192

194193
_interactionService.DisplayMessage(KnownEmojis.StopSign, "Sending stop signal...");
195194

196-
// Get the CLI process ID - this is the process we need to kill
197-
// Killing the CLI process will tear down everything including the AppHost
198-
var cliProcessId = appHostInfo?.CliProcessId;
199-
200-
if (cliProcessId is int cliPid)
195+
if (appHostInfo?.CliProcessId is int cliPid)
201196
{
202197
_logger.LogDebug("Sending stop signal to CLI process (PID {Pid})", cliPid);
203198
try
204199
{
205-
SendStopSignal(cliPid);
200+
SendStopSignal(cliPid, appHostInfo?.CliStartedAt);
206201
}
207202
catch (Exception ex)
208203
{
@@ -213,7 +208,7 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
213208
}
214209
else
215210
{
216-
// Fallback: Try the RPC method if we don't have CLI process ID
211+
// Fallback: Try the RPC method if we don't have CLI process ID.
217212
_logger.LogDebug("No CLI process ID available, trying RPC stop");
218213
var rpcSucceeded = false;
219214
try
@@ -228,10 +223,10 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
228223
// If RPC didn't work, try sending SIGINT to AppHost process directly
229224
if (!rpcSucceeded && appHostInfo?.ProcessId is int appHostPid)
230225
{
231-
_logger.LogDebug("RPC stop not available, sending SIGINT to AppHost PID {Pid}", appHostPid);
226+
_logger.LogDebug("RPC stop not available, sending SIGTERM to AppHost PID {Pid}", appHostPid);
232227
try
233228
{
234-
SendStopSignal(appHostPid);
229+
SendStopSignal(appHostPid, appHostInfo?.StartedAt);
235230
}
236231
catch (Exception ex)
237232
{
@@ -247,21 +242,37 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
247242
}
248243
}
249244

245+
var manager = new RunningInstanceManager(_logger, _interactionService, _timeProvider);
250246
var stopped = await _interactionService.ShowStatusAsync(
251247
StopCommandStrings.StoppingAppHost,
252248
async () =>
253249
{
254250
try
255251
{
256-
// Wait for processes to terminate
257-
var manager = new RunningInstanceManager(_logger, _interactionService, _timeProvider);
252+
if (appHostInfo is null)
253+
{
254+
return true;
255+
}
256+
257+
if (await manager.MonitorProcessesForTerminationAsync(appHostInfo, cancellationToken).ConfigureAwait(false))
258+
{
259+
return true;
260+
}
258261

259-
if (appHostInfo is not null)
262+
var procsToKill = new HashSet<(int, DateTimeOffset?)> { (appHostInfo.ProcessId, appHostInfo.StartedAt) };
263+
264+
if (appHostInfo.CliProcessId is int cliPid)
265+
{
266+
procsToKill.Add((cliPid, appHostInfo.CliStartedAt));
267+
}
268+
269+
foreach (var (pid, startTime) in procsToKill)
260270
{
261-
return await manager.MonitorProcessesForTerminationAsync(appHostInfo, cancellationToken).ConfigureAwait(false);
271+
_logger.LogWarning("AppHost did not stop gracefully within timeout. Forcing process {Pid} to terminate.", pid);
272+
ForceKillProcess(pid, startTime);
262273
}
263274

264-
return true;
275+
return await manager.MonitorProcessesForTerminationAsync(appHostInfo, cancellationToken).ConfigureAwait(false);
265276
}
266277
catch (Exception ex)
267278
{
@@ -286,28 +297,21 @@ private async Task<int> StopAppHostAsync(IAppHostAuxiliaryBackchannel connection
286297
}
287298

288299
/// <summary>
289-
/// Sends a stop signal to a process to terminate it and its process tree.
290-
/// Uses Process.Kill(entireProcessTree: true) to ensure all child processes are terminated.
300+
/// Sends a best-effort graceful shutdown signal to the target process.
301+
/// Uses Ctrl-Break on Windows and SIGTERM on non-Windows.
291302
/// </summary>
292-
private static void SendStopSignal(int pid)
303+
private void SendStopSignal(int pid, DateTimeOffset? startTime)
293304
{
294-
try
295-
{
296-
using var process = Process.GetProcessById(pid);
297-
process.Kill(entireProcessTree: true);
298-
}
299-
catch (ArgumentException)
300-
{
301-
// Process doesn't exist - already terminated
302-
}
303-
catch (InvalidOperationException)
304-
{
305-
// Process has already exited
306-
}
307-
catch (Exception)
308-
{
309-
// Some other error (e.g., permission denied) - ignore
310-
}
305+
ProcessSignaler.RequestGracefulShutdown(pid, startTime, _logger);
306+
}
307+
308+
/// <summary>
309+
/// Forcefully kills the target process after the graceful shutdown timeout elapses.
310+
/// This does not terminate the entire process tree.
311+
/// </summary>
312+
private void ForceKillProcess(int pid, DateTimeOffset? startTime)
313+
{
314+
ProcessSignaler.ForceKill(pid, startTime, _logger);
311315
}
312316

313317
}

src/Aspire.Cli/Processes/DetachedProcessLauncher.Windows.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,17 @@ private static Process StartWindows(string fileName, IReadOnlyList<string> argum
7777

7878
var si = new STARTUPINFOEX();
7979
si.cb = Marshal.SizeOf<STARTUPINFOEX>();
80-
si.dwFlags = StartfUseStdHandles;
80+
si.dwFlags = StartfUseStdHandles | StartfUseShowWindow;
8181
si.hStdInput = nint.Zero;
8282
si.hStdOutput = nulRawHandle;
8383
si.hStdError = nulRawHandle;
8484
si.lpAttributeList = attrList;
85+
si.wShowWindow = ShowWindowHide;
8586

8687
// Build the command line string: "fileName" arg1 arg2 ...
8788
var commandLine = BuildCommandLine(fileName, arguments);
8889

89-
var flags = CreateUnicodeEnvironment | ExtendedStartupInfoPresent | CreateNoWindow;
90+
var flags = CreateUnicodeEnvironment | ExtendedStartupInfoPresent | CreateNewProcessGroup;
9091

9192
// Build a filtered environment block if variables need to be removed.
9293
// CreateProcessW with lpEnvironment=nint.Zero inherits the parent's
@@ -293,9 +294,11 @@ private static nint BuildFilteredEnvironmentBlock(Func<string, bool> shouldRemov
293294
private const uint OpenExisting = 3;
294295
private const uint HandleFlagInherit = 0x00000001;
295296
private const uint StartfUseStdHandles = 0x00000100;
297+
private const uint StartfUseShowWindow = 0x00000001;
296298
private const uint CreateUnicodeEnvironment = 0x00000400;
297299
private const uint ExtendedStartupInfoPresent = 0x00080000;
298-
private const uint CreateNoWindow = 0x08000000;
300+
private const uint CreateNewProcessGroup = 0x00000200;
301+
private const ushort ShowWindowHide = 0x0000;
299302
private static readonly nint s_procThreadAttributeHandleList = (nint)0x00020002;
300303

301304
// --- Structs ---

src/Aspire.Cli/Program.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.CommandLine.Parsing;
66
using System.Diagnostics;
77
using System.Globalization;
8+
using System.Runtime.InteropServices;
89
using System.Text;
910
using System.Text.Json;
1011
using Aspire.Cli.Agents;
@@ -680,6 +681,13 @@ public static async Task<int> Main(string[] args)
680681
cts.Cancel();
681682
eventArgs.Cancel = true;
682683
};
684+
using var sigTermRegistration = OperatingSystem.IsWindows()
685+
? null
686+
: PosixSignalRegistration.Create(PosixSignal.SIGTERM, context =>
687+
{
688+
cts.Cancel();
689+
context.Cancel = true;
690+
});
683691

684692
Console.OutputEncoding = Encoding.UTF8;
685693

@@ -695,6 +703,7 @@ public static async Task<int> Main(string[] args)
695703
// Logging the log file path is useful so that when console logging is enabled (for example with --log-level debug),
696704
// the path is written to the console logger (stderr) for easier discovery.
697705
logger.LogInformation("Log file: {LogFilePath}", loggingOptions.LogFilePath);
706+
logger.LogInformation("CLI process ID: {ProcessId}", Environment.ProcessId);
698707

699708
IHost? app = null;
700709
try

src/Aspire.Hosting/Aspire.Hosting.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<Compile Include="$(SharedDir)UserSecrets\UserSecretsPathHelper.cs" Link="UserSecrets\UserSecretsPathHelper.cs" />
5757
<Compile Include="$(SharedDir)X509Certificate2Extensions.cs" Link="Utils\X509Certificate2Extensions.cs" />
5858
<Compile Include="$(SharedDir)PasswordGenerator.cs" Link="Utils\PasswordGenerator.cs" />
59+
<Compile Include="$(SharedDir)ProcessSignaler.cs" Link="Utils\ProcessSignaler.cs" />
5960
</ItemGroup>
6061

6162
<ItemGroup>

src/Aspire.Hosting/Backchannel/AuxiliaryBackchannelRpcTarget.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,13 +366,20 @@ public Task<AppHostInformation> GetAppHostInformationAsync(CancellationToken can
366366
{
367367
cliProcessId = parsedCliPid;
368368
}
369+
DateTimeOffset? cliStartedAt = null;
370+
var cliStartedAtString = configuration[KnownConfigNames.CliProcessStarted];
371+
if (!string.IsNullOrEmpty(cliStartedAtString) && long.TryParse(cliStartedAtString, out var parsedCliStartedAt))
372+
{
373+
cliStartedAt = DateTimeOffset.FromUnixTimeSeconds(parsedCliStartedAt);
374+
}
369375

370376
return Task.FromResult(new AppHostInformation
371377
{
372378
AppHostPath = appHostPath,
373379
ProcessId = Environment.ProcessId,
374380
CliProcessId = cliProcessId,
375-
StartedAt = new DateTimeOffset(Process.GetCurrentProcess().StartTime)
381+
StartedAt = new DateTimeOffset(Process.GetCurrentProcess().StartTime),
382+
CliStartedAt = cliStartedAt
376383
});
377384
}
378385

src/Aspire.Hosting/Backchannel/BackchannelDataTypes.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,12 @@ internal sealed class AppHostInformation
10201020
/// Gets or sets when the AppHost process started.
10211021
/// </summary>
10221022
public DateTimeOffset? StartedAt { get; init; }
1023+
1024+
/// <summary>
1025+
/// Gets or sets when the CLI process that launched the AppHost started.
1026+
/// This value is only set when the AppHost is launched via the Aspire CLI.
1027+
/// </summary>
1028+
public DateTimeOffset? CliStartedAt { get; init; }
10231029
}
10241030

10251031
/// <summary>

src/Aspire.Hosting/Cli/CliOrphanDetector.cs

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Diagnostics;
54
using Microsoft.Extensions.Configuration;
65
using Microsoft.Extensions.Hosting;
76
using Microsoft.Extensions.Logging;
@@ -12,37 +11,14 @@ internal sealed class CliOrphanDetector(IConfiguration configuration, IHostAppli
1211
{
1312
internal Func<int, bool> IsProcessRunning { get; set; } = (int pid) =>
1413
{
15-
try
16-
{
17-
return !Process.GetProcessById(pid).HasExited;
18-
}
19-
catch (ArgumentException)
20-
{
21-
// If Process.GetProcessById throws it means the process in not running.
22-
return false;
23-
}
14+
using var process = ProcessSignaler.TryGetRunningProcess(pid, null, logger);
15+
return process is not null;
2416
};
2517

2618
internal Func<int, long, bool> IsProcessRunningWithStartTime { get; set; } = (int pid, long expectedStartTimeUnix) =>
2719
{
28-
try
29-
{
30-
var process = Process.GetProcessById(pid);
31-
if (process.HasExited)
32-
{
33-
return false;
34-
}
35-
36-
// Check if the process start time matches the expected start time exactly.
37-
var actualStartTimeUnix = ((DateTimeOffset)process.StartTime).ToUnixTimeSeconds();
38-
return actualStartTimeUnix == expectedStartTimeUnix;
39-
}
40-
catch
41-
{
42-
// If we can't get the process and/or can't get the start time,
43-
// then we interpret both exceptions as the process not being there.
44-
return false;
45-
}
20+
using var process = ProcessSignaler.TryGetRunningProcess(pid, DateTimeOffset.FromUnixTimeSeconds(expectedStartTimeUnix), logger);
21+
return process is not null;
4622
};
4723

4824
protected override async Task ExecuteAsync(CancellationToken stoppingToken)

0 commit comments

Comments
 (0)