[KVCache] Fix RDMA connection memory leak#8025
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8025 +/- ##
==========================================
Coverage ? 67.67%
==========================================
Files ? 470
Lines ? 66111
Branches ? 10187
==========================================
Hits ? 44740
Misses ? 18520
Partials ? 2851
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment was marked as off-topic.
This comment was marked as off-topic.
cmcamdy
changed the title
There was a problem hiding this comment.
Pull request overview
该 PR 聚焦在 KVCache RDMA 通信链路的资源清理与失败路径处理，目标是降低连接失败/断开/销毁过程中 RDMA 资源（QP/CQ/channel/socket/MR/read buffer 等）的泄漏风险，并改进多连接场景下的 MR 复用逻辑。
Changes:
- 在 QP 建链阶段以本地/远端 MTU 的较小值作为 path MTU，并增加合法性校验。
- 增强多处失败路径与析构路径的清理逻辑（包括 read buffer/MR、socket、QP/CQ/channel 等），并补齐 MR exchange 失败处理。
- 服务端（decode）侧 MR 支持跨连接复用，并补充 key/value scale MR 的清理。
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| fastdeploy/cache_manager/transfer_factory/kvcache_transfer/src/kvcache_rdma.cpp | 调整 server/client 线程启动方式、增强连接失败/销毁路径资源回收、服务端 MR 复用与清理逻辑 |
| fastdeploy/cache_manager/transfer_factory/kvcache_transfer/src/kvcache_connection.cpp | MTU 协商修正、QP 创建/销毁清理增强、epoll 配置与 MR 发送/接收失败清理补强 |
| fastdeploy/cache_manager/transfer_factory/kvcache_transfer/include/kvcache_rdma.h | 引入原子状态与线程句柄字段，支持新的生命周期/清理逻辑 |
| server_thread_ = std::thread([this]() { | ||
| try { | ||
| this->init_server(); | ||
| } catch (const std::exception& e) { | ||
| ERR("Server thread failed: %s", e.what()); | ||
| } | ||
| }); | ||
| server_thread.detach(); | ||
| server_thread_.detach(); | ||
| } |
| if (!start_client_listener.load()) { | ||
| std::lock_guard<std::mutex> lock(mutex_); | ||
| if (!start_client_listener.load()) { | ||
| client_thread_ = std::thread([this]() { this->client_listener(); }); | ||
| client_thread_.detach(); | ||
| start_client_listener = true; | ||
| } |
| for (size_t i = 0; i < ctx->conn.read_bufs.size(); ++i) { | ||
| if (ctx->conn.read_mrs[i]) ibv_dereg_mr(ctx->conn.read_mrs[i]); | ||
| if (ctx->conn.read_bufs[i]) free(ctx->conn.read_bufs[i]); | ||
| } | ||
| if (ctx->sock_fd > 0) close(ctx->sock_fd); |
| if (epoll_ctl(epollfd, EPOLL_CTL_ADD, sockfd, &ev) == -1) { | ||
| ERR("Failed to add listening socket to epoll"); | ||
| close(epollfd); | ||
| close(sockfd); | ||
| return -1; |
| struct RdmaContext *ctx = new RdmaContext(); | ||
| memset(ctx, 0, sizeof(struct RdmaContext)); | ||
| struct ibv_qp_init_attr qpInitAttr = {}; | ||
| ctx->context = ib_dev->context; |
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-06-10 14:16:34
📋 Review 摘要
PR 概述：修复 RDMA KV cache 连接失败、断开和析构路径中的资源清理。
变更范围：kvcache_transfer RDMA QP/MR/socket/listener 生命周期。
影响面 Tag：[KVCache] [PD Disaggregation]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🔴 Bug | fastdeploy/cache_manager/transfer_factory/kvcache_transfer/src/kvcache_rdma.cpp:131 |
server 线程启动早于运行状态置位，可能在构造期直接退出 |
| 🔴 Bug | fastdeploy/cache_manager/transfer_factory/kvcache_transfer/src/kvcache_rdma.cpp:263 |
listener 仍被 detach，析构释放 FD/MR/QP 前没有唤醒并 join，存在 use-after-free/并发注销 |
历史 Findings 修复情况
| Finding | 问题 | 状态 |
|---|---|---|
| F1 | server MR 生命周期与析构/线程退出路径没有同步闭环 |
📝 PR 规范检查
标题已符合规范；PR 描述仍保留模板占位内容，建议替换为：
PR 描述建议（点击展开，可直接复制）
## Motivation
Fix RDMA KV cache connection cleanup to reduce resource leaks in connection failure, disconnect, and communicator destruction paths.
## Modifications
- Negotiate QP path MTU with the smaller local/remote MTU.
- Add cleanup for QP/CQ/channel/socket/read-buffer resources in RDMA connection failure and destruction paths.
- Reuse server-side KV cache MRs across multiple connections and add cleanup for key/value scale MRs.
- Handle `client_exchange_mr` and `server_exchange_mr` failures instead of continuing with incomplete MR exchange.
## Usage or Command
N/A
## Accuracy Tests
N/A. This PR changes RDMA resource cleanup and does not change model numerical computation.
## Checklist
- [x] Add at least a tag in the PR title.
- Tag list: [`[FDConfig]`,`[APIServer]`,`[Engine]`, `[Scheduler]`, `[PD Disaggregation]`, `[Executor]`, `[Graph Optimization]`, `[Speculative Decoding]`, `[RL]`, `[Models]`, `[Quantization]`, `[Loader]`, `[OP]`, `[KVCache]`, `[DataProcessor]`, `[BugFix]`, `[Docs]`, `[CI]`, `[Optimization]`, `[Feature]`, `[Benchmark]`, `[Others]`, `[XPU]`, `[HPU]`, `[GCU]`, `[DCU]`, `[Iluvatar]`, `[Metax]`]
- You can add new tags based on the PR content, but the semantics must be clear.
- [ ] Format your code, run `pre-commit` before commit.
- [ ] Add unit tests. Please write the reason in this PR if no unit tests.
- [x] Provide accuracy results.
- [ ] If the current PR is submitting to the `release` branch, make sure the PR has been submitted to the `develop` branch, then cherry-pick it to the `release` branch with the `[Cherry-Pick]` PR tag.总体评价
资源清理方向是对的，但后台 listener 生命周期仍没有和析构同步；当前实现仍可能在 RDMA 通信关闭或对象释放时触发 use-after-free 或破坏 MR ownership，需要先修复后再合入。
| // Start the server thread (if in decode role) | ||
| if (splitwise_role == "decode") { | ||
| std::thread server_thread([this]() { | ||
| server_thread_ = std::thread([this]() { |
There was a problem hiding this comment.
🔴 Bug server listener 可能在构造期直接退出。
构造函数把 RDMACommunicator_status 初始化为 0，这里先启动 server 线程，之后才在第 140 行置为 1；如果新线程先跑到 start_server()，while (RDMACommunicator_status == 1) 会立即为 false，server socket 清理后退出，decode 端就不会再接受 RDMA 连接。
建议修复方式：在启动 server 线程前先把状态置为 running，或使用明确的启动同步原语保证 start_server() 进入 loop 前状态已就绪。
| server_mr_owned_by_destructor_ = true; | ||
|
|
||
| // Detached threads will notice status change on next epoll wake-up; | ||
| // no need to join — kernel reclaims resources on process exit. |
There was a problem hiding this comment.
🔴 Bug 析构仍然没有等待 listener 线程退出，就开始释放这些线程还会访问的资源。
start_server() 和 client_listener() 都可能阻塞在 epoll_wait(..., -1)；析构只写 RDMACommunicator_status = 0，不会唤醒 epoll，而且线程已 detach()，无法 join()。随后析构会释放 conn_map 里的 RdmaContext、关闭 fd、注销 MR/QP/PD；下一次 socket 事件到来时后台线程仍会通过 this 访问 mutex_、MR 列表或已删除的 ctx，触发 use-after-free 或并发 ibv_dereg_mr。这也没有闭环修复此前的 server MR 生命周期问题。
建议修复方式：不要 detach 这两个线程；析构先关闭/唤醒 server listen fd 和 client epoll fd，使 loop 退出，然后在释放 conn_map、MR、QP、PD 前 join() 线程，并让 server MR 只在同一个受同步保护的 owner 路径中注销。
* fix rdma connect mem leak * fix rdma connect mem leak * fix rdma connect mem leak * fix rdma free * fix rdma free
|
Thanks for your contribution! |
Motivation
Fix RDMA KV cache connection cleanup to reduce resource leaks in connection failure, disconnect, and communicator destruction paths.
Modifications
client_exchange_mrandserver_exchange_mrfailures instead of continuing with incomplete MR exchange.Usage or Command
N/A
Accuracy Tests
N/A. This PR changes RDMA resource cleanup and does not change model numerical computation.
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.