Skip to content

feat: add idle timeout for StreamableHTTP sessions#1994

Open
felixweinberger wants to merge 1 commit intov1.xfrom
fweinberger/idle-timeout-termination
Open

feat: add idle timeout for StreamableHTTP sessions#1994
felixweinberger wants to merge 1 commit intov1.xfrom
fweinberger/idle-timeout-termination

Conversation

@felixweinberger
Copy link
Contributor

Summary

Adds a session_idle_timeout parameter to StreamableHTTPSessionManager that enables automatic cleanup of idle sessions, fixing the memory leak described in #1283.

Motivation and Context

Sessions created via StreamableHTTPSessionManager persist indefinitely in _server_instances even after clients disconnect without sending DELETE. Over time this leaks memory. This was reported in #1283 and originally addressed in #1159 by @hopeful0 — this PR reworks that approach.

Key design decisions:

  • Idle timeout logic lives in the session manager, not the transport. The manager already owns the session lifecycle (_server_instances, crash cleanup), so this is the natural place for reaping. The transport stays unaware of timeout logic.
  • A background reaper task scans _server_instances periodically and terminates sessions that have been idle longer than the threshold.
  • When retry_interval (SSE polling) is configured, the effective timeout is at least retry_interval_seconds * 3 to avoid reaping sessions that are simply between polling reconnections.
  • terminate() on the transport is now idempotent (early return if already terminated), making it safe to call from both the reaper and explicit DELETE requests.
  • Default is None (no timeout, fully backwards compatible). Docstring recommends 1800s (30 min) for most deployments.

How Has This Been Tested?

6 new tests in tests/issues/test_1283_idle_session_cleanup.py:

  • Session reaped after idle timeout
  • Activity resets the idle timer
  • Multiple sessions reaped independently
  • terminate() idempotency
  • Effective timeout with retry_interval
  • No reaper when timeout is None

All existing tests pass unchanged.

Breaking Changes

None. session_idle_timeout defaults to None (no timeout).

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

Based on the approach from PR #1159 by @hopeful0, reworked to move idle tracking to the session manager, remove __aenter__/__aexit__ and condition variables from the transport, and account for retry_interval in the idle threshold.

Closes #1283

Add session_idle_timeout parameter to StreamableHTTPSessionManager that
enables automatic cleanup of idle sessions, fixing the memory leak
described in issue #1283.

Key design decisions:
- Idle timeout logic lives in the session manager, not the transport.
  The manager already owns the session lifecycle, so this is the natural
  place for reaping. The transport stays unaware of timeout logic.
- A background reaper task scans _server_instances periodically and
  terminates sessions that have been idle longer than the threshold.
- When retry_interval (SSE polling) is configured, the effective timeout
  is at least retry_interval_seconds * 3 to avoid reaping sessions that
  are simply between polling reconnections.
- terminate() on the transport is now idempotent (early return if already
  terminated), making it safe to call from both the reaper and explicit
  DELETE requests.

Based on the approach from PR #1159 by @hopeful0, reworked to:
- Move idle tracking to the session manager instead of the transport
- Remove __aenter__/__aexit__ context manager and condition variables
- Account for retry_interval in the idle threshold
- Add comprehensive tests

Github-Issue: #1283
Reported-by: hopeful0
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.

1 participant