Skip to content

Use eng\common\dotnet.cmd instead of .\dotnet.cmd in crossgen2 comparison legs#131007

Merged
jtschuster merged 1 commit into
dotnet:mainfrom
jtschuster:fix-crossgen2-dotnet-race
Jul 20, 2026
Merged

Use eng\common\dotnet.cmd instead of .\dotnet.cmd in crossgen2 comparison legs#131007
jtschuster merged 1 commit into
dotnet:mainfrom
jtschuster:fix-crossgen2-dotnet-race

Conversation

@jtschuster

Copy link
Copy Markdown
Member

dotnet.cmd unconditionally calls InitializeDotnetCLI $true $true which locks sdk.txt for a short period during startup. Later in the batch file, sdk.txt is read. In crossgen2 comparison jobs, over 200 parallel invocations happen at once, so occasionally sdk.txt is locked when trying to read, and the eventual call to the dotnet cli fails.

Instead, use .\eng\common\dotnet.ps1 to invoke the cli, which doesn't touch sdk.txt.

Fixes #131004
Fixes #131005

@jtschuster jtschuster self-assigned this Jul 18, 2026
Copilot AI review requested due to automatic review settings July 18, 2026 01:04
@github-actions github-actions Bot added the area-crossgen2-coreclr only use for closed issues label Jul 18, 2026
@jtschuster jtschuster added area-ReadyToRun and removed area-crossgen2-coreclr only use for closed issues labels Jul 18, 2026
@jtschuster

Copy link
Copy Markdown
Member Author

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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.

Pull request overview

This PR updates the crossgen2 comparison build job to avoid using the repo-root dotnet.cmd wrapper in Windows comparison runs, reducing contention around artifacts/toolset/sdk.txt when many parallel dotnet invocations occur.

Changes:

  • Removed the “pre-initialize dotnet CLI” step that existed to mitigate parallel dotnet.cmd races.
  • Switched the Windows --dotnet argument passed to crossgen2_comparison.py from $(Build.SourcesDirectory)\dotnet.cmd to $(Build.SourcesDirectory)\eng\common\dotnet.cmd.

Comment thread eng/pipelines/coreclr/templates/crossgen2-comparison-build-job.yml
Comment thread eng/pipelines/coreclr/templates/crossgen2-comparison-build-job.yml
@jtschuster
jtschuster marked this pull request as ready for review July 20, 2026 18:58
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "4b7bc55c09bfeaf11ee1bfeb67343d28a7c5fda0",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "6593571d45a57a32e83de2269649e9182ed3d7f8",
  "last_reviewed_commit": "4b7bc55c09bfeaf11ee1bfeb67343d28a7c5fda0",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "6593571d45a57a32e83de2269649e9182ed3d7f8",
  "last_recorded_worker_run_id": "29772925211",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "4b7bc55c09bfeaf11ee1bfeb67343d28a7c5fda0",
      "review_id": 4738476748
    }
  ]
}

@github-actions github-actions Bot 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.

Holistic Review

Motivation: In the crossgen2 comparison legs, the Windows dotnet.cmd wrapper unconditionally calls InitializeDotNetCLI $true $true, which briefly locks artifacts/toolset/sdk.txt at startup and reads it later. With 200+ parallel invocations from the crossgen2 fan-out, sdk.txt is occasionally locked when read, causing spurious "dotnet.exe not found" failures (issues #131004, #131005). A prior workaround added a pre-initialize step to populate sdk.txt before the fan-out.

Approach: Switch the Windows crossgen invocation from $(Build.SourcesDirectory)\dotnet.cmd to $(Build.SourcesDirectory)\eng\common\dotnet.cmd, which forwards to eng/common/dotnet.ps1. That path calls InitializeDotNetCli -install:$true and never touches sdk.txt, eliminating the lock contention. The now-unnecessary pre-initialize step (added specifically to warm sdk.txt) is removed for both OS branches.

Summary: The fix is correct and minimal. The race was Windows-specific because only the root dotnet.cmd/sdk.txt path uses a shared file that parallel processes contend on; the eng/common wrapper resolves the SDK via InitializeDotNetCli without a shared sdk.txt read, so it is inherently race-free. Leaving the non-Windows argument on the root dotnet.sh is consistent, since the shell wrapper resolves the SDK through the _InitializeDotNetCli variable rather than sdk.txt and never had this race. Removing the pre-initialize step is safe: its sole purpose was to pre-populate sdk.txt, which is no longer read; the SDK itself is already installed by earlier build stages, so InitializeDotNetCli no-ops rather than racing to reinstall across the parallel work items. No functional, correctness, or security concerns. LGTM.

Detailed Findings

No actionable findings.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 41.3 AIC · ⌖ 10.4 AIC · ⊞ 10K

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Crossgen2 comparison mismatch: System.IO.FileSystem.DriveInfo Crossgen2 comparison mismatch: Microsoft.Bcl.AsyncInterfaces

5 participants