Skip to content

Sidecar taskhost #12071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Sidecar taskhost #12071

wants to merge 21 commits into from

Conversation

SimaTian
Copy link
Member

Fixes #11335

Context

We already had TaskHost live alongside the build. It would get cleaned up when the build is done.
This is some additional plumbing to allow for TaskHost to live even after the build is done.
Now we have two cases:

  • when TaskHost is requested via TaskHostFactory, we assume that it is required for it to release the .dlls after the build is done and so we use the old build-lived TaskHost
  • when TaskHost is requested via MSBUILDFORCEALLTASKSOUTOFPROC (or any other way, something we need to keep in mind going forward), it will spawn a long-lived taskHost node that will live until it timeouts or is killed.

before merging this one, we need to merge Yuliias one first (since there is overlap, I based it on hers instead of on main)
#11393

Changes Made

Added plumbing to allow for long lived TaskHost spawning.

Testing

TODO

Notes

  • for review, list only the last X commits that are from me please. Rest should go away when Yuliia's PR is merged.
  • Naming in Taskhost is confusing due to the way it references explicit factory using. If anyone has a good idea how to rename things to consolidate them, please let me know.

@Copilot Copilot AI review requested due to automatic review settings June 25, 2025 15:41
Copy link
Contributor

@Copilot Copilot AI left a comment

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 adds plumbing to support long‐lived TaskHost processes by distinguishing between build‐lived and long‐lived task hosts. The changes refactor handshake serialization to use a structured HandshakeComponents type, update constants and environment retrieval for MSBuild paths, and adjust logging and process‐launch logic accordingly.

  • Introduce the _isNetRuntime flag in TaskHostConfiguration and update its constructor API.
  • Replace raw int arrays with a structured HandshakeComponents type and update related handshake loops.
  • Update constants, environment variable use, and process name references across the codebase for improved consistency.

Reviewed Changes

Copilot reviewed 20 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/UnitTests.Shared/RunnerUtilities.cs Use Constants.MSBuildAssemblyName for MSBuild assembly lookup.
src/Shared/TaskHostConfiguration.cs Introduce _isNetRuntime field and update constructor comments and parameters.
src/Shared/NodePipeServer.cs Switch from a for loop to a foreach loop (with manual index) when processing handshake components.
src/Shared/NodePipeClient.cs Update handshake serialization to use component enumeration.
src/Shared/NodePipeBase.cs Change HandshakeComponents property type from int[] to a structured type.
src/Shared/NodeEndpointOutOfProcBase.cs Add IsHandshakePartValid and related handshake improvements.
src/Shared/CommunicationsUtilities.cs Refactor handshake construction and update constants handling.
src/Shared/BuildEnvironmentHelper.cs Add NET-specific directory properties for tools and assemblies.
src/MSBuild/MSBuild/Microsoft.Build.Core.xsd Update runtime attribute documentation and enumeration to include NET.
src/Build/Resources/Constants.cs Introduce new constants for dotnet process names and MSBuild executable/assembly names.
src/Build/Instance/TaskFactories/TaskHostTask.cs Update TaskHost spawning logic and error logging naming.
src/Build/Evaluation/IntrinsicFunctions.cs Improve dictionary initialization and task host existence check.
src/Build/BackEnd/** Various updates addressing node launching, handshake logic, and process name consistency.
.editorconfig Minor header/BOM change.
Comments suppressed due to low confidence (2)

src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs:340

  • The typo 'realTaskAssemblyLoaction' has been corrected to 'realTaskAssemblyLocation' for improved clarity.
            string realTaskAssemblyLocation = TaskInstance.GetType().Assembly.Location;

.editorconfig:1

  • [nitpick] Ensure that the file encoding (with BOM) is intentional and consistent with the project’s standards.
# editorconfig.org

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add long-running "sidecar" TaskHosts
2 participants