refactor(tcp-forward): 重构 TCP 转发代码,让转发工作更加稳健 - #3593
Merged
Merged
Conversation
This comment was marked as low quality.
This comment was marked as low quality.
Contributor
审查者指南该 PR 删除旧的 TcpForward 实现,改用基于 Builder/Config/Worker 的异步 TCP 转发架构,增加并发限制、缓冲区配置和生命周期管理,同时统一 socket 关闭逻辑并迁移 Lobby 相关调用方。 有界双向 TCP 转发的时序图sequenceDiagram
participant Client
participant Worker as TcpForwardWorker
participant Target as RemoteServer
Client->>Worker: AcceptAsync
Worker->>Worker: WaitAsync(0)
alt connection slot available
Worker->>Target: ConnectAsync
par client to target
Worker->>Target: ReceiveAsync / SendAsync
and target to client
Target-->>Worker: ReceiveAsync / SendAsync
end
Worker->>Worker: CancelAsync
Worker->>Client: CloseGracefully
Worker->>Target: CloseGracefully
else maximum connections reached
Worker->>Client: CloseGracefully
end
TCP 转发器生命周期与 Lobby 集成的时序图sequenceDiagram
participant LobbyController
participant NetworkHelper
participant Builder as TcpForwardBuilder
participant Worker as TcpForwardWorker
participant Broadcast as BroadcastLocal
LobbyController->>NetworkHelper: NewTcpPort()
NetworkHelper-->>LobbyController: available port
LobbyController->>Builder: BindLocal(port)
LobbyController->>Builder: SetRemote(IPAddress.Loopback, port)
LobbyController->>Builder: Build()
Builder-->>LobbyController: TcpForwardWorker
LobbyController->>Worker: Start()
LobbyController->>Broadcast: Start()
文件级变更
可能关联的问题
提示和命令与 Sourcery 交互
自定义使用体验访问你的控制面板即可:
获取帮助Original review guide in EnglishReviewer's Guide该 PR 删除旧的 TcpForward 实现,改用基于 Builder/Config/Worker 的异步 TCP 转发架构,增加并发限制、缓冲区配置和生命周期管理,同时统一 socket 关闭逻辑并迁移 Lobby 相关调用方。 Sequence diagram for bounded bidirectional TCP forwardingsequenceDiagram
participant Client
participant Worker as TcpForwardWorker
participant Target as RemoteServer
Client->>Worker: AcceptAsync
Worker->>Worker: WaitAsync(0)
alt connection slot available
Worker->>Target: ConnectAsync
par client to target
Worker->>Target: ReceiveAsync / SendAsync
and target to client
Target-->>Worker: ReceiveAsync / SendAsync
end
Worker->>Worker: CancelAsync
Worker->>Client: CloseGracefully
Worker->>Target: CloseGracefully
else maximum connections reached
Worker->>Client: CloseGracefully
end
Sequence diagram for TCP forwarder lifecycle and lobby integrationsequenceDiagram
participant LobbyController
participant NetworkHelper
participant Builder as TcpForwardBuilder
participant Worker as TcpForwardWorker
participant Broadcast as BroadcastLocal
LobbyController->>NetworkHelper: NewTcpPort()
NetworkHelper-->>LobbyController: available port
LobbyController->>Builder: BindLocal(port)
LobbyController->>Builder: SetRemote(IPAddress.Loopback, port)
LobbyController->>Builder: Build()
Builder-->>LobbyController: TcpForwardWorker
LobbyController->>Worker: Start()
LobbyController->>Broadcast: Start()
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI 代理的提示
请处理本次代码审查中的评论:
## 个别评论
### 评论 1
<location path="PCL.Core/Link/Lobby/LobbyController.cs" line_range="99" />
<code_context>
+ var tcpPortForForward = NetworkHelper.NewTcpPort();
+ McForward = new TcpForwardBuilder()
+ .BindLocal((ushort)tcpPortForForward)
+ .SetRemote(IPAddress.Loopback, (ushort)tcpPortForForward)
+ .Build();
McForward.Start();
</code_context>
<issue_to_address>
**issue (bug_risk):** 转发工作器被配置为连接到 `127.0.0.1:tcpPortForForward`,也就是工作器自身的监听端口,而不是 EasyTier 转发的 Minecraft 端口 `localPort`。因此,每个被接受的连接都会回环到转发监听器,直到达到连接数限制,而 Minecraft 流量永远无法到达目标。
**触发条件:** 启动大厅并且客户端通过公布的本地转发端口连接时。
**建议修复:** 将 `localPort` 传递给 `SetRemote`,同时仅将 `tcpPortForForward` 保留用于本地绑定和广播端口。
</issue_to_address>
### 评论 2
<location path="PCL.Core/Link/Lobby/LobbyInfoProvider.cs" line_range="21" />
<code_context>
public static int ProtocolVersion { get; set; } = 6;
public static BroadcastLocal? McBroadcast { get; internal set; }
- public static TcpForward? McForward { get; internal set; }
+ public static TcpForwardWorker? McForward { get; internal set; }
</code_context>
<issue_to_address>
**issue (bug_risk):** 将 `McForward` 更改为 `TcpForwardWorker` 会破坏 `PageToolsGameLink.xaml.cs` 中现有的调用方,因为该调用方访问 `LobbyInfoProvider.McForward.LocalPort`;`TcpForwardWorker` 提供了 `LocalEndPoint`,但没有 `LocalPort` 属性,因此项目将无法编译。
**建议修复:** 在 `TcpForwardWorker` 上公开兼容的 `LocalPort` 属性,或者更新调用方,使其使用 `LocalEndPoint?.Port` 并进行适当的空值处理。
</issue_to_address>
### 评论 3
<location path="PCL.Core/IO/Net/SocketForward/TcpForwardWorker.cs" line_range="177" />
<code_context>
+ var bytesRead = await source.ReceiveAsync(buffer, SocketFlags.None, cancellationToken).ConfigureAwait(false);
+ if (bytesRead == 0) break; // 连接已关闭
+
+ await destination.SendAsync(buffer[..bytesRead], SocketFlags.None, cancellationToken).ConfigureAwait(false);
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** 转发循环假设一次 `SendAsync` 调用就能发送接收到的整个缓冲区。套接字发送可能只完成部分操作,因此未发送的后缀会被丢弃,TCP 流会被无提示地截断或损坏。
**触发条件:** 目标套接字在一次发送操作中接收的字节数少于请求发送的字节数时。
**建议修复:** 循环发送,直到全部 `bytesRead` 个字节都已发送;每次根据 `SendAsync` 返回的字节数推进缓冲区切片。
</issue_to_address>帮助我变得更有用!请对每条评论点击 👍 或 👎,我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="PCL.Core/Link/Lobby/LobbyController.cs" line_range="99" />
<code_context>
+ var tcpPortForForward = NetworkHelper.NewTcpPort();
+ McForward = new TcpForwardBuilder()
+ .BindLocal((ushort)tcpPortForForward)
+ .SetRemote(IPAddress.Loopback, (ushort)tcpPortForForward)
+ .Build();
McForward.Start();
</code_context>
<issue_to_address>
**issue (bug_risk):** The forwarding worker is configured to connect to `127.0.0.1:tcpPortForForward`, which is the worker's own listening port, instead of the EasyTier-forwarded Minecraft port `localPort`. Every accepted connection therefore loops back into the forwarding listener until the connection limit is reached, and Minecraft traffic is never reached.
**Triggers:** When a lobby is launched and a client connects through the advertised local forwarding port.
**Suggested fix:** Pass `localPort` to `SetRemote` while retaining `tcpPortForForward` only as the local bind and broadcast port.
</issue_to_address>
### Comment 2
<location path="PCL.Core/Link/Lobby/LobbyInfoProvider.cs" line_range="21" />
<code_context>
public static int ProtocolVersion { get; set; } = 6;
public static BroadcastLocal? McBroadcast { get; internal set; }
- public static TcpForward? McForward { get; internal set; }
+ public static TcpForwardWorker? McForward { get; internal set; }
</code_context>
<issue_to_address>
**issue (bug_risk):** Changing `McForward` to `TcpForwardWorker` breaks the existing caller in `PageToolsGameLink.xaml.cs`, which accesses `LobbyInfoProvider.McForward.LocalPort`; `TcpForwardWorker` exposes `LocalEndPoint` but has no `LocalPort` property, so the project fails to compile.
**Suggested fix:** Expose a compatible `LocalPort` property on `TcpForwardWorker`, or update the caller to use `LocalEndPoint?.Port` with appropriate null handling.
</issue_to_address>
### Comment 3
<location path="PCL.Core/IO/Net/SocketForward/TcpForwardWorker.cs" line_range="177" />
<code_context>
+ var bytesRead = await source.ReceiveAsync(buffer, SocketFlags.None, cancellationToken).ConfigureAwait(false);
+ if (bytesRead == 0) break; // 连接已关闭
+
+ await destination.SendAsync(buffer[..bytesRead], SocketFlags.None, cancellationToken).ConfigureAwait(false);
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The forwarding loop assumes one `SendAsync` call transmits the entire received buffer. Socket sends are allowed to complete partially, so the unsent suffix is discarded and the TCP stream is silently truncated or corrupted.
**Triggers:** When the destination socket accepts fewer bytes than requested in a send operation.
**Suggested fix:** Loop until all `bytesRead` bytes have been sent, advancing the buffer slice by the number of bytes returned from each `SendAsync` call.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Pigeon0v0
approved these changes
Sep 3, 2026
Chiloven945
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sourcery 摘要
围绕可配置的工作器重构 TCP 转发,以改进连接管理、关闭可靠性和大厅端口转发集成。
新功能:
错误修复:
增强功能:
维护工作:
Original summary in English
Sourcery 总结
以可配置且具备生命周期管理能力的 TCP 转发工作器替换旧实现,提升连接管理、关闭可靠性及大厅端口转发集成。
新功能:
错误修复:
增强功能:
杂项:
Original summary in English
Sourcery 摘要
用可配置、受生命周期管理的工作线程替换旧版 TCP 转发实现,使转发和大厅集成更加可靠。
新功能:
错误修复:
增强功能:
日常维护:
Original summary in English
Summary by Sourcery
Replace the legacy TCP forwarding implementation with a configurable, lifecycle-managed worker to make forwarding and lobby integration more reliable.
New Features:
Bug Fixes:
Enhancements:
Chores:
Original summary in English