-
Notifications
You must be signed in to change notification settings - Fork 659
Enhance orphan detection in Aspire AppHost to be robust against PID reuse #10673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
…and remove fallback logic Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
…dback Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances Aspire's orphan detection mechanism to be robust against PID reuse by adding process start time verification alongside existing PID checks. The enhancement prevents scenarios where the OS reuses a PID for a different process, causing the AppHost to incorrectly continue running when it should shut down.
Key Changes:
- Added
ASPIRE_CLI_STARTED
environment variable to store CLI process start time - Enhanced
CliOrphanDetector
to verify both PID existence and start time match - Maintained backwards compatibility with graceful fallback to PID-only logic
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/Shared/KnownConfigNames.cs |
Added constant for new ASPIRE_CLI_STARTED environment variable |
src/Aspire.Cli/KnownFeatures.cs |
Added feature flag for orphan detection with timestamp |
src/Aspire.Cli/DotNet/DotNetCliRunner.cs |
Enhanced to set both PID and start time environment variables with feature flag |
src/Aspire.Hosting/Cli/CliOrphanDetector.cs |
Added robust process verification using both PID and start time |
tests/Aspire.Cli.Tests/Hosting/CliOrphanDetectorTests.cs |
Added comprehensive test coverage for new functionality |
tests/Aspire.Cli.Tests/DotNet/DotNetCliRunnerTests.cs |
Updated constructor calls to include new IFeatures dependency |
tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs |
Updated DotNetCliRunner factory to include IFeatures dependency |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Problem
The current AppHost orphan detection mechanism only uses the
ASPIRE_CLI_PID
environment variable to check if the launching Aspire CLI process is still alive. This approach is vulnerable to PID reuse on some systems, where the OS may assign the same PID to a different process after the original CLI process dies. This can cause the AppHost to incorrectly continue running when it should have shut down.Solution
This PR enhances the orphan detection to be robust against PID reuse by:
ASPIRE_CLI_STARTED
environment variable that stores the CLI process start time in binary formatCliOrphanDetector
now verifies both PID existence and start time match (with ±1 second tolerance) to conclusively determine if the monitored process is the same original CLI processChanges Made
Core Implementation
KnownConfigNames.cs
: AddedCliProcessStarted
constant for the newASPIRE_CLI_STARTED
environment variableDotNetCliRunner.cs
: Enhanced to set bothASPIRE_CLI_PID
andASPIRE_CLI_STARTED
environment variables when launching AppHost processesCliOrphanDetector.cs
: Added robustIsProcessRunningWithStartTime
function and updated detection logic to use both PID and start time verificationTesting
Technical Details
DateTime.ToBinary()
for reliable cross-platform start time serializationExample Scenario
Before: If CLI process (PID 1234) dies and OS reuses PID 1234 for a different process, AppHost incorrectly continues running.
After: AppHost verifies both PID 1234 exists AND has the expected start time. If PID 1234 exists but has a different start time (indicating PID reuse), AppHost correctly shuts down.
Testing
All changes have been thoroughly tested with:
This enhancement significantly improves the reliability of Aspire's orphan detection while maintaining full backwards compatibility.
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.