Skip to content
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

Implement out-of-proc RAR node lifecycle #11383

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

Conversation

ccastanedaucf
Copy link
Contributor

Context

This implements the node lifecycle for an out-of-proc RAR node via an additional nodemode.

Changes Made

  • Added RarNodeLauncher to run a single instance of msbuild.exe /nodemode:3 and validate the node connection.
  • Added OutOfProcRarNode as the node implementation backed via named pipes. At the point, the server does not receive or process messages apart from the handshake to validate the connection. The handshake is based on the MSBuild Server handshake implementation, as it avoids baking the process information into the hash.
  • Refactored existing code for creating and connecting named pipe client / servers into CommunicationUtilities. This is needed since the RAR node implementation must live in Microsoft.Build.Tasks, and either does not have access to specific types, or runs into compile issues due to duplicated types from Microsoft.Build.Shared once referenced in MSBuild.

Testing

New code is largely IO / glue, so I haven't added new UTs. Let me know if anything sticks out to be tested (will likely need to dep inject in next PRs for testing anyways).

You can validate the server, handshake, and exclusivity are working by setting $env:MSBUILDOUTOFPROCRARNODE=1 and checking tracing logs + active processes.

Notes

  • Env var should be replaced by an actual MSBuild flag in the future, but holding off as the feature is still WIP
  • Due to potential type duplication, the RAR node implementation will need translation layers between inner types and Microsoft.Build types, and avoid explicitly using node interfaces. For consistency I'm matching function signatures / types as close as possible in case a future refactor can resolve this (see OutOfProcRarNode vs INodeEndpoint, RarNodeShutdownReasonvsNodeEngineShutdownReason`).
  • Connection timeouts are temporary, reused from other nodes for simplicity
  • Node exclusivity check currently only works on Windows (have not tested / researched other platforms). Currently using pipe name existence as it's less overhead than a mutex approach and the pipe only allows a single server instance. May need to revisit or use two pipes when multiple workers are implemented.
  • Need feedback for consistent naming scheme (e.g. is this the RAR out-of-proc service or RAR node? Should RAR be abbreviated everywhere in the node-related types?)

Next PRs will implement the client connection from the RAR task and serialization format.

@JanKrivanek
Copy link
Member

Would you be able to separate the pure code moves and actual code changes? It's fine to keep all in a single PR, but separate commits would be great.

It'll help speed up and focus the reviews.

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.

2 participants