Fixes several use cases where aspire-managed processes may leak - #18566
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18566Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18566" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR addresses leaked aspire-managed (and AppHost) processes — most visible on non-Windows platforms — that accumulate when a launching/parent process dies abnormally during startup or PID reuse defeats the existing PID-only orphan detection (issue #18050). It introduces a shared, dependency-free ProcessStartTimeHelper (PID + start-time identity, surviving PID reuse) and a reusable ParentProcessLivenessMonitor, then wires several watchdogs across the CLI, RemoteHost, and the aspire-managed NuGet helper, plus an on-demand OrphanedAppHostCollector invoked by aspire ps / aspire stop --all.
Changes:
- New shared primitives:
ProcessStartTimeHelper(PID + start-time liveness checks) andParentProcessLivenessMonitor(poll loop with idempotent teardown), withProcessSignaler.AreCloserefactored to reuse the helper. - RemoteHost
OrphanDetectornow verifies parent PID and start time (viaIConfiguration), falling back to PID-only for older CLIs;aspire-managed nugetgains aParentProcessWatchdog; detachedaspire start/run --detachchildren watch their launcher during startup viaLauncherLivenessMonitorand disarm at readiness. - New
OrphanedAppHostCollectorreaps AppHost trees whose launching CLI is gone (used bypsandstop --all); newASPIRE_LAUNCHER_*/REMOTE_APP_HOST_STARTEDconfig names and start-time propagation in the AppHost server spawn paths.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/ProcessStartTimeHelper.cs | New dependency-free PID+start-time liveness/parse helpers with 1s tolerance. |
| src/Shared/ParentProcessLivenessMonitor.cs | New shared poll-based monitor invoking a callback once when the parent is gone; idempotent async disposal. |
| src/Shared/ProcessSignaler.cs | AreClose now delegates the truncate-and-compare math to the shared helper (behavior preserved). |
| src/Shared/KnownConfigNames.cs | Adds REMOTE_APP_HOST_STARTED, ASPIRE_LAUNCHER_PID/STARTED constants. |
| src/Aspire.Hosting.RemoteHost/OrphanDetector.cs | Reads config instead of env directly; adds start-time verification with PID-only fallback. |
| src/Aspire.Managed/Program.cs · NuGet/ParentProcessWatchdog.cs | NuGet helper self-terminates if the launching CLI dies (cancel then force-exit 124). |
| src/Aspire.Cli/Processes/LauncherLivenessMonitor.cs | Watches the foreground launcher of a detached child during startup. |
| src/Aspire.Cli/Commands/RunCommand.cs | Starts the launcher monitor for detached children; disarms at readiness; disposes on all exit paths. |
| src/Aspire.Cli/Commands/AppHostLauncher.cs | Stamps launcher PID + start time onto the detached child environment. |
| src/Aspire.Cli/Layout/LayoutProcessRunner.cs | Stamps CLI identity onto layout tool env without mutating the caller's dictionary. |
| src/Aspire.Cli/Backchannel/OrphanedAppHostCollector.cs | New collector that stops AppHosts whose launching CLI is dead and removes their sockets. |
| src/Aspire.Cli/Commands/PsCommand.cs · StopCommand.cs · Program.cs | Inject and invoke the collector; register it in DI. |
| src/Aspire.Cli/Projects/{PrebuiltAppHostServer,DotNetBasedAppHostServerProject}.cs | Use config-name constants and propagate parent start time. |
| *.csproj (Aspire.Cli, Aspire.Hosting, Aspire.Hosting.RemoteHost, Aspire.Managed) | Link the new shared files into each consuming assembly. |
| tests/** (7 new/updated test files) | Cover helper behavior, the monitors, the orphan detector start-time paths, env stamping, and IsOrphaned. |
I verified the cross-cutting concerns that this kind of change typically breaks: every project linking ProcessSignaler.cs also links the new ProcessStartTimeHelper.cs it now depends on; the RemoteHost default host builder surfaces environment variables through IConfiguration (so the env→config switch is safe); the detached launcher stays alive until the child reports readiness (so disarming at readiness is correct and a normal launcher exit won't tear down a healthy AppHost); and the watchdog/monitor disposal ordering avoids use-after-dispose and skips the hard Environment.Exit when the operation finishes cleanly. No blocking correctness issues were found, but the change spans process-killing, Environment.Exit, and concurrent teardown across multiple components, which carries meaningful regression risk.
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Posted 2 review comments: 1 incomplete process-leak coverage issue and 1 missing regression coverage issue.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Two failures were found while testing detached `aspire start` (PR #18566): - Scenario 2 (happy path): the launcher liveness monitor was disarmed only after full readiness (after GetDashboardUrlsAsync + the 2s early-exit observation window + NotifyAppHostReadyAsync). The foreground launcher exits as soon as it observes readiness over the auxiliary backchannel, so its ~1s-later detection by the monitor cancelled a healthy run. Disarm the monitor as soon as the child<->AppHost backchannel is established instead; from that point the AppHost's orphan detector anchors to this child via ASPIRE_CLI_PID, so the launcher's normal exit is irrelevant. - Scenario 3 (launcher killed before readiness): a termination signal during startup unwound to the cancellation catch without awaiting pendingRun, so the CLI exited while the dotnet run shutdown ladder was still running, orphaning an AppHost that aspire ps could not see. Hoist pendingRun and the run CTS to the outer scope and add a bounded teardown backstop in the outer finally so a detached child waits for the AppHost process tree to be killed before it returns. Adds two deterministic regression tests (verified to fail without the fixes). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 53 / 99 test projects · 5 jobs, from 71 changed files. Selected test projects (53 / 99)
Selected jobs (5)
How these were chosen — grouped by what changed
🔧 show 31
🔧 🔧 🧪 🔧 📦 affected project 📦 affected project 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedChange Categories
The 4 leak scenarios the PR fixes
Environment Notes
Test Scenarios ExecutedScenario 1: Regression — clean start / ps / stopCoverage: Happy path · Status: ✅ Passed
Scenario 2: Core leak fix — hard-kill the CLI supervisorCoverage: Core fix (leak) · Status: ✅ Passed
Scenario 3: Standalone dashboard orphan (fix #2)Coverage: New behavior · Status: ✅ Passed
Scenario 4: ps/stop accuracy + orphan reaping (fix #4)Coverage: Correctness / backstop · Status: ✅ Passed (with note)
Scenario 5: Launcher killed mid-start (fix #3,
|
| Command | Result | Exit |
|---|---|---|
aspire stop (nothing running) |
"No running AppHost found." | 0 |
aspire stop --apphost <valid, not running> |
"No AppHost is currently running for '…'. Use 'aspire run' to start it first." | 0 |
aspire stop --apphost <non-existent path> |
"❌ The --apphost option specified a project that does not exist." | 7 |
aspire ps (nothing running) |
"No running AppHost found." | 0 |
Not reproduced end-to-end (covered by unit tests)
- RemoteHost PID-reuse fix (PR scenario 1): requires a remote-apphost server context not reachable via the dogfood CLI. Covered by
OrphanDetectorTests(166 lines) verifying PID+start-time and legacy fallback. - Active reaping via
OrphanedAppHostCollector(fix Move repo to 8.0.100-rtm build #4): see Scenario 4 note.
Summary
| Scenario | Status | Notes |
|---|---|---|
| 1. Regression clean start/stop | ✅ Passed | stop in 1s, full teardown |
| 2. Hard-kill CLI supervisor | ✅ Passed | whole tree reaped in 3s, no leak |
| 3. Standalone dashboard orphan | ✅ Passed | dashboard reaped in 2s after CLI kill |
| 4. ps/stop accuracy + reaping | ✅ Passed | no false "running"; socket-residue note |
| 5. Launcher killed mid-start | ✅ Passed | detached child aborts in 2s; log-confirmed |
| 6. Unhappy paths | ✅ Passed | correct messages + exit codes |
Overall Result
✅ PR VERIFIED — All executed scenarios passed. The core process-leak fixes behave as intended on Windows: hard-killing the CLI supervisor or the dashboard CLI reaps the whole tree promptly, and killing the launcher mid-start aborts the detached child instead of leaking an AppHost.
Recommendations / minor observations (non-blocking)
- Consider a cleaner detached-child abort message instead of the raw
TaskCanceledExceptionsurface ("An unexpected error occurred: A task was canceled.") whenLauncherLivenessMonitorcancels a mid-start run. - Consider whether stale 0-byte backchannel socket files in
~/.aspire/cli/bchshould be pruned byps/stopon Windows (harmless today, but they accumulate). Verify whether this predates the PR. - Re-run the Docker/Redis container path in an environment where the container reaches
Runningto also confirm container cleanup on hard-kill (not exercised here due to a Docker warmup issue unrelated to the PR).
a89e7ad
into
main
This comment has been minimized.
This comment has been minimized.
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
|
📝 Documentation has been drafted in microsoft/aspire.dev#1439 targeting Triggered signal Note This draft PR needs human review before merging. |
Description
There are at least 4 scenarios when
aspire-managedprocesses may leak:PID-reuse.
RemoteHost/OrphanDetector.cspolls the parent by PID only (REMOTE_APP_HOST_PID), no start-time check, and the CLI's start time is never propagated. When the launching CLI dies and its PID is reused (pretty common if machine is heavily used, e.g. during test run), the server thinks its parent is alive and never exits → permanent leak. The .NET AppHost path (CliOrphanDetector) already does PID+start-time; theRemoteHostpath was never brought to that bar.dashboardandnugetcommands have no orphan detection at all. A hard-killed CLI (SIGKILL from a test runner, Ctrl-C during a slow restore) leaves them lingering forever.During
aspire start, if the launcher CLI process is killed (SIGKILL) mid-start, the detached CLI process wil just complete the startup and keep running.There is no active reaping of already-leaked orphans. once leaked they persist and get rediscovered by the auxiliary backchannel monitor, producing the "started successfully but isn't running" false success.
Fixes #18050
Checklist
<remarks />and<code />elements on your triple slash comments?