Fix telemetry UUID mutex acquisition - #28026
Merged
Dongbo Wang (daxian-dbw) merged 2 commits intoSep 21, 2026
Merged
Dongbo Wang (daxian-dbw) merged 2 commits into
Dongbo Wang (daxian-dbw) merged 2 commits into
Conversation
Avoid recursively acquiring the named mutex so a telemetry-enabled process does not block subsequent shells when the UUID cache is missing. Add a bounded concurrent-process regression test. Co-authored-by: Copilot App <[email protected]>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Copilot started reviewing on behalf of
Justin Chung (jshigetomi)
September 18, 2026 02:33
View session
7 tasks done
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The abandoned-mutex path can still retain ownership because WaitOne() occurs outside the release finally block.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes telemetry UUID mutex ownership and adds a regression test for concurrent startup with a missing UUID cache.
Changes:
- Creates the mutex without initial ownership.
- Adds bounded holder/child process coverage.
File summaries
| File | Description |
|---|---|
src/System.Management.Automation/utils/Telemetry.cs |
Adjusts mutex acquisition and release. |
test/powershell/engine/Basic/Telemetry.Tests.ps1 |
Adds concurrent startup regression coverage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Dongbo Wang (daxian-dbw)
approved these changes
Sep 18, 2026
Dongbo Wang (daxian-dbw)
requested changes
Sep 18, 2026
Handle abandoned mutex ownership and stop UUID creation when acquiring the named mutex exceeds 200 milliseconds. Cover the timeout path with a bounded child-process regression test. Co-authored-by: Copilot App <[email protected]>
Dongbo Wang (daxian-dbw)
approved these changes
Sep 21, 2026
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.
PR Summary
Fix the telemetry UUID cache mutex so each process acquires it exactly once and releases every acquired ownership path. The mutex is created with
initiallyOwned: false, acquisition is bounded to 200 ms, and ownership transferred byAbandonedMutexExceptionis tracked and released.Add regression coverage for both concurrency cases: a telemetry-enabled holder process followed by an empty-cache child, and a child that must stop UUID creation when another process holds the named mutex past the acquisition timeout. Process completion is bounded to 15 seconds so a recurrence fails instead of hanging the suite indefinitely.
PR Context
Creating the named mutex with
initiallyOwned: trueacquires it immediately. CallingWaitOne()again recursively acquires the same mutex, while the existing cleanup calledReleaseMutex()only once. The process therefore retained ownership until exit and could block another telemetry-enabled shell indefinitely when that shell needed to recreate a missing UUID cache.The acquisition state now also accounts for
WaitOne()transferring ownership before throwingAbandonedMutexException. If acquisition exceeds 200 ms, UUID creation stops and telemetry is disabled for that process rather than blocking startup.Validation:
test/powershell/engine/Basic/Telemetry.Tests.ps1.ReleaseAutomationTest-719777-ps) succeeded from Release-Automation SHA2e97805a8732f12a8478ad06a4a97af885ccfed6, using PowerShell sourcerelease/v7.7.99-preview.94at9e1bf1694d95a1e684b20039720ba438a2e94606, coordinated build 719739, and package build 719755. Native x64 and ARM64 concurrent-holder/empty-cache children each completed in 3 seconds within a 20-second gate; the full elevated and unelevated suites reported 0 failures.11.0.100-rc.1.26425.128, which is not installed on this host.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header