Skip to content

Support full duplex traffic and half close socket in NAT localhost relay - #41458

Open
Feng Wang (chemwolf6922) wants to merge 3 commits into
masterfrom
user/chemwolf6922/support-full-duplex-in-NAT-localhost-relay
Open

Support full duplex traffic and half close socket in NAT localhost relay#41458
Feng Wang (chemwolf6922) wants to merge 3 commits into
masterfrom
user/chemwolf6922/support-full-duplex-in-NAT-localhost-relay

Conversation

@chemwolf6922

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

This is a continuation of wangxin12's work in #40171.

The original localhost relay, on both sides, would:

  1. Block the other direction's traffic if one direction's write would block.
  2. Close both sides if one side's read is closed. Which causes problems in half close socket scenarios.

This PR makes these changes to support full duplex traffic and half close socket in NAT localhost relay:

  1. Refactor the windows bi-directional relay with MultiHandleWait with a new half close capable relay handle.
  2. Refactor the Linux relay with a fully event driven non-blocking epoll loop. Which also avoids creating a new thread for each connection.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Add tests:
NetworkTests::NetworkTests::NatLocalhostRelayHalfClose
NetworkTests::NetworkTests::NatLocalhostRelayFullDuplex
UnitTests::UnitTests::BidirectionalRelayContinuesAfterHalfClose

@chemwolf6922
Feng Wang (chemwolf6922) requested a review from a team as a code owner August 27, 2026 03:12
Copilot AI lite review requested due to automatic review settings August 27, 2026 03:12

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.

Pull request overview

This PR improves the NAT localhost relay implementation to correctly support full-duplex traffic and TCP half-close semantics, addressing hangs and premature connection teardown in bidirectional protocols.

Changes:

  • Refactors the Windows bidirectional relay to use MultiHandleWait, introducing HalfCloseRelayHandle to propagate half-close (shutdown(SD_SEND)) without terminating the opposite direction.
  • Rewrites the Linux localhost relay into a non-blocking, event-driven epoll loop to support full duplex and avoid per-connection threads.
  • Adds Windows unit/integration tests covering half-close and full-duplex behavior through the localhost relay.

Reviewed changes

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

Show a summary per file
File Description
test/windows/UnitTests.cpp Adds a unit test validating relay continues after half-close.
test/windows/NetworkTests.cpp Adds WSL2 NAT localhost relay tests for half-close and full-duplex behavior; extends socat helper.
src/windows/common/relay.cpp Replaces legacy WaitForMultipleObjects relay loop with MultiHandleWait + relay handle abstractions.
src/windows/common/HandleIO.h Extends ReadHandle to accept a buffer size; adds HalfCloseRelayHandle declaration.
src/windows/common/HandleIO.cpp Implements buffer-sized ReadHandle construction and HalfCloseRelayHandle shutdown behavior.
src/linux/init/localhost.cpp Replaces blocking/threaded relay with per-connection state machine driven by epoll on non-blocking sockets.

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

Comment thread src/linux/init/localhost.cpp 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.

Pull request overview

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

Copilot AI review requested due to automatic review settings August 27, 2026 04:18

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.

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/linux/init/localhost.cpp:521

  • connect() can return EINTR when a signal interrupts the attempt; for this case Linux leaves the connection attempt in progress, but this branch currently tears down the relay connection. The previous blocking implementation retried interrupted connects, and the new accept/read/write paths also handle interruptions. Treat EINTR like EINPROGRESS and let epoll complete the connection instead.
        const int result = connect(m_tcpSocket.get(), socketAddress, socketAddressSize);
        if (result < 0 && errno != EINPROGRESS)

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