Fix guest AppHost Ctrl+C shutdown#17642
Closed
danegsta wants to merge 12 commits into
Closed
Conversation
Launch guest AppHost processes and AppHost servers in Windows process groups so aspire run can request graceful shutdown with CTRL_BREAK_EVENT before falling back to force termination. Route guest and server cancellation through ProcessShutdownService so guest fallbacks can kill the full tree while server fallbacks avoid killing in-tree DCP on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17642Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17642" |
Treat an AppHost server that exits before shutdown coordination can read its start time as already stopped. This preserves process identity validation and avoids signaling a reused PID without a start-time guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve ProcessGuestLauncher changes from main by preserving the new lifecycle logging while retaining graceful process-group shutdown before force-kill fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Ctrl+C shutdown race for guest AppHosts in aspire run by ensuring cancellation follows a graceful-then-force termination flow (instead of immediately force-killing), and by adding a Windows process-group shutdown path that uses CTRL_BREAK_EVENT. It also adjusts Windows fallback behavior for AppHost servers to avoid tearing down in-tree DCP during server cleanup.
Changes:
- Launch guest AppHosts and AppHost server processes in Windows process groups, enabling targeted
CTRL_BREAK_EVENTshutdown. - Add process-group graceful shutdown support to
ProcessShutdownService/ProcessSignaler, and route guest/server cancellation through the coordinated graceful-then-force flow. - Add/adjust tests to validate Unix SIGTERM graceful behavior and Windows CTRL_BREAK_EVENT behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Projects/ProcessGuestLauncherTests.cs | Adds coverage for cancellation-driven graceful shutdown on Unix and Windows (process-group Ctrl+Break). |
| tests/Aspire.Cli.Tests/Projects/GuestAppHostProjectTests.cs | Wires a ProcessShutdownService into test project construction to match updated runtime requirements. |
| src/Shared/ProcessSignaler.cs | Adds a process-group graceful shutdown API and Windows GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT) support. |
| src/Aspire.Cli/Projects/ProcessGuestLauncher.cs | Starts guest processes in a Windows process group and uses coordinated graceful-then-force shutdown on cancellation. |
| src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs | Starts the AppHost server process in a Windows process group. |
| src/Aspire.Cli/Projects/GuestRuntime.cs | Plumbs an optional ProcessShutdownService through to the default guest launcher. |
| src/Aspire.Cli/Projects/GuestAppHostProject.cs | Requires and threads ProcessShutdownService through guest runtime and server session startup paths. |
| src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs | Starts the dotnet-based AppHost server in a Windows process group. |
| src/Aspire.Cli/Projects/AppHostServerSession.cs | Adds optional coordinated shutdown for the server process; adjusts Windows fallback kill semantics. |
| src/Aspire.Cli/Processes/ProcessShutdownService.cs | Introduces process-group shutdown flow and makes force-kill scope (tree vs single process) configurable per call. |
Member
Author
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR CLI install and version verificationObjective: Install the PR dogfood CLI and verify it matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: Template smoke testObjective: Verify the PR CLI can acquire templates from the PR hive and create a fresh project non-interactively. Steps:
Evidence:
Observations:
Scenario 3: TypeScript guest AppHost shutdown on macOSObjective: Exercise the changed guest AppHost run/shutdown path with a fresh TypeScript AppHost and verify SIGINT-triggered shutdown exits cleanly. Steps:
Evidence:
Observations:
Scenario 4: Missing AppHost pathObjective: Verify a bad Steps:
Evidence:
Expected Unhappy-Path Outcome: A non-zero exit code with a clear error that the AppHost project does not exist. Observations:
Summary
Overall ResultPR VERIFIED Recommendations
|
Treat a guest AppHost process that exits before shutdown coordination can read its start time as already stopped. This preserves process identity validation and avoids signaling a reused PID without a start-time guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
reviewed
May 29, 2026
Member
There was a problem hiding this comment.
I think StopProcessGroupAsync, and this, plus WaitForDrainAsync needs a timeout.
Basically, after the CT is raised, there should be 3 seconds to do clean up before an OCE is thrown out of the method.
Add a ProcessStartInfo extension for Windows process-group creation and use it for guest AppHost and AppHost server launches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Derive the AppHost startup cancellation wait from ProcessShutdownService so aspire run allows the graceful shutdown monitor and force-kill verification windows to complete instead of timing out after five seconds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
reviewed
May 29, 2026
mitchdenny
left a comment
mitchdenny
left a comment
Member
There was a problem hiding this comment.
One inline comment on the Windows CTRL_BREAK test.
Make ProcessShutdownService termination monitoring configurable and use a shorter timeout for aspire run owned guest/server processes while keeping the longer default for detached/start/stop paths. Relax guest launcher test startup waits for slower CI process startup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Include the ProcessSignaler diagnostic update for process-group shutdown signaling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use fresh timeout cancellation tokens for aspire run guest and server shutdown coordination instead of passing CancellationToken.None after command cancellation. Keep shutdown bounded while avoiding the already-canceled command token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Let RunCommand own the shorter run shutdown budget and pass it down explicitly to guest AppHost and AppHost server shutdown coordination. Use bounded timeout tokens instead of CancellationToken.None and add AppHost server shutdown logging before the shared shutdown service takes over. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Do not gate Windows process-group CTRL_BREAK_EVENT delivery on process start-time validation. Aspire owns these child process groups directly, and skipping the event after a failed PID lookup makes the shutdown service wait as though a graceful signal was sent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover TryGetRunningProcess with a real child process start-time round trip and a mismatched start-time rejection, matching how guest AppHost shutdown captures process identity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26616580186 |
Member
|
Test failure looks relevant. |
radical
added
the
needs-author-action
JamesNK
reviewed
Jun 3, 2026
Comment on lines
+22
to
24
| TimeSpan? processTerminationTimeout = null, | ||
| TimeSpan? processShutdownTimeout = null, | ||
| Func<Task>? afterLaunchAsync = null); |
Member
There was a problem hiding this comment.
CancellationToken should be the last parameter.
Member
|
My CTRL+C PR merged. Rebase/merge latest main. |
Member
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Guest AppHost Ctrl+C shutdownObjective: Verify a foreground TypeScript guest AppHost exits promptly when interrupted with Ctrl+C and leaves no related processes behind. Steps:
Evidence:
Observations:
Scenario 2: Guest AppHost start/stop shutdownObjective: Verify detached TypeScript guest AppHost lifecycle works with Steps:
Evidence:
Observations:
Scenario 3: .NET AppHost smoke regressionObjective: Verify normal C#/.NET AppHost execution still works after the CLI process-group changes. Steps:
Evidence:
Observations:
Scenario 4: Invalid guest AppHost startup cleanupObjective: Verify a guest AppHost startup failure is reported clearly and the temporary server/guest processes are cleaned up. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Non-zero exit code with the intentional startup error; no lingering related processes. Scenario 5: Shutdown-resisting guest force cleanupObjective: Verify a guest AppHost that ignores shutdown signals is still force-cleaned within the shutdown budget. Steps:
Evidence:
Observations:
Expected Unhappy-Path Outcome: Ctrl+C returns to the shell and no related guest processes remain even though the guest ignored graceful signals. Summary
Overall ResultPR VERIFIED Recommendations
Artifact Notes
|
IEvangelist
reviewed
Jun 5, 2026
|
|
||
| if (OperatingSystem.IsWindows()) | ||
| { | ||
| RequestGracefulShutdownWindowsProcessGroup(pid, logger); |
Member
There was a problem hiding this comment.
This Windows path bypasses the stale-PID protection that the Unix branch and ForceKill get from TryGetRunningProcess(pid, expectedStartTime, logger). If the target exits and the PID/process-group id is reused, we can send Ctrl+Break to an unrelated process group. Please validate the PID/start-time pair before the OS-specific branch so Windows also returns without signaling when the process is gone or the start time no longer matches, and add a regression test for the mismatched-start-time process-group case.
IEvangelist
reviewed
Jun 5, 2026
Member
There was a problem hiding this comment.
I do feel like the complexity of this service is getting more and more involved.
Member
Author
|
#17814 effectively supersedes this work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
aspire runcould race graceful shutdown for guest AppHosts: cancellation stopped the CLI wait immediately and the guest launcher could force-kill the guest process tree before the process handled Ctrl+C/Ctrl+Break. On Windows, the AppHost server also needs different fallback behavior from the guest because DCP is in the server process tree and should not be killed by server fallback cleanup.This change launches guest AppHost processes and AppHost servers in Windows process groups, adds a
ProcessShutdownServiceprocess-group shutdown path that sendsCTRL_BREAK_EVENT, and routes guest/server cancellation through the shared graceful-then-force shutdown flow. Guest fallback still kills the whole process tree. Server fallback kills the whole tree on Unix, but only the target server process on Windows so in-tree DCP can keep handling cleanup.User-facing behavior
Pressing Ctrl+C during
aspire runfor guest AppHosts now gives the guest and AppHost server a graceful shutdown window before force termination, aligning the run path with the existing shutdown coordinator behavior.Validation:
dotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ProcessGuestLauncherTests" --filter-class "*.ProcessShutdownServiceTests" --filter-class "*.AppHostServerSessionTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?