Skip to content

Conversation

@jlowin
Copy link
Owner

@jlowin jlowin commented May 31, 2025

Follow-up to #635 that fixes Client.close() to manage concurrency properly and also adds a test for Client concurrency

Copilot AI review requested due to automatic review settings May 31, 2025 22:03
@github-actions github-actions bot added tests client Related to the FastMCP client SDK or client-side functionality. labels May 31, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

The purpose of this PR is to ensure that the client context is cleaned up appropriately when close() is called, addressing issues identified in #635. Key changes include the addition of a new test for handling concurrent tasks in the proxy, refactoring of the client context management in client.py by introducing _connect and _disconnect methods, and the integration of new concurrency primitives.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/server/test_proxy.py Added a test to verify correct handling of concurrent tasks
src/fastmcp/client/client.py Refactored context management to improve session cleanup and prevent race conditions

@jlowin jlowin added the bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. label May 31, 2025
@jlowin jlowin changed the title Ensure close() cleans up client context appropriately Ensure Client.close() cleans up client context appropriately May 31, 2025
@jlowin jlowin merged commit 0882894 into main May 31, 2025
5 checks passed
@jlowin jlowin deleted the concurrency-2 branch May 31, 2025 22:15
jordicore pushed a commit to jordicore/fastmcp that referenced this pull request Jul 2, 2025
Ensure close() cleans up client context appropriately
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants