Fix TaskHost routing order after nested build callbacks

[JanProvaznik]

Fixes #15067.

Ordering bug

A TaskHost normally runs one task at a time, but a task waiting for a nested build can let another task use the same process. The earlier task can then resume first.

The parent remembered the last task started, not the task that had resumed. If A waited, then B waited, and A got its node back first, the parent still sent A's messages to B. In the reproducer, B's child needed A's target result: A's completion waited in B's queue while B waited for A.

Fix

When a task gets its node back after a nested build, the parent first restores the TaskHost's association with that task, then sends back the build results. The existing callback response already identifies which wait to release in the TaskHost. No extra identity needs to be sent.

The TaskHost already saves a task's working directory and environment before it waits. When the response arrives, its existing RequestId wakes the correct callback, and the task restores its saved environment before BuildProjectFile returns. That part is unchanged: the missing step was restoring the parent's association before sending the reply.

sequenceDiagram
    participant H as TaskHost
    participant P as Parent
    H->>P: A calls BuildProjectFile for ChildA
    Note over H: A waits
    H->>P: B calls BuildProjectFile for ChildB
    Note over H,P: B waits and ChildB needs target A
    P->>P: ChildA finishes and A gets its node back
    P->>P: Remember A as the current task
    P-->>H: A's build results with the existing RequestId
    H->>P: A's next callback or completion
    Note over P: Deliver to A

Loading

Other waiting tasks remain registered, and late replies cannot affect a replacement TaskHost. Finished tasks leave their results in a queue so one result cannot overwrite another before it is sent. Console output is sent before the corresponding result.

There are no packet-format changes, additional invocation identifiers, or protocol-version changes. The same fix applies to .NET and full-framework MSBuild.

How this was validated

The reproducer builds two targets of the same project configuration through two parallel wrappers, using -m:2 -nr:false. File gates establish the ordering rather than relying on sleeps:

1. A's child announces that it is waiting.
2. Only then can B enter its nested build.
3. B's child releases A's child, then requests A's target result.

This forces A to return while B remains blocked. Before the fix, sender-side traces showed A returning but neither parent target completing, and the MT build reached the 120-second bound. Normal mode served as the control.

The same graph was run against locally built .NET MSBuild and full-framework MSBuild.exe. The MT runs verify that A and B used the same external TaskHost. Runtime traces distinguish System.Private.CoreLib from mscorlib, so the CLR4 result is not inferred from the driver's target framework. Each parent must receive its own role and process-ID outputs exactly once, and the call/return log entries must keep the original parent task ID. Both runtimes now complete with those checks satisfied.

The edge cases keep three tasks attached to one host, resume the earliest one, remove another, and check which task receives the next message. A closed connection is checked both before and after a replacement is attached. For result delivery, two results are queued together and another is added during console flushing, checking that no result is lost, duplicated, or sent ahead of its buffered output. Cancellation coverage exercises active and waiting task contexts and isolated AppDomains.

[Copilot]

🔵 Needs a closer look

The broad protocol change and remaining documentation-reference nit warrant final human review.

Pull request overview

This pull request fixes non-LIFO TaskHost completion routing by associating task-originated packets with their invocation owners.

Changes:

• Adds protocol-v8 invocation IDs and packet envelopes.
• Routes logs, callbacks, and completions to exact handlers.
• Adds compatibility, exception, lifecycle, regression tests, and documentation.

File summaries

