Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 25, 2025

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:

  1. Adding process start time verification: Introduces a new ASPIRE_CLI_STARTED environment variable that stores the CLI process start time in binary format
  2. Robust process checking: The CliOrphanDetector 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 process
  3. Backwards compatibility: Gracefully falls back to PID-only logic when the start time environment variable is not present or invalid

Changes Made

Core Implementation

  • KnownConfigNames.cs: Added CliProcessStarted constant for the new ASPIRE_CLI_STARTED environment variable
  • DotNetCliRunner.cs: Enhanced to set both ASPIRE_CLI_PID and ASPIRE_CLI_STARTED environment variables when launching AppHost processes
  • CliOrphanDetector.cs: Added robust IsProcessRunningWithStartTime function and updated detection logic to use both PID and start time verification

Testing

  • Added comprehensive test coverage for new scenarios including:
    • Robust detection with start time verification
    • Backwards compatibility fallback
    • PID reuse detection
    • Invalid start time handling
    • Process lifecycle monitoring with enhanced detection

Technical Details

  • Uses DateTime.ToBinary() for reliable cross-platform start time serialization
  • 1-second tolerance accommodates timing differences in process start detection
  • Maintains full backwards compatibility with older CLI versions
  • Minimal code changes with surgical precision

Example 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:

  • ✅ Successful compilation of all affected projects
  • ✅ Manual verification of core logic (process start time verification, binary serialization)
  • ✅ Comprehensive unit test coverage for new functionality
  • ✅ Backwards compatibility validation

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.

Enhance orphan detection in Aspire AppHost to be robust against PID reuse. Currently, the AppHost only uses the ASPIRE_CLI_PID environment variable to detect if the launching Aspire CLI process is still alive, by checking for the PID. This is vulnerable to PID reuse on some systems. The solution is to set an additional environment variable, ASPIRE_CLI_STARTED, which stores the CLI process start time. The AppHost orphan detector (CliOrphanDetector) should check both the PID and the process start time (if present) to conclusively determine if the parent process is the same. If ASPIRE_CLI_STARTED is not present, fallback to PID-only logic for backwards compatibility. Also update relevant tests.


💡 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.

Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Robustify AppHost orphan detection against PID reuse with ASPIRE_CLI_STARTED Enhance orphan detection in Aspire AppHost to be robust against PID reuse Jul 25, 2025
@Copilot Copilot AI requested a review from mitchdenny July 25, 2025 13:25
Copilot finished work on behalf of mitchdenny July 25, 2025 13:25
…and remove fallback logic

Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
@Copilot Copilot AI requested a review from mitchdenny July 25, 2025 13:56
Copilot finished work on behalf of mitchdenny July 25, 2025 13:56
…dback

Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
Copilot finished work on behalf of mitchdenny July 25, 2025 14:07
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
@Copilot Copilot AI requested a review from mitchdenny July 25, 2025 14:26
Copilot finished work on behalf of mitchdenny July 25, 2025 14:26
Co-authored-by: mitchdenny <513398+mitchdenny@users.noreply.github.com>
@Copilot Copilot AI requested a review from mitchdenny July 26, 2025 03:04
Copilot finished work on behalf of mitchdenny July 26, 2025 03:04
@mitchdenny mitchdenny marked this pull request as ready for review July 26, 2025 03:34
@Copilot Copilot AI review requested due to automatic review settings July 26, 2025 03:34
@mitchdenny mitchdenny added this to the 9.5 milestone Jul 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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

mitchdenny and others added 2 commits July 26, 2025 13:47
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mitchdenny mitchdenny added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-cli NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants