Skip to content

fix(session): keep RPC routing alive across reconnects - #810

Merged
re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:pms396845/07-rpc-routing-v2
Oct 23, 2026
Merged

re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:pms396845/07-rpc-routing-v2

Conversation

@re2zero

@re2zero re2zero commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Part 7/8 of the reverse tunnel stack (recovery of #803).

This reapplies the reviewed commit on top of the corrected stack because #803 was accidentally merged into its stacked base branch instead of master.

Merge order: #805 → #806 → #807 → #808 → #809 → #810 → #811.
Retargeted to master after #809 merged.

The server kept one session per peer address. When a reconnect registered
a new session before the old one was torn down, the new entry was dropped
and the old session's disconnect then erased the address entirely, so
B -> A requests silently stopped being delivered.

Keep every session, route to the newest, and skip routing cleanup while
another session for the same peer remains. Also surface synchronous send
failures and briefly retry RPCs that no endpoint could accept.

Log: keep RPC routing alive across reconnects
PMS: TASK-396845
Influence: B -> A requests keep being delivered when a peer reconnects.

Summary by Sourcery

Keep peer-to-peer RPC delivery alive across reconnects by routing through replacement sessions and retrying temporarily unavailable endpoints.

Bug Fixes:

  • Preserve RPC routing when peers reconnect by retaining concurrent sessions, routing to the newest session, and avoiding cleanup while a replacement remains connected.
  • Retry RPC requests briefly when no endpoint can accept them and report synchronous send failures instead of silently dropping requests.

Enhancements:

  • Improve session availability checks and send failure reporting during connection and handshake transitions.

Tests:

  • Add coverage for reconnect routing continuity, real-session login state, and asynchronous request timeout behavior.

@sourcery-ai

sourcery-ai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens reverse-tunnel RPC routing across reconnects by retaining and selecting the newest per-peer session, preserving mappings during overlapping disconnects, surfacing synchronous send failures, and retrying briefly while connectivity stabilizes; integration tests cover the reconnect and timeout scenarios.

Sequence diagram for RPC retry during peer reconnect

sequenceDiagram
    participant Caller
    participant SessionManager
    participant SessionWorker
    participant ProtoEndpoint
    participant ProtoServer
    participant Peer

    Caller->>SessionManager: sendRpcRequest
    SessionManager->>SessionWorker: sendAsyncRequest
    SessionWorker->>ProtoEndpoint: asyncRequestWithHandler
    ProtoEndpoint->>ProtoServer: asyncRequest
    ProtoServer->>ProtoServer: onSend
    alt session unavailable or send fails
        ProtoServer-->>ProtoEndpoint: false
        ProtoEndpoint-->>SessionWorker: false
        SessionWorker-->>SessionManager: false
        SessionManager->>SessionManager: scheduleRpcRetry
        SessionManager->>SessionWorker: sendAsyncRequest
    else newest session accepts request
        ProtoServer->>Peer: SendAsync
        ProtoServer-->>ProtoEndpoint: true
        ProtoEndpoint-->>SessionWorker: true
    end
Loading

File-Level Changes

Change Details Files
Preserve reconnecting peer routes by retaining multiple sessions per address and routing through the newest live session.
  • Changed the session index to a multimap and select the newest session for direct and NAT-mapped targets.
  • Avoided disconnect cleanup and disconnected-state notifications when another session for the same address remains.
  • Added handshake/send readiness checks and propagated send failures instead of reporting undelivered frames as successful.
src/lib/common/session/protoserver.cpp
src/lib/common/session/protoserver.h
src/lib/common/session/protoclient.cpp
Make RPC delivery failures observable and retry requests while a peer is reconnecting.
  • Changed asynchronous endpoint sends to return success or failure, including synchronous transport failures.
  • Added bounded delayed retries from SessionManager when no endpoint accepts an RPC.
  • Added warning logs for failed sends and exhausted retries.
src/lib/common/manager/sessionmanager.cpp
src/lib/common/manager/sessionmanager.h
src/lib/common/manager/sessionworker.cpp
src/lib/common/session/protoendpoint.cpp
src/lib/common/session/protoendpoint.h
Replace fragile session-table test setup with integration coverage for reconnect routing and asynchronous timeout behavior.
  • Added a test that connects two clients from the same address, disconnects the older one, and verifies the newer route remains.
  • Updated server-login coverage to use a real handshaked connection.
  • Updated async request timeout coverage to exercise a connected server that withholds its response.
tests/manager/protoserver_session_test.cpp
tests/manager/sessionworker_test.cpp
tests/session_core/session_integration_test.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/lib/common/session/protoendpoint.h" line_range="31-32" />
<code_context>
-    // async call request and with 3s timeout result callback
-    void asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler);
+    // async call request and with 3s timeout result callback. Returns false
+    // when the request could not be handed to the session at all.
+    bool asyncRequestWithHandler(const std::string &target, const proto::OriginMessage &request, RpcHandler resultHandler);

     // Send a one-way frame to a specific peer. It does not create a request
