Partial Stream Read Fix and Floating Node Fix

[sec]

Fixes #12944

cc @Thefrank @arrowd @am11

Confirmed the fix by doing full VMR builds under FreeBSD 14 and 15 (soon arm64 should also complete).

1. Floating Node Fix (Shutdown Logic)

Context: When a build fails or aborts and MSBuild is tearing down, child nodes were sometimes left floating as zombie processes if the build was initially started with /nodeReuse:true.

Fix: Modified BuildManager.HandleNodeShutdown to explicitly pass enableNodeReuse: false when aborting the build (e.g., when the shutdownPacket indicates ConnectionFailed). This guarantees that nodes are correctly forcefully shut down upon an abnormal termination instead of lingering in the background waiting for a new connection.

2. Partial Stream Read Fix (MSB4166 crash on FreeBSD 15)

Context: When building the dotnet VMR on FreeBSD 15, MSBuild frequently crashes with MSB4166: Child node "N" exited prematurely.
Root Cause: MSBuild had a long-standing assumption in its IPC code where it assumed that a single Stream.ReadAsync or Stream.EndRead call on a NamedPipe/Unix Domain Socket would always return the exact requested number of bytes (in this case, the 5-byte MSBuild packet header). While this generally holds true for Windows Named Pipes operating in message mode, Unix Domain Sockets on Linux/FreeBSD are purely stream-oriented. The FreeBSD 15 kernel occasionally fragments the UDS packet delivery (e.g., delivering 2 bytes, then 3 bytes). When MSBuild received only 2 bytes from ReadAsync, it immediately failed its bytesRead == Expected check and threw a ConnectionFailed exception, killing the child process that was otherwise completely healthy.

Fix: Rewrote the IPC stream reading loops on both the parent side (NodeProviderOutOfProcBase.cs) and the child side (NodeEndpointOutOfProcBase.cs). The logic now correctly utilizes while loops that accumulate stream fragments synchronously and asynchronously until the full 5-byte header or packet body is received, fully mitigating fragmentation on Unix Domain Sockets.

[Copilot]

Pull request overview

This PR hardens MSBuild’s out-of-proc node IPC and shutdown behavior to address MSB4166/ConnectionFailed scenarios on stream-based transports (notably Unix domain sockets on FreeBSD) and to avoid leaving reusable nodes running after abnormal build termination.

Changes:

• Update parent-side packet reading in NodeProviderOutOfProcBase to reliably accumulate fragmented reads for both the 5-byte header and the packet body.
• Update child-side header reads in NodeEndpointOutOfProcBase to tolerate fragmented delivery of the 5-byte header.
• Adjust BuildManager.HandleNodeShutdown abort shutdown logic to ensure connected nodes are shut down without reuse after abnormal termination.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/Shared/NodeEndpointOutOfProcBase.cs	Accumulates partial reads to reliably read the 5-byte packet header on stream-based IPC.
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs	Accumulates partial reads for packet headers and bodies to prevent false ConnectionFailed failures on fragmented reads.
src/Build/BackEnd/BuildManager/BuildManager.cs	Forces non-reuse shutdown of connected nodes on abnormal node shutdown to avoid lingering reusable nodes.

[am11]

When a build fails or aborts and MSBuild is tearing down

I'm a bit confused by the description, is it:

1. build was failing due to legitimate error on FreeBSD 15 but this broken pipe thing is overshaddowing the real error. after this fix, it unconvered the real error which will be fixed separately.

2. OR this is because build was failing and fixing it succeeds the build

Either ways, I think the better fix would be:

#if NET
// use https://learn.microsoft.com/dotnet/api/system.io.stream.readexactly
// and/or https://learn.microsoft.com/dotnet/api/system.io.stream.readexactlyasync
#else
// existing code here
#endif

without forcing localReadPipe over readTask in newer .NET.

[sec]

When a build fails or aborts and MSBuild is tearing down

I'm a bit confused by the description, is it:

1. build was failing due to legitimate error on FreeBSD 15 but this broken pipe thing is overshaddowing the real error. after this fix, it unconvered the real error which will be fixed separately.
2. OR this is because build was failing and fixing it succeeds the build

Either ways, I think the better fix would be:

#if NET
// use https://learn.microsoft.com/dotnet/api/system.io.stream.readexactly
// and/or https://learn.microsoft.com/dotnet/api/system.io.stream.readexactlyasync
#else
// existing code here
#endif

without forcing localReadPipe over readTask in newer .NET.

This PR fix two separate issues:

• zombie processes left after msbuild crashed during builds (that was one line change, so I included this)
• second was msbuild crashing becuase of wrong packet reading from unix domain sockets - FreeBSD 15 recent Local (UNIX domain) sockets were refactored in 15.0, resulting in increased throughput and reduced latency for local stream sockets. - which caused the bug to surface

zombie processes were there before the crash, but it was quite "troublesome" to handle them always and the fix was easy (as they were left there for 15min after each failed build for ex.).

As for using readexactly - using loop was straightforward fix regardless of .net version running - but if you think using it will bring more benefits maybe it's worth to take a look at - for me the code align with the current style in there :)

[am11]

As for using readexactly - using loop was straightforward fix regardless of .net version running - but if you think using it will bring more benefits maybe it's worth to take a look at - for me the code align with the current style in there :)

It just expands the matrix for maintainers to review, there is a huge legacy code depending on the existing behavior. Therefore, it make sense to keep the fix as targeted as possible. Also, it's not exactly 'correct'; on .NET Core, the code in main branch prefers readTask, while your change has switched both branches to localReadPipe. I would structure it so the fix is applied under #if NET branch with ReadExactly API, create a test-only PR in dotnet/dotnet repo to get larger CI validation, once CI is green, close that PR.

[sec]

As for using readexactly - using loop was straightforward fix regardless of .net version running - but if you think using it will bring more benefits maybe it's worth to take a look at - for me the code align with the current style in there :)

It just expands the matrix for maintainers to review, there is a huge legacy code depending on the existing behavior. Therefore, it make sense to keep the fix as targeted as possible. Also, it's not exactly 'correct'; on .NET Core, the code in main branch prefers readTask, while your change has switched both branches to localReadPipe. I would structure it so the fix is applied under #if NET branch with ReadExactly API, create a test-only PR in dotnet/dotnet repo to get larger CI validation, once CI is green, close that PR.

can someone more familiar with msbuild maybe help on that? for me those places looked the the fail point, becuase they were reading for ex. 2 bytes and expecting 5 - which was causing nodes to crash, as internals on FreeBSD 15 got changed, but the work in correct way, if the stream have more data, calling one read shouldn't not assume all data is returned.

[am11]

FreeBSD 15 is a community platform released few months ago, so selling a "fix" on a 23y old product for a non-official platform raises questions. I was trying to help you since you've pinged me on the issue. Good luck!

[arrowd]

#12944 is not FreeBSD-specific, it is just much easier to bump into this issue on FreeBSD.

[am11]

#12944 is not FreeBSD-specific, it is just much easier to bump into this issue on FreeBSD.

That doesn't say anything about .NET Framework / Windows. My suggestion above isn't #if FREEBSD but rather #if NET, which is for .NET Core and new API is available on the version of .NET Core this project is targeting so it's safe and targeted.