Skip to content

Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node#13350

Draft
JanProvaznik wants to merge 13 commits intodotnet:mainfrom
JanProvaznik:ibuildengine-callbacks-stage3
Draft

Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node#13350
JanProvaznik wants to merge 13 commits intodotnet:mainfrom
JanProvaznik:ibuildengine-callbacks-stage3

Conversation

@JanProvaznik
Copy link
Member

@JanProvaznik JanProvaznik commented Mar 9, 2026 •
edited
Loading

Summary

Stage 3 of IBuildEngine callback support for out-of-process TaskHost. Implements forwarding of all 4 BuildProjectFile* overloads (IBuildEngine, IBuildEngine2, IBuildEngine3) from OOP TaskHost to the owning worker node, plus Yield/Reacquire forwarding and TaskHost process reuse for nested builds.

Design

Single packet pair for BuildProjectFile: All 4 overloads normalize into IBuildEngine3 6-param canonical form, then serialize into TaskHostBuildRequest (0x20) / TaskHostBuildResponse (0x21) pair.

Yield/Reacquire forwarding: TaskHostYieldRequest (0x26) / TaskHostYieldResponse (0x27) packets forward IBuildEngine3.Yield() (fire-and-forget) and Reacquire() (blocking) from OOP TaskHost to worker node.

Yield-for-callback pattern: BuildProjectFilesInParallel calls YieldForCallback() before sending the request, decrementing _activeTaskCount\ so the scheduler can reuse the same TaskHost process for nested builds (e.g., when a child project needs to run a task with the same runtime/architecture).

Handler stack (NodeProviderOutOfProcTaskHost): Replaced _nodeIdToPacketHandler\ with _nodeIdToPacketHandlerStack\ (Stack). When a task yields and a new task is scheduled on the same process, the new handler is pushed; when it completes, the handler is popped. Packet routing always delegates to _localPacketFactory\ to avoid recursive StackOverflow.

Concurrent task execution (OutOfProcTaskHostNode):

  • _activeTaskCount/_yieldedTaskCount\ replace _isTaskExecuting\ bool
  • \TaskExecutionContext\ per-task state (config, thread, saved environment, pending callbacks) in ConcurrentDictionary + AsyncLocal
  • \EffectiveConfiguration\ property resolves per-task config via AsyncLocal, preventing NRE when nested task's CompleteTask clears the global config
  • Save/restore operating environment (current directory, env vars) around yield

Nested Build Flow (TaskHost Process Reuse)

  1. OOP Task A calls BuildProjectFile → YieldForCallback (activeTaskCount→0) → sends request → blocks
  2. Worker: HandleBuildRequest calls engine3.BuildProjectFilesInParallel → scheduler evaluates child
  3. Scheduler finds child needs task with same TaskHostFactory runtime → AcquireAndSetUpHost → pushes handler B
  4. OOP: receives config (activeTaskCount==0) → spawns Task B thread → B executes → B completes
  5. Worker: DisconnectFromHost pops handler B → scheduler completes child → sends response
  6. OOP: Task A unblocks → ReacquireAfterCallback → continues

Tests

  • 29 callback tests pass (13 serialization + 16 integration across Stages 1-3)
  • E2E test verifying parent and child tasks report same PID (TaskHost process reused)

Context

JanProvaznik and others added 13 commits March 9, 2026 19:59
@JanProvaznik
Add TaskHostBuildRequest/Response packet pair for BuildProjectFile ca…
fccc27a 
…llbacks

Introduce two new IPC packets for forwarding BuildProjectFile* calls from
the OOP TaskHost to the worker node:

- TaskHostBuildRequest (0x20): Carries all 6-param canonical form args
  (projectFileNames, targetNames, globalProperties, removeGlobalProperties,
  toolsVersions, returnTargetOutputs)
- TaskHostBuildResponse (0x21): Carries bool success + target outputs using
  proven TaskParameter/TaskParameterTaskItem serialization

Includes TranslateNullableStringArray helper for string arrays with null
elements (e.g. toolsVersions), and IDictionary<->Dictionary<string,string>
conversion helpers matching TaskHostConfiguration precedent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Implement BuildProjectFile* callbacks in OutOfProcTaskHostNode
42cba20 
Replace all 4 BuildProjectFile* stubs (IBuildEngine 4-param, IBuildEngine2
5-param, IBuildEngine2 7-param, IBuildEngine3 6-param) with real callback
implementations that forward to the worker node.

All overloads normalize to the canonical 6-param form, send a
TaskHostBuildRequest via SendCallbackRequestAndWaitForResponse, and
unpack the TaskHostBuildResponse. Gated behind CallbacksSupported with
MSB5022 fallback for older worker nodes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Handle BuildProjectFile requests in TaskHostTask (worker node side)
561e914 
Add HandleBuildRequest to TaskHostTask that receives TaskHostBuildRequest
from the OOP TaskHost, forwards to IBuildEngine3.BuildProjectFilesInParallel,
and sends back a TaskHostBuildResponse.

Wraps entire handler in try/catch with ExceptionHandling.IsCriticalException
filter to always send a failure response on exception, preventing the OOP
task thread from blocking forever on TCS.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add BuildProjectFileTask test task and E2E test assets
29b3f59 
- BuildProjectFileTask: configurable test task calling BuildEngine.BuildProjectFile
  with ProjectPath, Targets, GlobalProperties, and BuildSucceeded output
- TestNetTaskBuildCallback: E2E .NET task project with ChildProject.proj
- Link BuildProjectFileTask into ExampleTask.csproj for E2E use

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add comprehensive tests for BuildProjectFile* callback forwarding
ac6f952 
Serialization tests (4):
- TaskHostBuildRequest round-trip with all fields
- TaskHostBuildRequest with null arrays (toolsVersions, globalProperties)
- TaskHostBuildResponse success with target outputs + ITaskItem metadata
- TaskHostBuildResponse failure with no outputs

Integration tests (6):
- BuildProjectFile with explicit TaskHostFactory
- BuildProjectFile with global properties forwarding
- BuildProjectFile with target output extraction
- BuildProjectFile child project failure returns false
- BuildProjectFile auto-ejected in multithreaded mode
- BuildProjectFile MSB5022 fallback when callbacks not supported

E2E test (1):
- Cross-runtime BuildProjectFile via .NET TaskHost

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add defensive null check for TargetOutputsPerProject in 7-param overload
b42f880 
Guard against null TargetOutputsPerProject in the result-copy loop.
BuildEngineResult defensively converts null to empty list, but the
explicit check makes intent clearer and prevents potential NRE if
that invariant ever changes.

Found via multi-model code review (Gemini).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Fix nullable warnings in TaskHostBuildRequest serialization test
1d52091 
Use properly typed nullable arrays and null-forgiving operators
to satisfy warnings-as-errors in full build.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add TaskHostYieldRequest/Response packets for Yield/Reacquire forwarding
623a7f8 
Add packet pair (0x26/0x27) to forward IBuildEngine3.Yield() and
Reacquire() calls from OOP TaskHost to the owning worker node.

TaskHostYieldRequest carries a YieldOperation enum (Yield or Reacquire).
TaskHostYieldResponse is a simple acknowledgment for Reacquire (Yield
is fire-and-forget).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Implement yield-for-callback pattern and Yield/Reacquire forwarding
456531c 
OOP TaskHost side (OutOfProcTaskHostNode):
- Add _yieldedTaskCount for tracking yielded state during callbacks
- YieldForCallback/ReacquireAfterCallback bracket BuildProjectFile calls
- Real Yield() sends TaskHostYieldRequest(Yield) — fire-and-forget
- Real Reacquire() sends TaskHostYieldRequest(Reacquire) — blocking
- HandleTaskHostConfiguration allows new config when task is yielded
  (_isTaskExecuting && _yieldedTaskCount > 0)

Worker side (TaskHostTask):
- HandleYieldRequest forwards Yield as fire-and-forget to engine3.Yield()
- HandleYieldRequest forwards Reacquire with blocking engine3.Reacquire()
  then sends TaskHostYieldResponse acknowledgment

