Skip to content

Fix telemetry UUID mutex acquisition - #28026

Merged
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
jshigetomi:jshigetomi-fix-telemetry-mutex
Sep 21, 2026
Merged

Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
jshigetomi:jshigetomi-fix-telemetry-mutex

Conversation

@jshigetomi

@jshigetomi Justin Chung (jshigetomi) commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

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 by AbandonedMutexException is 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: true acquires it immediately. Calling WaitOne() again recursively acquires the same mutex, while the existing cleanup called ReleaseMutex() 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 throwing AbandonedMutexException. If acquisition exceeds 200 ms, UUID creation stops and telemetry is disabled for that process rather than blocking startup.

Validation:

  • PowerShell parser: passed for test/powershell/engine/Basic/Telemetry.Tests.ps1.
  • Local Pester 4.10.1: discovered 18 telemetry tests; all 18 were skipped because this Windows host's diagnostics policy disables PowerShell telemetry. The regression tests did not execute locally.
  • Native Linux validation: ADO run 719777 (ReleaseAutomationTest-719777-ps) succeeded from Release-Automation SHA 2e97805a8732f12a8478ad06a4a97af885ccfed6, using PowerShell source release/v7.7.99-preview.94 at 9e1bf1694d95a1e684b20039720ba438a2e94606, 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.
  • Local product build was not run because this checkout pins .NET SDK 11.0.100-rc.1.26425.128, which is not installed on this host.

PR Checklist

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]>
@jshigetomi
Justin Chung (jshigetomi) requested a review from a team as a code owner September 18, 2026 02:33
Copilot AI lite review requested due to automatic review settings September 18, 2026 02:33
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/System.Management.Automation/utils/Telemetry.cs Outdated
@daxian-dbw Dongbo Wang (daxian-dbw) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 18, 2026
Comment thread src/System.Management.Automation/utils/Telemetry.cs Outdated
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label 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]>
@microsoft-github-policy-service microsoft-github-policy-service Bot removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Sep 18, 2026
@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 03aae69 into PowerShell:master Sep 21, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants