Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node by JanProvaznik · Pull Request #13350 · dotnet/msbuild

JanProvaznik · 2026-03-09T19:01:04Z

Summary

Stage 3 of IBuildEngine callback support for out-of-process TaskHost. Implements forwarding of BuildProjectFile* from OOP TaskHost to the owning worker node, enabling tasks ejected to TaskHost (via -mt or explicit TaskHostFactory) to build other projects.

Real-world impact: WPF XAML compilation uses GenerateTemporaryTargetAssembly which calls BuildProjectFile from TaskHost. Without this change, building any WPF project with -mt fails with MSB5022.

Design

Packet pair

TaskHostBuildRequest (0x20) / TaskHostBuildResponse (0x21): All 4 BuildProjectFile* overloads normalize to the 6-param BuildProjectFilesInParallel canonical form.

Callback flow (no Yield/Reacquire)

Unlike the original design, we do not use Yield/Reacquire for BuildProjectFile callbacks. Instead:

Task thread calls BuildProjectFile → gates on CallbacksSupported
BlockForCallback() transitions the task from active to blocked (_activeTaskCount--, _blockedTaskCount++)
SendCallbackRequestAndWaitForResponse<T> serializes the request and blocks on TaskCompletionSource
Worker's TaskHostTask.HandleBuildRequest calls IBuildEngine3.BuildProjectFilesInParallel — exceptions propagate to TaskBuilder naturally (matching in-proc behavior), finally sends the response
Response arrives → TCS completes → ResumeAfterCallback() restores counters and environment

A blocked task does not prevent the node from accepting new work — HandleTaskHostConfiguration checks _activeTaskCount == 0 (not blocked count), so nested task dispatch works.

Yield/Reacquire remain no-ops in the OOP TaskHost.

Per-task isolation

TaskExecutionContext stores per-task state via ConcurrentDictionary<int, TaskExecutionContext> + AsyncLocal: configuration, task wrapper, saved environment, debug flags, warning settings, pending callback requests. GetCurrentConfiguration() and EffectiveWarningsAs* read from per-task context on task threads, falling back to _currentConfiguration on the main communication thread.

Handler routing (process reuse)

NodeProviderOutOfProcTaskHost routes packets by TaskHostNodeKey, supporting nested task dispatch when an outer task is blocked on a callback and a new task arrives on the same TaskHost process.

Version gating

Gated behind CallbacksSupported (_parentPacketVersion >= 4 OR MSBUILDENABLETASKHOSTCALLBACKS=1). Each BuildProjectFile overload explicitly checks. Follow-up PR bumps PacketVersion to 4 and removes the env var.

Tests

Serialization tests — packet round-trip for TaskHostBuildRequest/Response with null/empty/populated properties
Integration tests — BuildProjectFile via TaskHostFactory, global property forwarding, target output retrieval, callbacks-not-supported error path (MSB5022)
E2E tests — cross-runtime BuildProjectFile callback, TaskHost process reuse (same PID verification)
MT mode test — BuildProjectFile during -mt build

Context

Spec: TaskHost IBuildEngine Callback Support spec #12868
Stage 1: [IBuildEngine callbacks] Stage 1: Packet infrastructure + IsRunningMultipleNodes #13149 (merged) — IsRunningMultipleNodes
Stage 2: [IBuildEngine callbacks] Stage 2: RequestCores/ReleaseCores #13306 (merged) — RequestCores/ReleaseCores
WPF repro: TaskHost Does Not Support Critical IBuildEngine Callbacks #12863

## What Adds folder-scoped **instructions**, topic-scoped **skills**, and an **expert review agent** for GitHub Copilot working in this repo. ## How it works - **Instructions** (`.github/instructions/`) activate automatically when editing files matching their `applyTo` glob. They provide folder-specific guidance (e.g., concurrency rules for `src/Build/BackEnd/`, evaluation model rules for `src/Build/Evaluation/`). - **Skills** (`.github/skills/`) activate on topic keywords in conversation. They cover overarching concerns like backward compatibility assessment, performance optimization, SDK integration, error authoring, and binary log compatibility. The existing `changewaves` skill gets a cross-reference to the new `assessing-breaking-changes` skill (how vs when). - **Review agent** (`.github/agents/expert-reviewer.md`) is invoked via `@expert-reviewer`. It runs a 4-wave review process: 1. **Find** — one sub-agent per quality dimension (24 total), parallel batches of 6. Each returns LGTM or a finding with exact file:line and a concrete failing scenario. 2. **Validate** — proves each finding by tracing code flow in the PR branch, writing proof-of-concept tests, or simulating thread timelines. Multi-model consensus (Opus + Codex + Gemini) for borderline cases. 3. **Post** — inline review comments at exact diff locations via GitHub CLI. 4. **Summary** — 24-dimension checkbox table as review body. ## Example Applied to [PR #13350](#13350 (review)) — found a shared-state concurrency race, an unlogged exception, and a null-deref edge case. 20 of 24 dimensions passed LGTM. ## Scope No production code changes. Only `.github/` files (instructions, skills, agent). One minor addition to the existing `changewaves` skill (cross-reference line). --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>

## What Adds folder-scoped **instructions**, topic-scoped **skills**, and an **expert review agent** for GitHub Copilot working in this repo. ## How it works - **Instructions** (`.github/instructions/`) activate automatically when editing files matching their `applyTo` glob. They provide folder-specific guidance (e.g., concurrency rules for `src/Build/BackEnd/`, evaluation model rules for `src/Build/Evaluation/`). - **Skills** (`.github/skills/`) activate on topic keywords in conversation. They cover overarching concerns like backward compatibility assessment, performance optimization, SDK integration, error authoring, and binary log compatibility. The existing `changewaves` skill gets a cross-reference to the new `assessing-breaking-changes` skill (how vs when). - **Review agent** (`.github/agents/expert-reviewer.md`) is invoked via `@expert-reviewer`. It runs a 4-wave review process: 1. **Find** — one sub-agent per quality dimension (24 total), parallel batches of 6. Each returns LGTM or a finding with exact file:line and a concrete failing scenario. 2. **Validate** — proves each finding by tracing code flow in the PR branch, writing proof-of-concept tests, or simulating thread timelines. Multi-model consensus (Opus + Codex + Gemini) for borderline cases. 3. **Post** — inline review comments at exact diff locations via GitHub CLI. 4. **Summary** — 24-dimension checkbox table as review body. ## Example Applied to [PR dotnet#13350](dotnet#13350 (review)) — found a shared-state concurrency race, an unlogged exception, and a null-deref edge case. 20 of 24 dimensions passed LGTM. ## Scope No production code changes. Only `.github/` files (instructions, skills, agent). One minor addition to the existing `changewaves` skill (cross-reference line). --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>

JanProvaznik · 2026-03-27T14:58:31Z

Post-review notes (automated multi-agent review of new commits)

5 new commits pushed addressing review meeting feedback. Multi-agent review (5 agents: expert-reviewer, 2× Opus 4.6, 2× GPT-5.4) ran against the delta. Findings addressed inline; remaining items for human attention:

Items addressed in these commits

✅ ETW refactor: diagnostic strings → ETW events (6 resource strings + 13 xlf translations removed)
✅ Shutdown ordering: FailAllPendingCallbackRequests before _taskRunnerThread.Join() (deadlock fix)
✅ ObjectDisposedException guard on _taskCompleteEvent.Set()
✅ ETW Stop event moved to finally block (all exit paths covered)
✅ Non-atomic field reads in ETW params fixed (capture Interlocked return values)
✅ TaskHostNestedTaskStart renamed to TaskHostNestedTaskDispatched (point-in-time, not unpaired Start)
✅ BuildInParallel=""true"" added to orchestrator tests (was defaulting to sequential execution)
✅ Coexistence test doc comment corrected (no longer overclaims handler stack isolation proof)

Items for human reviewer attention