File	Summary
src/Shared/TaskHostTaskPacket.cs	Defines invocation packet envelopes.
src/Shared/TaskHostConfiguration.cs	Serializes versioned invocation identity.
src/MSBuild/OutOfProcTaskHostNode.cs	Preserves ownership for queued completions and logs.
src/MSBuild/MSBuild.csproj	Includes the shared packet implementation.
src/MSBuild.UnitTests/OutOfProcTaskHostNode_Tests.cs	Tests completion draining and console flushing.
src/Framework/BackEnd/NodePacketTypeExtensions.cs	Adds protocol versioning and compatibility gates.
src/Framework/BackEnd/NodePacketType.cs	Registers the new envelope packet type.
src/Build/Microsoft.Build.csproj	Includes the shared packet implementation.
src/Build/Instance/TaskFactories/TaskHostTask.cs	Handles invocation ownership and callback exceptions.
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs	Resolves packet owners and removes exact handlers.
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs	Handles malformed-packet connection failures.
src/Build.UnitTests/BackEnd/TaskHostTaskPacket_Tests.cs	Tests envelope serialization and compatibility.
src/Build.UnitTests/BackEnd/TaskHostNonLifoReentrancyTasks.cs	Provides non-LIFO regression tasks.
src/Build.UnitTests/BackEnd/TaskHostNonLifoReentrancy_Tests.cs	Tests non-LIFO completion behavior.
src/Build.UnitTests/BackEnd/TaskHostLifetimeProtocol_Tests.cs	Tests lifecycle and invalid identity handling.
src/Build.UnitTests/BackEnd/TaskHostCallbackException_Tests.cs	Tests callback exception propagation and policies.
documentation/specs/multithreading/taskhost-threading.md	Documents invocation ownership and compatibility.

Review details

Suppressed comments (1)

src/Framework/BackEnd/NodePacketType.cs:161

• This <see cref> cannot resolve in the Framework project: TaskHostTaskPacket is only linked into the Build and MSBuild projects, not Microsoft.Build.Framework, while documentation generation is enabled for the repository (src/Directory.Build.props:59). That produces the XML documentation warning for this changed comment and can fail the warnings-as-errors build. Use a non-reference code span here, or move the shared type into a project that Framework can reference.

    /// enclose this payload in <see cref="TaskHostTaskPacket"/> to identify its owner.

• Files reviewed: 17/17 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

#	Dimension	Verdict
13	Concurrency & Thread Safety	🔴 1 BLOCKING
17	File I/O & Path Handling	🟠 1 MAJOR

✅ 22/24 dimensions clean.

• Concurrency & Thread Safety — exception/cancellation responses can resume a task without restoring its handler, misrouting terminal packets
• File I/O & Path Handling — semicolons in the opt-in evidence path break logger parameter parsing

Warning

Firewall blocked 4 domains

The following domains were blocked by the firewall during workflow execution:

• api.github.com
• github.com
• patchdiff.githubusercontent.com
• raw.githubusercontent.com

[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:

tools:
  github:
    mode: gh-proxy

See GitHub Tools for more information on gh-proxy mode.

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.github.com"
    - "github.com"
    - "patchdiff.githubusercontent.com"
    - "raw.githubusercontent.com"

See Network Configuration for more information.

Generated by Expert Code Review (on open) for #15069 · copilot · gpt56 · 1.4K AIC · ⌖ 4.51 AIC · ⊞ 17.8K

[JanProvaznik]

Added a small scheduler-state guard in 813a57e.

_taskCancelled == false does not prove that the task got its node back: cancellation signals the blocked wait before calling TaskHostTask.Cancel(). Reactivation now also requires the owning BuildRequestEntry to be Active. Cancelling a waiting request leaves it Ready; normal scheduler resumption makes it Active.

This check runs in the parent. TaskHostTask is the parent-side wrapper, and its BuildEngine is the parent's TaskHost, which holds that request entry. The actual task runs in the external process with OutOfProcTaskHostNode as its build engine.

No protocol or cancellation-order changes are involved. The ordering gap is supported by the source, but the quoted interleaving alone does not establish a cancellation hang. The deterministic cancellation demo remains to be validated.

addressed/dismissed

[rainersigwald]

"resume" or "become unblocked"? That is, does the task actually do anything, or is it blocked until the taskhost node is free, like it would have been blocked until the worker was freed in in-proc mode?

[rainersigwald]

Calling this handlers in the new code and handlerStack in other code threw me for a bit of a loop, should it be more consistent?