Skip to content

Fix partial reads error when reading packet - #14997

Open
sec wants to merge 3 commits into
dotnet:mainfrom
sec:bug/fix-read-packet-header
Open

sec wants to merge 3 commits into
dotnet:mainfrom
sec:bug/fix-read-packet-header

Conversation

@sec

@sec sec commented Sep 10, 2026

Copy link
Copy Markdown

This should fix #12944 - at least under my testing under FreeBSD 15 it enabled to complete the dotnet build. Recent builds under 15 are failing due to IPC communication errors, which this PR address.

The change followed the code pattern used in other places, which handle packet read in proper way. It was propoed to use ReadExactly, but I didn't saw any async versions/more usage in the code base, so simple loop was used.

sec added 2 commits September 10, 2026 14:08
# Conflicts:
#	src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs
Copilot AI lite review requested due to automatic review settings September 10, 2026 15:02
@sec
sec temporarily deployed to copilot-pat-pool September 10, 2026 15:02 — with GitHub Actions Inactive
@sec
sec temporarily deployed to copilot-pat-pool September 10, 2026 15:02 — with GitHub Actions Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The updated header-read loop reads from _pipeStream while the body reads from _readStream (possibly buffered), which can desynchronize the stream and corrupt packet framing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR aims to improve robustness of out-of-proc node IPC on Unix-like platforms (notably FreeBSD) by ensuring packet headers are fully read before processing, avoiding failures caused by partial reads.

Changes:

  • Update the async packet read loop to read the full packet header in a loop rather than assuming a single ReadAsync returns all bytes.
File summaries
File Description
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs Makes packet header reads resilient to partial ReadAsync results in the out-of-proc node communications loop.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Add regression coverage for partial-header reads and EOF handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +1121 to +1123
while (totalBytesRead < _headerByte.Length)
{
int bytesRead = await _readStream.ReadAsync(_headerByte.AsMemory(totalBytesRead, _headerByte.Length - totalBytesRead), CancellationToken.None).ConfigureAwait(false);
@AR-May AR-May self-assigned this Sep 15, 2026
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.

error MSB4166: Child node "3" exited prematurely

3 participants