Skip to content

fix: Prevent MCPToolset GC during toolset add#2019

Merged
vblagoje merged 2 commits into
mainfrom
mcp_toolset_gc
Jul 2, 2025
Merged

fix: Prevent MCPToolset GC during toolset add#2019
vblagoje merged 2 commits into
mainfrom
mcp_toolset_gc

Conversation

@vblagoje
Copy link
Copy Markdown
Member

@vblagoje vblagoje commented Jun 30, 2025

Why:

Fixes a garbage collection issue that prevented users from properly combining multiple MCPToolset instances using the inline syntax tools.add(MCPToolset(...)). When MCPToolset objects were created inline and added to another toolset, Python's garbage collector would prematurely destroy the temporary toolset objects, closing their MCP connections and leaving orphaned tools that failed with "Not connected to an MCP server" errors.

What:

  • Useowner_toolset parameter to maintain strong references to prevent garbage collection

How can it be used:

# Previously broken - temporary MCPToolset would be garbage collected
server_info = SSEServerInfo(url="https://mcp.amap.com/sse?key=xxx")
htw_server_info = SSEServerInfo(url="http://127.0.0.1:8333/sse")
tools = MCPToolset(server_info=htw_server_info)
tools.add(MCPToolset(server_info=server_info))  # Now works correctly!

# Alternative approach that always worked
tools_a = MCPToolset(server_info=htw_server_info)
tools_b = MCPToolset(server_info=server_info)
all_tools = tools_a + tools_b  # Still works

How did you test it:

  • Manual confirmation of the original issue example
  • Using itinerary agent with the previously "broken" setup of MCPToolset usage
  • Ran full MCP integration test suite to ensure no regressions in existing functionality

Notes for the reviewer:

N/A

@github-actions github-actions Bot added integration:mcp type:documentation Improvements or additions to documentation labels Jun 30, 2025
@vblagoje vblagoje marked this pull request as ready for review June 30, 2025 07:43
@vblagoje vblagoje requested a review from a team as a code owner June 30, 2025 07:43
@vblagoje vblagoje requested review from Amnah199 and removed request for a team June 30, 2025 07:43
@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented Jul 1, 2025

Hey @Amnah199 do you have time to take a look at this one today?

@Amnah199
Copy link
Copy Markdown
Contributor

Amnah199 commented Jul 1, 2025

@vblagoje taking a look now 👍

Comment thread integrations/mcp/src/haystack_integrations/tools/mcp/mcp_toolset.py Outdated
Copy link
Copy Markdown
Contributor

@Amnah199 Amnah199 left a comment

Choose a reason for hiding this comment

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

LG already! Just a small comment.

@vblagoje vblagoje requested a review from Amnah199 July 1, 2025 12:22
@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented Jul 2, 2025

@Amnah199 can you please take another look?

Copy link
Copy Markdown
Contributor

@Amnah199 Amnah199 left a comment

Choose a reason for hiding this comment

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

Lets go!

@vblagoje vblagoje merged commit 630e524 into main Jul 2, 2025
10 checks passed
@vblagoje vblagoje deleted the mcp_toolset_gc branch July 2, 2025 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration:mcp type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add multi server support to MCPTool and MCPToolset.

2 participants