Workaround: route NuGet RestoreTask to transient TaskHost in server or mt modes by OvesN · Pull Request #13660 · dotnet/msbuild

OvesN · 2026-04-30T12:44:06Z

Context

NuGet's RestoreTask holds static singletons (PluginManager, EnvironmentVariableWrapper) that assume
one invocation per process. Two MSBuild modes break that assumption:

MSBuild Server (DOTNET_CLI_USE_MSBUILD_SERVER=1 / MSBUILDUSESERVER=1): one process services many
builds back-to-back.
Multi-threaded MSBuild (/mt): RestoreTask is already routed to a TaskHost for thread safety, because it is not migrated, but it's a long-lived sidecar reused for every invocation in the build — statics leak between projects.

In both cases NuGet's static state survives past its intended scope.

This PR routes RestoreTask to a transient TaskHost (nodeReuse=false) when either mode is active, so
the spawned MSBuild.exe exits after Execute() and statics die with it

Changes Made

Why the original attempt didn't work

Original commit.
The server-mode trigger read Traits.Instance.UseMSBuildServer, which checks
MSBUILDUSESERVER. That env var is 0/null in the worker process where tasks run,
stripped by:

NodeLauncher.DisableMSBuildServer zeroes it before spawning the Server child
(recursion guard).
OutOfProcServerNode.HandleServerNodeBuildCommand overwrites the server's env from
the client's snapshot, which doesn't include MSBuild internals.

Net effect: the workaround branch never fired.

What this PR does

Host-info component IHostInfo
(src/Build/BackEnd/Components/Host/IHostInfo.cs) with two impls:
TransientHostInfo (default, registered by BuildComponentFactoryCollection)
and LongLivedServerHostInfo (registered once at startup by
OutOfProcServerNode.Run).
Allow-list TaskRouter.KnownProblematicTaskName +
TaskRouter.IsKnownProblematicTask — currently the single entry
NuGet.Build.Tasks.RestoreTask.
Routing decision AssemblyTaskFactory.CreateTaskInstance — when the task
is on the allow-list AND (/mt mode OR the registered IHostInfo reports
IsLongLivedHost == true), force useTaskFactory = true and
forceTransientTaskHost = true. The host lookup is null- and
missing-component-safe (returns false for hosts that don't register
IHostInfo).

Diagnostic logging added

Per-invocation TaskHost diagnostics in TaskHostTask.Execute (low importance, captured
in binlog) — complements the existing ExecutingTaskInTaskHost message. Records
ProcessId, ParentProcessId, NewNodeContext, IsSidecar, NodeReuseEffective.
Useful when investigating any TaskHost-routing question.

Testing

Unit tests in src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs:

Manual end-to-end with dotnet restore App.csproj /bl:r.binlog:

With DOTNET_CLI_USE_MSBUILD_SERVER=1 (server mode) and again with /mt, opened the
binlog and confirmed two TaskHost details for task "RestoreTask" lines with
IsSidecar=False, NodeReuseEffective=False, and different ProcessId between
multiple dotnet restore invocations.
End-to-end repro — manual two-PAT scenario against the private
DevDiv vssdk NuGet feed, exercising NuGet's static EnvironmentWrapper /
PluginManager over a single long-lived MSBuild Server process. On main
the second restore reproduces the bug (401 Unauthorized); with this PR
it succeeds. Details and steps in the PR comment below.

Notes

This is just a temporary fix, should be deleted when we make restore task thread safe. Create and document the lifetime and correct usage of static members in Tasks #12246

Workaround for static singleton state issues in NuGet RestoreTask (e.g., PluginManager, EnvironmentWrapper) that persist across builds when running in sidecar TaskHost processes. When /mt mode or MSBuild server (MSBUILDUSESERVER=1) is active, RestoreTask is now forced to run in a transient (non-sidecar) TaskHost that terminates after execution, ensuring all static state is cleaned up. Changes: - TaskRouter: Add IsKnownProblematicTask() to identify tasks by full name - AssemblyTaskFactory: Force transient TaskHost for problematic tasks - Tests: Add unit and integration tests for the workaround Fixes dotnet#13315 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…alue of MSBUILDUSESERVER. Add logging for task host spawning details.