</code_context>
<issue_to_address>
**Send failures are reported as accepted**

When an Android RPC cannot be handed to the session, `SessionWorker::doAsyncRequest` ignores the `asyncRequestWithHandler` result and returns `true`, so callers treat the unsent RPC as accepted and drop it without retry or failure handling.

Return the result of `asyncRequestWithHandler` from Android’s `SessionWorker::doAsyncRequest`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread src/lib/common/session/protoendpoint.h
lzwind
lzwind previously approved these changes Oct 22, 2026
@re2zero
re2zero force-pushed the pms396845/07-rpc-routing-v2 branch from 851527f to ae550e0 Compare October 22, 2026 07:54
@re2zero
re2zero force-pushed the pms396845/06-read-buffer-v2 branch from a827357 to 2c5e661 Compare October 22, 2026 07:55
@re2zero
re2zero force-pushed the pms396845/07-rpc-routing-v2 branch from ae550e0 to 38646d6 Compare October 22, 2026 08:18
@re2zero
re2zero force-pushed the pms396845/06-read-buffer-v2 branch 2 times, most recently from 3947d06 to c5ee806 Compare October 22, 2026 08:46
@re2zero
re2zero force-pushed the pms396845/07-rpc-routing-v2 branch from 38646d6 to 2753fb4 Compare October 22, 2026 08:46
@re2zero
re2zero force-pushed the pms396845/06-read-buffer-v2 branch from c5ee806 to 2178710 Compare October 22, 2026 08:58
@re2zero
re2zero force-pushed the pms396845/07-rpc-routing-v2 branch from 2753fb4 to eb2e0fb Compare October 22, 2026 08:58
The server kept one session per peer address. When a reconnect registered
a new session before the old one was torn down, the new entry was dropped
and the old session's disconnect then erased the address entirely, so
B -> A requests silently stopped being delivered.

Keep every session, route to the newest, and skip routing cleanup while
another session for the same peer remains. Also surface synchronous send
failures and briefly retry RPCs that no endpoint could accept.

Log: keep RPC routing alive across reconnects
PMS: TASK-396845
Influence: B -> A requests keep being delivered when a peer reconnects.
@re2zero
re2zero force-pushed the pms396845/07-rpc-routing-v2 branch from eb2e0fb to 54fc556 Compare October 22, 2026 14:02
@re2zero
re2zero changed the base branch from pms396845/06-read-buffer-v2 to master October 22, 2026 14:02
@re2zero
re2zero dismissed lzwind’s stale review October 22, 2026 14:02

The base branch was changed.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 96 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交修复了RPC路由在重连时丢失的问题,通过将session表改为multimap以支持同IP多session、增加重试机制、以及正确传播发送失败状态,整体逻辑清晰、实现合理。四维度均通过,未发现安全漏洞。代码质量良好,注释充分,测试覆盖到位。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['建议在asyncRequest的early future检查处补充更详细注释,说明FBE框架在onSend返回0时会让future立即就绪并携带异常,因此需要通过get()检测并返回false']


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['将waitFor辅助函数提取到tests/common/test_utils.h等共享头文件中', '将protoserver.cpp中的std::cout替换为DLOG/WLOG日志宏,与其他文件保持一致']


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['若未来需要根据不同网络环境调优重试参数,可考虑从配置文件或环境变量读取']


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: []


💡 改进建议代码示例

// 建议:将protoserver.cpp中的std::cout调试输出替换为日志框架
// 修改前:
//   std::cout << "hasConnected: " << ip << " true" << std::endl;
// 修改后:
//   DLOG << "hasConnected: " << ip << " true";

// 建议:将测试中的waitFor辅助函数提取到共享头文件
// tests/common/test_utils.h
// #pragma once
// #include <QElapsedTimer>
// #include <QTest>
// #include <functional>
// inline bool waitFor(const std::function<bool()> &condition, int timeoutMs = 3000)
// {
//     QElapsedTimer timer;
//     timer.start();
//     while (!condition() && timer.elapsed() < timeoutMs)
//         QTest::qWait(10);
//     return condition();
// }

本报告由 AI 代码审查工具自动生成

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, re2zero

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@re2zero
re2zero merged commit 343e115 into linuxdeepin:master Oct 23, 2026
26 of 28 checks passed
@re2zero
re2zero deleted the pms396845/07-rpc-routing-v2 branch October 23, 2026 02:04
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