8000 fix: mcp mem leak by akitaSummer · Pull Request #329 · eggjs/tegg · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: mcp mem leak #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 16, 2025
Merged

fix: mcp mem leak #329

merged 1 commit into from
Jun 16, 2025

Conversation

akitaSummer
Copy link
Contributor
@akitaSummer akitaSummer commented Jun 16, 2025
Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

Summary by CodeRabbit

  • Bug Fixes
    • Improved cleanup of closed connections to prevent stale session data.

Copy link
coderabbitai bot commented Jun 16, 2025

Walkthrough

A line was added to the sseCtxStorageRun method in the MCPControllerRegister class to remove the session ID from the sseConnections map when an SSE transport closes. This supplements the existing cleanup logic for closed transports.

Changes

File(s) Change Summary
plugin/controller/lib/impl/mcp/MCPControllerRegister.ts Added deletion of session ID from sseConnections in transport.onclose handler.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant MCPControllerRegister
    participant sseConnections

    Client->>MCPControllerRegister: Open SSE transport
    MCPControllerRegister->>sseConnections: Store session ID
    Client--xMCPControllerRegister: Close SSE transport
    MCPControllerRegister->>sseConnections: Delete session ID
    MCPControllerRegister->>transports/sseTransportsRequestMap: Cleanup
Loading

Possibly related PRs

  • fix: zod phantom dependency #322: Adds a static clean() method to MCPControllerRegister and updates lifecycle cleanup, related by focus on cleanup mechanisms in the same class.
  • fix: mcp context proto #318: Modifies the same onclose handler to remove the transport from other maps, directly related as both PRs adjust SSE transport cleanup logic in MCPControllerRegister.ts.

Suggested reviewers

  • killagu

Poem

A session departs, the transport is done,
Cleanup now finishes what it begun.
With a hop and a sweep, the map is made neat—
No session remains, no lingering seat.
The code is now tidy, as rabbits prefer,
Hopping through fields where bugs never occur! 🐇

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

plugin/controller/lib/impl/mcp/MCPControllerRegister.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-eggache".

(The package "eslint-plugin-eggache" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-eggache@latest --save-dev

The plugin "eslint-plugin-eggache" was referenced from the config file in ".eslintrc » eslint-config-egg/typescript » ./index.js".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
plugin/controller/lib/impl/mcp/MCPControllerRegister.ts (1)

382-387: Ensure immediate cleanup of the SSE keepalive interval on transport close

Deleting the sseConnections entry stops future heartbeats, but the interval timer continues until the next tick. To fully prevent resource leaks, clear the stored intervalId immediately in the onclose handler before removing the map entry.

Proposed diff:

 transport. => {
   closeFunc?.(...args);
   delete self.transports[transport.sessionId];
   self.sseTransportsRequestMap.delete(transport);
+  // clear the heartbeat interval immediately
+  const conn = self.sseConnections.get(transport.sessionId);
+  if (conn) clearInterval(conn.intervalId);
   self.sseConnections.delete(transport.sessionId);
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b6fbbd0 and 93e8a04.

📒 Files selected for processing (1)
  • plugin/controller/lib/impl/mcp/MCPControllerRegister.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
  • GitHub Check: Runner-macos (16)
  • GitHub Check: Runner-macos (18)
  • GitHub Check: Runner-ubuntu (20)
  • GitHub Check: Runner-ubuntu (22)
  • GitHub Check: Runner-ubuntu (18)
  • GitHub Check: Runner-ubuntu (16)
  • GitHub Check: Runner-macos (20)
  • GitHub Check: Analyze (typescript)
  • GitHub Check: Analyze (javascript)

@fengmk2 fengmk2 merged commit bddf8ed into eggjs:master Jun 16, 2025
13 checks passed
@fengmk2
Copy link
Member
fengmk2 commented Jun 17, 2025

@coderabbitai 创建 issues 跟进此修复会被合并到 next 分支

Copy link
coderabbitai bot commented Jun 17, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@fengmk2 我已经创建了一个 GitHub issue 来跟进此 MCP 内存泄漏修复合并到 next 分支的进度。这个 issue 将帮助跟踪修复的合并状态和验证内存泄漏问题是否已彻底解决。


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to track merging this memory leak fix to next branch
gh issue create \
  --title "跟进 MCP 内存泄漏修复合并到 next 分支" \
  --body "此 issue 用于跟进 PR #329 中的 MCP 内存泄漏修复合并到 next 分支的进度。

**相关 PR:**
- #329: fix: mcp mem leak

**修复内容:**
在 MCPControllerRegister 的 sseCtxStorageRun 方法中,当 SSE transport 关闭时,从 sseConnections map 中删除对应的 session ID,防止内存泄漏。

**待办事项:**
- [ ] 确认修复已合并到 next 分支
- [ ] 验证内存泄漏问题已解决

**相关链接:**
- PR: https://github.com/eggjs/tegg/pull/329
- 评论: https://github.com/eggjs/tegg/pull/329#issuecomment-$(gh pr view 329 --json comments --jq '.comments | length')" \
  --assignee fengmk2

Length of output: 562


@fengmk2 已成功创建 GitHub issue #330 来跟进此 MCP 内存泄漏修复合并到 next 分支的进度。

Issue 链接: #330

这个 issue 包含了:

  • 对当前 PR fix: mcp mem leak #329 修复内容的详细描述
  • 跟进合并到 next 分支的待办事项
  • 相关链接和引用

您可以通过这个 issue 来跟踪修复的合并状态和验证内存泄漏问题的解决情况。

@coderabbitai coderabbitai bot mentioned this pull request Jun 18, 2025
4 tasks
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
0