OvesN · 2026-04-30T13:53:44Z

/review

github-actions · 2026-04-30T13:56:28Z

✅ Expert Code Review (command) completed successfully!

Copilot

Pull request overview

Implements a workaround to isolate NuGet’s RestoreTask (which relies on process-wide static singletons) by forcing it onto a transient TaskHost when running under MSBuild Server mode or /mt, preventing static state from leaking across builds/invocations.

Changes:

Add an internal “original server mode” env var (_MSBUILDORIGINALUSESERVER) to preserve server-mode detection through environment snapshotting.
Route allow-listed “known problematic” tasks (currently NuGet.Build.Tasks.RestoreTask) to a non-sidecar (transient) TaskHost in /mt or server mode.
Add TaskHost diagnostic logging and integration tests validating routing + per-invocation process isolation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Framework/Traits.cs	Adds `_MSBUILDORIGINALUSESERVER` env var name and trait for detecting server-mode launch context.
src/Build/BackEnd/Components/Communications/NodeLauncher.cs	Stashes/restores the original `MSBUILDUSESERVER` value into the new internal env var around child process creation.
src/Build/BackEnd/Node/OutOfProcServerNode.cs	Preserves the internal env var across `SetEnvironment(...)` so server node can still detect original server-mode intent.
src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs	Introduces allow-list logic to identify “known problematic” tasks by full type name.
src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs	Forces problematic tasks to run in TaskHost and disables sidecar reuse in `/mt` or server mode.
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs	Extends host acquisition to report host PID / creation status for diagnostics.
src/Build/Instance/TaskFactories/TaskHostTask.cs	Logs per-invocation TaskHost details (PID, reuse, etc.) into the build log/binlog.
src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs	Adds integration tests for problematic-task routing and “fresh process per invocation” behavior.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

github-actions

Expert Review — 24-Dimension Analysis

Summary

#	Dimension	Verdict
1	Backward Compatibility	✅ LGTM (test-only)
2	ChangeWave Discipline	✅ LGTM (test-only)
3	Performance	✅ LGTM (test-only)
4	Allocation Awareness	✅ LGTM (test-only)
5	Test Coverage	✅ LGTM — covers MT mode, server mode, fresh-process guarantee, and no-workaround fallback
6	Error Message Quality	✅ LGTM (test-only)
7	Logging Fidelity	✅ LGTM (test-only)
8	String Comparison	✅ LGTM — regex matches ASCII digits only, `ShouldContain` is ordinal
9	API Surface	✅ LGTM (test-only)
10	Target Authoring	✅ LGTM (test-only)
11	Cross-Platform	✅ LGTM — uses `Path.Combine`, `Process.GetCurrentProcess().Id`, standard APIs
12	Code Simplification	✅ LGTM — boilerplate duplication is acceptable for test readability
13	Concurrency	✅ LGTM — `xunit.runner.json` enforces `parallelizeTestCollections: false` / `maxParallelThreads: 1`
14	Naming Precision	✅ LGTM — names are descriptive and consistent with existing tests
15	SDK Integration	✅ LGTM (test-only)
16	Evaluation Model	✅ LGTM (test-only)
17	Correctness	✅ LGTM — fake `RestoreTask` FullName matches `TaskRouter.IsKnownProblematicTask` check; `Build()` overload is self-contained
18	Documentation Accuracy	📝 NIT — see inline comment
19	Dependency Management	✅ LGTM (test-only)
20	Scope Discipline	✅ LGTM — tests + comment cleanup in the same PR is reasonable
21	Security	✅ LGTM (test-only)
22	Build Infrastructure	✅ LGTM (test-only)
23	Binary Log Compatibility	✅ LGTM (test-only)
24	Error Handling	✅ LGTM (test-only)

