Consolidate common IPC / named pipe code

[ccastanedaucf]

Motivation

The various node components contain a lot of duplicated code related to reading and writing packets, sequencing pipe operations, managing reusable buffers, handling broken connections, etc. along with differing implementations of performance optimizations and buffer reuse.

This PR aims to encapsulate all common logic into a simple API for reading and writing packets in a single call, along with extracting out the pipe connection and handshake logic for use outside of build nodes.

The primary motivation is to allow the RAR-out-of-proc client and server to share the same IPC code with MSBuild, as that must partially live in the Tasks assembly and cannot depend on much of the node provider/endpoint framework. Given that existing IPC logic already has test coverage and continued optimizations, it makes more sense to reuse as much as possible instead of adding (yet another) implementation of pipe communication, unit tests, ect.

Notes

Although I've kept behavior as close as possible, there's a few additional changes here to be noted:

• Many ifdefs were removed or combined. There is a lot of dead code which appears to be a holdover from Mono or unsupported targets, and can now be reduced to: (Framework, .NET Core, or TaskHost).
• TaskHost implementation calls now async-over-sync code. This should be safe because the TaskHost is not competing with build engine and VS thread resources as it executes in a separate process, and such should not be starved by a single blocking IO thread for reads. Writing a proper APM-compatible Begin/End API and custom IAsyncResult implementation adds a lot of complexity and more compile flags.
• BufferedReadStream was removed. as it performs the same function as the MemoryStream buffering. Likely more inefficiently as well due to the small buffer size (meaning more IO ops on the named pipe). I did not test against the most recent perf optimization here, but I would still expect this to be the case.
• INodePacketFactory.DeserializeAndRoutePacket() was split into two methods (which accounts for most of the file changes here). Motivation is so NodePipeBase can directly return an INodePacket instance but still allow the caller to decide whether to route it. This is not publicly exposed so it shouldn't be a breaking change.
• Exception handling boils down to: "If the pipe is broken but we expected it and aren't in the middle of a packet read, gracefully stop; otherwise, error." No need to condition on the number of read bytes on every single operation, which will can be logged from the exception anyways.
• Removed PipeOptions.WriteThrough. This flag has no effect on local IPC, which will always use kernel buffers.
• In the future, it may be worth doing a new perf analysis on buffer sizes and write limits per-operation. A named pipe shares a "quota" across all requests in a single direction, which defines the maximum number of bytes that can be pending for reads. This corresponds to the in/out buffer sizes set on server creation, which actually defines multiple kernel-level buffers. Exceeding this number will cause any additional writes to block (postpone if async) until the OS decides to allocate more or buffers are cleared. Out of the scope for this PR, but as a safe choice I've picked the implementation from node provider which already limits max write sizes per write operation.

Let me know if there's anything I need to split off to make this more digestible (e.g. INodePacketFactory plumbing, BufferedReadStream removal, some of the flag consolidation would be very quick), but this gives the full context.

[Copilot]

Pull Request Overview

This PR consolidates common IPC/named pipe functionality across multiple node components by refactoring packet deserialization methods, removing redundant code (including the BufferedReadStream), and updating the underlying pipe abstraction to use NodePipeClient/NodePipeServer.

• Refactored methods for deserializing packets (now DeserializePacket returning INodePacket) across components.
• Replaced direct stream operations with NodePipeClient/NodePipeServer and removed unused ifdef dead code.
• Removed BufferedReadStream to rely on MemoryStream buffering and simplified handshake and connection logic.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Build/BackEnd/Client/MSBuildClientPacketPump.cs	Updated to use NodePipeClient; refactored packet read loop to call asynchronous ReadPacketAsync.
src/Shared/NodePacketFactory.cs	Changed DeserializeAndRoutePacket to DeserializePacket and updated its usage.
src/Shared/NodeEndpointOutOfProcBase.cs	Updated to use NodePipeServer and simplified handshake/connection logic by replacing obsolete code.
src/Build/BackEnd/Client/MSBuildClient.cs	Refactored to use NodePipeClient for writing packets and connecting to server.
Other files	Method renames and refactoring for unified IPC API, along with removal of BufferedReadStream.

Comments suppressed due to low confidence (3)

src/Shared/NodeEndpointOutOfProcBase.cs:274

• [nitpick] The method WaitForConnection() returns a LinkStatus value but its name does not indicate that it determines the connection status. Consider renaming it or adding documentation to clarify its side effects and return value.

ChangeLinkStatus(localPipeServer.WaitForConnection());

src/Build/BackEnd/Client/MSBuildClient.cs:403

• Verify that the new NodePipeClient.WritePacket method correctly replicates the previous buffering and flushing behavior, ensuring no regression in packet transmission.

_pipeClient.WritePacket(packet);

src/Build/BackEnd/Node/InProcNode.cs:222

• After refactoring to return INodePacket via DeserializePacket, ensure that callers of this method are aware that a null return (as seen in the in-proc node implementation) is an acceptable outcome or handle it appropriately.

ErrorUtilities.ThrowInternalError("Unexpected call to DeserializeAndRoutePacket on the in-proc node.");

[rainersigwald]

Ideally we'd move the code shared between MSBuild and Microsoft.Build into Framework or another common location so it only has to be compiled/loaded from disk/jitted once. We can't always do that but for node comm stuff, we shouldn't ever be in the "one assembly reflection loaded but not hooked up to find the others" scenario.

But we can do that (much) later.

[ccastanedaucf]

Just ported the relevant changes over from #11448 (changing #if NETCOREAPP -> #if NET, returning ValueTask from inner pipe read loop)

Otherwise this is good to go 👍

[YuliiaKovalova]

@SimaTian , could you have a final look please?

[YuliiaKovalova]

@ccastanedaucf sorry, we have to revert this PR due to regression caused. See #11648 for more details.

[ccastanedaucf]

Shoot, I'll break it off into chunks then. Aka get shared code in to unblock RAR stuff -> incrementally remove dead code / flip references over in the nodes with some more testing.