Skip to content

Fix potential channel corruption after cancelled ReceiveMessageOrClosed()#40663

Merged
OneBlue merged 3 commits into
masterfrom
user/oneblue/channel-corrupt-fix
May 29, 2026
Merged

Fix potential channel corruption after cancelled ReceiveMessageOrClosed()#40663
OneBlue merged 3 commits into
masterfrom
user/oneblue/channel-corrupt-fix

Conversation

@OneBlue
Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue commented May 28, 2026

Summary of the Pull Request

This change fixes a potential channel corruption if a transaction is cancelled after its reply has been fully read.

This solves the issue by keeping track of the receives bytes at the channel level, and injecting them in the next receive. Stale messages will be discarded based on transaction ids.

This issue could manifest as the following error:

Microsoft.Windows.Wslc	Error	05-28-2026 10:05:41.134	"HRESULTString: 	E_UNEXPECTED
code: 	messageSize < sizeof(MESSAGE_HEADER)
failurecount: 	2
file: 	wsl\src\windows\common\HandleIO.cpp
function: 	wsl::windows::common::io::ReadSocketMessageHandle::ProcessRecvResult
hr: 	0x8000FFFF
linenumber: 	608
message: 	Unexpected message size: 1
threadid: 	63244
type: 	0"	wsl\src\windows\common\HandleIO.cpp		wsl::windows::common::io::ReadSocketMessageHandle::ProcessRecvResult	9812	63244	2		19a8ec70-4423-48d5-838d-c1b35aff6683		

Which causes the session termination logic to hang for up to a minute, since it fails to signal processes

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings May 28, 2026 21:11
Copy link
Copy Markdown
Contributor

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 addresses a Windows socket-channel edge case where cancelling ReceiveMessageOrClosed() after the reply bytes have already been read can leave the channel desynchronized/corrupted. It introduces channel-level tracking of already-received bytes so a subsequent receive can consume those bytes deterministically instead of re-reading from the socket stream.

Changes:

  • Extend ReadSocketMessageHandle to accept and persist “pending” already-received bytes across aborted receives, draining them before issuing a new WSARecv.
  • Add SocketChannel state (m_pendingBytes) to store pending bytes at the channel level and pass them into ReadSocketMessageHandle.
  • Add new unit-test scenarios validating delivery from PendingBytes (complete message, partial header, partial body, invalid header size).

Reviewed changes

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

File Description
test/windows/UnitTests.cpp Adds test coverage for PendingBytes-based message assembly and validation.
src/windows/common/HandleIO.h Updates ReadSocketMessageHandle API to accept a PendingBytes reference and adds ProcessChunk().
src/windows/common/HandleIO.cpp Implements pending-bytes draining, cancellation capture of already-buffered bytes, and chunk processing logic.
src/shared/inc/SocketChannel.h Adds per-channel m_pendingBytes and wires it into the Windows receive path.

Comment thread src/shared/inc/SocketChannel.h
Comment thread src/windows/common/HandleIO.cpp Outdated
Comment thread test/windows/UnitTests.cpp Outdated
@OneBlue OneBlue marked this pull request as ready for review May 28, 2026 23:30
@OneBlue OneBlue requested a review from a team as a code owner May 28, 2026 23:30
Copilot AI review requested due to automatic review settings May 28, 2026 23:30
Copy link
Copy Markdown
Contributor

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

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

Comment thread src/windows/common/HandleIO.cpp Outdated
Comment thread src/windows/common/HandleIO.cpp Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 28, 2026 23:46
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Member

@benhillis benhillis left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for adding the unit test.

@OneBlue OneBlue merged commit 33dcac0 into master May 29, 2026
12 checks passed
@OneBlue OneBlue deleted the user/oneblue/channel-corrupt-fix branch May 29, 2026 18:08
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.

3 participants