Handle process inspection race during shutdown#17676
Conversation
Treat Win32Exception while inspecting process metadata as a stale or non-targetable process so stop monitoring does not fail when an AppHost exits during shutdown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17676Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17676" |
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR CLI install and version verificationObjective: Install the dogfood CLI for PR #17676 and verify it matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: Basic stop lifecycleObjective: Verify a fresh AppHost still starts and stops cleanly through the PR CLI using an explicit AppHost path. Steps:
Evidence:
Observations:
Scenario 3: Stop all lifecycleObjective: Verify Steps:
Evidence:
Observations:
Scenario 4: No running AppHostsObjective: Verify repeating Steps:
Evidence:
Expected Unhappy-Path Outcome: The command should not crash or surface an unexpected process inspection error when there are no AppHosts left to stop. Observations:
Summary
Overall Result✅ PR VERIFIED Recommendations
|
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR CLI install and version verificationObjective: Install the dogfood CLI for PR #17676 and verify it matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: Basic stop lifecycleObjective: Verify a fresh AppHost still starts and stops cleanly through the PR CLI using an explicit AppHost path. Steps:
Evidence:
Observations:
Scenario 3: Stop all lifecycleObjective: Verify Steps:
Evidence:
Observations:
Scenario 4: No running AppHostsObjective: Verify repeating Steps:
Evidence:
Expected Unhappy-Path Outcome: The command should not crash or surface an unexpected process inspection error when there are no AppHosts left to stop. Observations:
Summary
Overall Result✅ PR VERIFIED Recommendations
|
1 similar comment
PR Testing ReportPR Information
CLI Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR CLI install and version verificationObjective: Install the dogfood CLI for PR #17676 and verify it matches the PR head commit. Steps:
Evidence:
Observations:
Scenario 2: Basic stop lifecycleObjective: Verify a fresh AppHost still starts and stops cleanly through the PR CLI using an explicit AppHost path. Steps:
Evidence:
Observations:
Scenario 3: Stop all lifecycleObjective: Verify Steps:
Evidence:
Observations:
Scenario 4: No running AppHostsObjective: Verify repeating Steps:
Evidence:
Expected Unhappy-Path Outcome: The command should not crash or surface an unexpected process inspection error when there are no AppHosts left to stop. Observations:
Summary
Overall Result✅ PR VERIFIED Recommendations
|
There was a problem hiding this comment.
Pull request overview
This PR hardens process shutdown handling by treating Win32Exception during process inspection as an already-stopped or unverifiable process, preventing aspire stop --all from failing when an AppHost exits during termination monitoring.
Changes:
- Adds
Win32Exceptionhandling inProcessSignaler.TryGetRunningProcess. - Ensures process handles are disposed on inspection failure paths.
- Avoids returning processes that have exited or cannot be safely validated.
|
/backport to release/13.4 |
|
Started backporting to |
|
❓ CLI E2E Tests unknown — 108 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26656778748 |
|
✅ No documentation update needed. docs_required → already documented by name The only triggered signal is The
No documentation update is needed. |
Description
aspire stop --allcan monitor AppHost termination after an AppHost has already exited. On macOS, readingProcess.StartTimeduring that race can throwWin32Exception, which previously bubbled out ofProcessShutdownServiceand failed the stop command even though shutdown had completed.This updates
ProcessSignaler.TryGetRunningProcessto treatWin32Exceptionduring process inspection the same way it treats missing or already-exited processes: dispose the process handle, log at debug level, and returnnullso callers do not signal or kill a process they cannot safely validate.Validation:
dotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ProcessShutdownServiceTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes #17662
Checklist
<remarks />and<code />elements on your triple slash comments?