Findings: 1 NIT

One nit on the server-mode test's EnableNodeReuse comment — // Load-bearing: see the /mt counterpart. is a fragile cross-reference that could become a dead pointer. Consider duplicating the 1-line explanation inline. Details in the inline comment.

The comment cleanup is well-executed overall: removed XML docs restated what the method name already says, // Arrange/// Act/// Assert markers were noise for these straightforward tests, and the condensed // Load-bearing: comments on the MT test preserve the non-obvious reasoning. The tests themselves are correct and well-structured.

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

#13660 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
#13660 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (command) for issue #13660 · ● 9.7M

…OvesN/msbuild into workaround-restore-issues-in-mt

AR-May and others added 2 commits April 22, 2026 10:52

Add tests, fix workaround by adding new env variable that preserves v…

a1ce582

…alue of MSBUILDUSESERVER. Add logging for task host spawning details.

OvesN force-pushed the workaround-restore-issues-in-mt branch from 9793967 to a1ce582 Compare April 30, 2026 13:08

OvesN added 2 commits April 30, 2026 15:17

Fix merge conflicts

655b66b

Delete comment

692e76f

OvesN changed the title ~~Workaround restore issues in mt~~ Workaround: route NuGet RestoreTask to transient TaskHost in server or mt modes Apr 30, 2026

Delete unnecessary comments in tests

1894d52

OvesN marked this pull request as ready for review April 30, 2026 13:53

Copilot AI review requested due to automatic review settings April 30, 2026 13:53

Copilot started reviewing on behalf of OvesN April 30, 2026 13:54 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs

Comment thread src/Framework/Traits.cs Outdated

Remove unused directive

f056afd

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

github-actions Bot reviewed Apr 30, 2026

View reviewed changes

Comment thread src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs

OvesN requested a review from AR-May April 30, 2026 14:08

OvesN added 2 commits April 30, 2026 17:08

Fix tests and comment.

761ed02

Merge branch 'workaround-restore-issues-in-mt' of https://github.com/…

c31d787

…OvesN/msbuild into workaround-restore-issues-in-mt

JanProvaznik reviewed Apr 30, 2026

View reviewed changes

Comment thread src/Build/BackEnd/Components/RequestBuilder/TaskRouter.cs Outdated

This was referenced May 4, 2026

Expose operational counters #12695

Closed

Rewrite resolveprojectreferences MSBuild calls to reduce the amount of batching done #12631

Closed

JanProvaznik mentioned this pull request May 6, 2026

MSBuild Server - enable for all dotnet CLI-based builds #9379

Open

Fix import

6f99ec9

github-actions Bot mentioned this pull request May 11, 2026

Add CI/CD provider-specific loggers for GitHub Actions, Azure DevOps, and GitLab #12738

Closed

OvesN and others added 3 commits May 12, 2026 10:11

Use component instead of env var to decide if node is server or not

3d12009

make IsLongLivedHost null safe

1b3d836

Merge branch 'main' into workaround-restore-issues-in-mt

506eb5d

JanProvaznik mentioned this pull request May 12, 2026

Provide environment variables overriding routing decision for tasks #13738

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workaround: route NuGet RestoreTask to transient TaskHost in server or mt modes#13660

Workaround: route NuGet RestoreTask to transient TaskHost in server or mt modes#13660
OvesN wants to merge 12 commits into
dotnet:mainfrom
OvesN:workaround-restore-issues-in-mt

OvesN commented Apr 30, 2026 •

edited

Loading

Uh oh!

OvesN commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

OvesN commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes Made

Why the original attempt didn't work

What this PR does

Diagnostic logging added

Testing

Notes

Uh oh!

OvesN commented Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Expert Review — 24-Dimension Analysis

Summary

Findings: 1 NIT

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

OvesN commented Apr 30, 2026 •

edited

Loading

github-actions Bot commented Apr 30, 2026 •

edited

Loading