Skip to content

Add idle timeout termination for StreamableHTTPServerTransport #1159

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add idle timeout termination for StreamableHTTPServerTransport #1159

wants to merge 2 commits into from

Conversation

hopeful0
Copy link
Contributor

By adding an optional idle timeout termination to the StreamableHTTPServerTransport, this PR allows the stateful server to close long-idle sessions to avoid memory leak issues when the client does not terminate the session as expected

These changes comply with the specifications for Streamable HTTP transports session management in the MCP protocol.

Meanwhile, optimized the logic for transport termination and session management without affecting existing functionality, including:

  • Moved the transport termination after handle stateless requests to the finally block
  • Called the terminate function directly after the transport connect finished
  • Removed the condition of the transport is not terminated in the finally block of the run_server in handle_stateful_request, because that instances should be cleared even if transport terminates first
  • The manager returns 404 instead of 400 when it receives a non-existent session ID, because this might be due to the server actively terminating the transport

Motivation and Context

#1116 addressed the memory leak issue caused by session not terminating in stateless requests, but in stateful requests, if the client does not close the session as expected, the session on the server will never be closed, leading to a similar memory leak issue.

This PR adds an optional idle timeout parameter to StreamableHTTPServerTransport, a context manager for tracking idle status, and an _idle_timeout_terminate method. When the idle timeout parameter is set, an _idle_timeout_terminate task is initiated when transport connect is called. Once the transport does not receive or hanle any requests within the timeout period, the transport will be automatically terminated. To ensure that correctly tracks the idle status of transport, the handle_request method of the transport needs to be called within the context manager.

How Has This Been Tested?

I have added a test to verify that the transfer and session will exist before the idle timeout and will be terminated after the idle timeout.

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@hopeful0 hopeful0 force-pushed the idle-timeout-termination branch from ec3655c to df9d081 Compare July 16, 2025 16:24
@felixweinberger felixweinberger requested a review from a team as a code owner September 22, 2025 20:27
@felixweinberger felixweinberger added enhancement New feature or request needs more eyes Needs alignment among maintainers whether this is something we want to add needs motivation When a PR is submitted without clear intent or motivation for the changes. labels Sep 22, 2025
felixweinberger
felixweinberger reviewed Sep 22, 2025
View reviewed changes
Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

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

Hi @hopeful0 thank you for this contribution! And apologies for the time it took to get back to you on this.

For this change, I think we'd want more motivation that a timeout on the server is the right approach. The memory leak seems valid but there might be alternative ways to address it that don't rely on users to set the timeout (only users aware of and using this timeout param will actually benefit from the improvement.)

I'd like to bring this to the team and will come back with feedback asap.

Related (though I think between these 2 this is the right approach): #1343

@felixweinberger felixweinberger mentioned this pull request Sep 22, 2025
Open
@hopeful0
Copy link
Contributor Author

Thank you @felixweinberger for your attention to this PR.

I'd like to provide some additional clarification to further elaborate on the motivation behind this PR.

The core issue this PR aims to solve is the server's lack of a defensive mechanism to terminate sessions proactively (at least, this was the case when the PR was submitted). This leaves the server constantly exposed to the risk of memory leaks. The server is powerless against clients that fail to terminate sessions as expected, whether intentionally or unintentionally, in scenarios such as:

  • Network interruptions preventing the client from sending, or the server from receiving, a DELETE request.
  • The client crashing or being maliciously terminated before sending a DELETE request.
  • The client opting to start a new session after a network interruption or crash, rather than attempting to reconnect.

To address these issues, I believe that idle timeout termination is a time-tested and reasonable countermeasure (also common in protocols like HTTP), which has the following characteristics:

  • It complies with the third point of the session management section in the specification: The server MAY terminate the session at any time.
  • When combined with rate limiting, it can prevent server crashes caused by malicious requests from clients.
  • It prevents resource waste caused by clients not terminating sessions as expected.
  • Clients can implement a corresponding KEEP-ALIVE mechanism (e.g., sending ping requests at intervals shorter than the server's timeout) to prevent the server from unexpectedly terminating the session.

In summary, there is a strong motivation for adding such a defensive mechanism to the server. Idle timeout termination is a simple and elegant solution.

I hope these explanations address your concerns, and I'm more than happy to hear any other thoughts you or the team might have on this matter.

@felixweinberger felixweinberger added needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention and removed needs motivation When a PR is submitted without clear intent or motivation for the changes. needs more eyes Needs alignment among maintainers whether this is something we want to add labels Sep 30, 2025
@felixweinberger felixweinberger self-assigned this Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixweinberger felixweinberger felixweinberger left review comments

@ihrpr ihrpr Awaiting requested review from ihrpr ihrpr was automatically assigned from modelcontextprotocol/python-sdk

At least 1 approving review is required to merge this pull request.

Assignees

@felixweinberger felixweinberger

Labels
enhancement New feature or request needs maintainer action Potentially serious issue - needs proactive fix and maintainer attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hopeful0 @felixweinberger