Note: Same-process TaskHost reuse (child task running on the same OOP
process as the yielded parent) requires additional changes to
NodeProviderOutOfProcTaskHost connection management. Currently, nested
TaskHostFactory tasks spawn separate OOP processes because the parent
TaskHostTask holds its connection via a shared TaskHostNodeKey.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add yield packet serialization tests
3398df3 
Add Theory tests for TaskHostYieldRequest round-trip serialization
(both Yield and Reacquire operations) and a Fact test for
TaskHostYieldResponse round-trip serialization.

Use byte parameter with cast to avoid CS0051 (internal YieldOperation
enum used as public method parameter).

Also remove blank line before closing brace (SA1508) in
TaskHostCallback_Tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add TaskHost process reuse for nested BuildProjectFile callbacks
44f445a 
Implement handler stack pattern and concurrent task execution support
in the OOP TaskHost, enabling a yielded task to allow the scheduler to
reuse the same TaskHost process for nested builds.

Key changes:
- NodeProviderOutOfProcTaskHost: Replace _nodeIdToPacketHandler with
  _nodeIdToPacketHandlerStack (Stack<INodePacketHandler>) for push/pop
  of handlers when tasks yield and new tasks arrive on same process.
- OutOfProcTaskHostNode: Replace _isTaskExecuting with _activeTaskCount
  and _yieldedTaskCount for concurrent task tracking.
- TaskExecutionContext: Per-task state (config, environment, pending
  callbacks) stored in ConcurrentDictionary + AsyncLocal for thread
  isolation.
- EffectiveConfiguration property: Uses per-task context first, falling
  back to global _currentConfiguration. Fixes NRE when a nested task's
  CompleteTask clears _currentConfiguration while the yielded parent
  task still needs it for logging.
- YieldForCallback/ReacquireAfterCallback: Save/restore operating
  environment (current directory, env vars) around callbacks.
- E2E test verifying parent and child tasks report same PID when using
  TaskHostFactory with same Runtime.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Fix 6 concurrency issues found by multi-model review
cd69cb9 
Fixes identified by parallel review with Sonnet, Opus, Gemini, GPT:

1. _taskWrapper overwrite: Capture in local variable before ExecuteTask
   so nested Task B cannot overwrite it. Prevents leaked wrappers and
   double-cleanup.

2. _taskRunnerThread overwrite: HandleShutdown now joins all active
   task threads from _taskContexts (not just the last one), and fails
   their pending callbacks so they can unblock before WaitHandle
   disposal.

3. _taskCompletePacket single-slot race: Replace with ConcurrentQueue
   and drain in CompleteTask. Prevents lost completion packets when
   multiple tasks finish concurrently.

4. Non-atomic active/yielded transition: Reverse Interlocked order in
   YieldForCallback (increment yielded BEFORE decrementing active) and
   ReacquireAfterCallback (increment active BEFORE decrementing
   yielded). Ensures the sum is never zero during transition, preventing
   CompleteTask from prematurely nulling _currentConfiguration.

5. Shared warning fields overwrite: Save/restore WarningsAsErrors,
   WarningsNotAsErrors, WarningsAsMessages in TaskExecutionContext
   alongside environment variables.

6. Yield/Reacquire counter imbalance: RunTask finally block checks
   TaskExecutionState.Yielded to determine whether _activeTaskCount
   was already decremented by YieldForCallback, preventing counters
   from going negative if a task exits after Yield without Reacquire.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JanProvaznik
Add external repo validation script and fix nullable warning
62eef28 
- scripts/test-external-repos.ps1: Reusable script to build Roslyn and WPF
  projects with bootstrap MSBuild. Enables callbacks (MSBUILDENABLETASKHOSTCALLBACKS=1)
  and -mt flag. Supports -SkipClone, -CleanBuild, -Repos parameters.
- Fix CS8600 nullable warning in BuildProjectFileAndReportPidTask.cs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@JanProvaznik