_taskRunnerThread?.Join() has no timeout (pre-existing, not introduced by this PR). If a user task hangs (not blocked on TCS), shutdown hangs indefinitely. Nested task threads get Join(TimeSpan.FromSeconds(5)) but the main runner thread does not. Consider adding a timeout in a follow-up.
Coexistence test is a smoke test, not a proof of handler-stack isolation. With MaxNodeCount=1, even with BuildInParallel=true, the scheduler may serialize execution. A stronger test would assert PID equality across MT-ejected and explicit TaskHostFactory tasks to prove they shared a process. Left as-is since it still provides value as a "both paths work in one build" test.
No cancellation-during-callback test yet. Meeting agreed on OS mutex-based coordination for this (Rainer's suggestion). Deferred to follow-up.

Copilot

Pull request overview

Stage 3 of out-of-process TaskHost IBuildEngine callback support: forwards BuildProjectFile* and Yield/Reacquire from the OOP TaskHost back to the owning worker node so ejected tasks (notably WPF XAML compilation) can build projects and cooperate with the scheduler.

Changes:

Add new callback packet types for BuildProjectFile* and Yield/Reacquire, including serialization round-trip tests.
Implement callback forwarding in OutOfProcTaskHostNode, worker-side handling in TaskHostTask, and nested-task process reuse routing via a handler stack in NodeProviderOutOfProcTaskHost.
Add integration/E2E tests and update TaskHost threading documentation plus resource strings.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Shared/TaskHostBuildRequest.cs	New packet carrying canonical BuildProjectFilesInParallel request data.
src/Shared/TaskHostBuildResponse.cs	New response packet with success + optional target outputs (TaskParameter-serialized).
src/Shared/TaskHostYieldRequest.cs	New packet for Yield/Reacquire operations.
src/Shared/TaskHostYieldResponse.cs	New response packet to unblock TaskHost on Reacquire.
src/Shared/Resources/Strings.shared.resx	Adds new resource strings for callback/yield diagnostics.
src/Shared/Resources/xlf/Strings.shared.*.xlf	Adds corresponding localization entries for the new resource strings.
src/MSBuild/TaskExecutionContext.cs	Introduces per-task execution context for concurrent/nested TaskHost execution.
src/MSBuild/OutOfProcTaskHostNode.cs	Implements forwarding for BuildProjectFile* and Yield/Reacquire; adds per-task context + counters + completion queue.
src/MSBuild/MSBuild.csproj	Includes new shared packets + TaskExecutionContext in MSBuild build.
src/Build/Microsoft.Build.csproj	Includes new shared packets in Microsoft.Build build.
src/Build/Instance/TaskFactories/TaskHostTask.cs	Worker-side handlers for build/yield requests and response emission.
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs	Adds handler stack to support nested TaskHostTask routing and reuse.
src/Build.UnitTests/BackEnd/TaskHostCallbackPacket_Tests.cs	Adds serialization tests for the new request/response packet types.
src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs	Adds integration tests validating BuildProjectFile forwarding and fallback behavior.
src/Build.UnitTests/NetTaskHost_E2E_Tests.cs	Adds E2E tests for cross-runtime BuildProjectFile and same-PID reuse for nested builds.
src/Build.UnitTests/BackEnd/BuildProjectFileTask.cs	New test task that triggers BuildProjectFile callback and captures outputs.
src/Build.UnitTests/BackEnd/BuildProjectFileAndReportPidTask.cs	New test task for PID reporting + nested BuildProjectFile call (reuse verification).
src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskBuildCallback/*	New E2E asset project for callback validation.
src/Build.UnitTests/TestAssets/ExampleNetTask/TestNetTaskHostReuse/*	New E2E asset project for nested-build process reuse validation.
src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.csproj	Links new test tasks into ExampleTask assembly.
documentation/specs/multithreading/taskhost-threading.md	Documents Stage 3 callback flow, yield-for-callback pattern, and handler stack.

JanProvaznik · 2026-04-02T14:52:55Z

I investigated supporting explicit IBuildEngine3.Yield() / Reacquire() in the OOP TaskHost and decided not to implement it in this change.

The short version is that this is not a small missing feature in the current design—it conflicts with several core assumptions of the OOP task-host architecture due to having one pipe for communication:

Packet routing breaks down. The parent side routes incoming packets to the top handler on a per-node stack. Once a task explicitly yields and another task is scheduled on the same OOP node, packets from the yielded task can be delivered to the wrong handler.
Logs can be silently misattributed. If a yielded task’s log packet is routed to the wrong TaskHostTask, the event gets stamped with the wrong BuildEventContext, which would corrupt console/binlog diagnostics.
We would need per-packet handler identity

By contrast, the existing internal callback-based blocking path works because it blocks the task thread while the callback is in flight. That preserves the single-active-handler assumption and avoids the routing/logging/state-machine issues above.

So the conclusion here is: supporting explicit yield in OOP TaskHost would require broader architectural and wire-protocol work than is appropriate for this PR, and a partial implementation would be brittle and potentially incorrect.

Not implementing Yield does not regress correctness, however negative perf impact in -mt mode is possible for custom tasks that use this feature and are not migrated, and we may revisit this if there is evidence of many customers using Yield while being blocked from migration to -mt aware tasks.

Updated [Microsoft.Build](https://github.com/dotnet/msbuild) from 18.6.3 to 18.7.1. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Build's releases](https://github.com/dotnet/msbuild/releases)._ ## 18.7.1 ## What's Changed * Fix TraceEngine file contention deadlock in multithreaded mode by @JanProvaznik in dotnet/msbuild#13446 * Remove duplicate test cases in MultithreadableTaskAnalyzer by @Youssef1313 in dotnet/msbuild#13483 * Ensure ThreadSafeTaskAnalyzer.Tests is considered as a unit test project by @Youssef1313 in dotnet/msbuild#13481 * Fix MSBuildTask0002 analyzer warnings in already-migrated tasks by @JanProvaznik in dotnet/msbuild#13466 * Fix race conditions in task host path resolution by @AR-May in dotnet/msbuild#13485 * Migrate ToolTask and Al task to TaskEnvironment API by @OvesN in dotnet/msbuild#13423 * Bump main to 18.7, add vs18.6 to merge flow by @MichalPavlik in dotnet/msbuild#13472 * Avoid allocations in GetHashCode implementations by @DustinCampbell in dotnet/msbuild#13475 * Add PATs rotation to agentic workflow(s) by @JanKrivanek in dotnet/msbuild#13496 * Fix ASP.NET WebSite projects to copy netstandard.dll facade when required by @JanProvaznik in dotnet/msbuild#13058 * Migrate AspNetCompiler to TaskEnvironment API by @OvesN in dotnet/msbuild#13424 * Add review workflow by @JanKrivanek in dotnet/msbuild#13503 * Strengthen reviewer skill: add step-back analysis dimensions by @JanProvaznik in dotnet/msbuild#13504 * Add 'Request Speedometer Perf Run' to VS experimental insertion build policies by @Copilot in dotnet/msbuild#13505 * Remove duplicate @ prefix from issueAuthor in GitOps by @akoeplinger in dotnet/msbuild#13492 * Improve review aw by @JanKrivanek in dotnet/msbuild#13510 * Migrates unit tests to use RoslynCodeTaskFactory to enable running tests under .NET Core by @jankratochvilcz in dotnet/msbuild#13500 * Fix cross-AppDomain TaskItem modifier cache regression by @DustinCampbell in dotnet/msbuild#13493 * Discourage review agent from approving PRs by @JanKrivanek in dotnet/msbuild#13512 * Stop trying to deploy ValueTuple by @rainersigwald in dotnet/msbuild#13507 * Ad-hoc re-sign bootstrap dotnet on macOS to prevent SIGKILL by @jankratochvilcz in dotnet/msbuild#13513 * RoslynCodeTaskFactory: Log MSB3753 when task class does not implement ITask by @jankratochvilcz in dotnet/msbuild#13517 * Update gh-aw (upon mcp policy changes) by @JanKrivanek in dotnet/msbuild#13526 * Eliminate XmlChildNodes allocations in GetXmlNodeInnerContents by @nareshjo in dotnet/msbuild#13509 * Fix telemetry allocation regression: per-engine collector ownership by @JanProvaznik in dotnet/msbuild#13516 * Migrate to xunit.v3 by @Youssef1313 in dotnet/msbuild#13482 * Fix stray brace in HandleBuildCancel trace string causing MSB1025 by @Copilot in dotnet/msbuild#13535 * Bumping to 10.0.4 runtime packages by @MichalPavlik in dotnet/msbuild#13533 * Remove early return in GetCanonicalForm, always call System.IO.Path by @OvesN in dotnet/msbuild#13532 * Do not overwrite GetCopyToOutputDirectoryItemsDependsOn, just add new… by @snechaev in dotnet/msbuild#13474 * Migrate GetReferenceAssemblyPaths task to TaskEnvironment API by @OvesN in dotnet/msbuild#13495 * Stabilize ToolTaskThatTimeoutAndRetry test by @rainersigwald in dotnet/msbuild#13489 * [automated] Merge branch 'vs18.6' => 'main' by @github-actions[bot] in dotnet/msbuild#13506 * Add extra test assertions around tests by @Youssef1313 in dotnet/msbuild#13536 * Add static eval for repo skills/agents via skill-validator by @JanKrivanek in dotnet/msbuild#13537 * Migrate SGen task to Task environment API by @OvesN in dotnet/msbuild#13457 * Fix TerminalLogger assert failure for metaproj files and cached project eval ID by @OvesN in dotnet/msbuild#13480 * Filter out approving review from pr-reviewer agent by @JanKrivanek in dotnet/msbuild#13553 * Use a unique task name per invocation to tabilize RoslynCodeTaskFactory_ReuseCompilation test by @huulinhnguyen-dev in dotnet/msbuild#13551 * Brief doc on feedback/logging/data systems by @rainersigwald in dotnet/msbuild#13554 * Localized file check-in by OneLocBuild Task: Build definition ID 9434: Build ID 13881982 by @dotnet-bot in dotnet/msbuild#13437 * Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node by @JanProvaznik in dotnet/msbuild#13350 * Enable TaskHost Callbacks by default by @JanProvaznik in dotnet/msbuild#13579 * Remove unactionable info from reviewer agent by @JanKrivanek in dotnet/msbuild#13578 * Enlighten RequiresFramework35SP1Assembly task for multithreaded mode by @jankratochvilcz in dotnet/msbuild#13575 * Make SdkResolver-provided environment variables take precedence over ambient environment by @Copilot in dotnet/msbuild#12655 * Add dotnet/skills marketplace and enable plugins by @Evangelink in dotnet/msbuild#13582 * The skills/agents check filters-in only touched files by @JanKrivanek in dotnet/msbuild#13586 * Fix skill-validation workflow failing when agents directory is deleted by @JeremyKuhne in dotnet/msbuild#13592 ... (truncated) Commits viewable in [compare view](dotnet/msbuild@v18.6.3...v18.7.1). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

This comment was marked as resolved.

Sign in to view

T-Gro mentioned this pull request Mar 17, 2026

Proposal: instructions, skills and review agent #13397

Merged

This comment was marked as resolved.

Sign in to view

This comment was marked as duplicate.

Sign in to view

This comment was marked as resolved.

Sign in to view

JanProvaznik commented Mar 23, 2026

View reviewed changes

Comment thread src/Build/Instance/TaskFactories/TaskHostTask.cs

JanProvaznik commented Mar 23, 2026

View reviewed changes

Comment thread src/MSBuild/OutOfProcTaskHostNode.cs

JanProvaznik commented Mar 23, 2026

View reviewed changes

Comment thread src/MSBuild/OutOfProcTaskHostNode.cs Outdated

JanProvaznik force-pushed the ibuildengine-callbacks-stage3 branch 4 times, most recently from b954baf to c135644 Compare March 24, 2026 17:17

0101 mentioned this pull request Mar 26, 2026

Parallelize assessment phase: one agent per finding 0101/focused-review#3

Merged

JanProvaznik commented Mar 26, 2026

View reviewed changes

Comment thread src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs

JanProvaznik marked this pull request as ready for review March 27, 2026 15:37

Copilot AI review requested due to automatic review settings March 27, 2026 15:37

Copilot started reviewing on behalf of JanProvaznik March 27, 2026 15:37 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread src/MSBuild/OutOfProcTaskHostNode.cs Outdated

Comment thread src/MSBuild/OutOfProcTaskHostNode.cs Outdated

Comment thread src/MSBuild/OutOfProcTaskHostNode.cs Outdated

AR-May reviewed Mar 31, 2026

View reviewed changes

Comment thread documentation/specs/multithreading/taskhost-threading.md Outdated

Comment thread documentation/specs/multithreading/taskhost-threading.md

Comment thread src/Shared/TaskHostBuildRequest.cs

Comment thread documentation/specs/multithreading/taskhost-threading.md

JanProvaznik self-assigned this Apr 1, 2026

JanProvaznik marked this pull request as draft April 2, 2026 11:54

JanProvaznik force-pushed the ibuildengine-callbacks-stage3 branch 3 times, most recently from 4e86e10 to b1c2ab1 Compare April 2, 2026 14:37

JanProvaznik force-pushed the ibuildengine-callbacks-stage3 branch from b1c2ab1 to 573ac4c Compare April 2, 2026 16:06

Uh oh!

Conversation

JanProvaznik commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design

Packet pair

Callback flow (no Yield/Reacquire)

Per-task isolation

Handler routing (process reuse)

Version gating

Tests

Context

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JanProvaznik commented Mar 27, 2026

Post-review notes (automated multi-agent review of new commits)

Items addressed in these commits

Items for human reviewer attention

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JanProvaznik commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JanProvaznik commented Mar 9, 2026 •

edited

Loading