Support session expiry controls for StreamableHTTPTransport#268
Support session expiry controls for StreamableHTTPTransport#268koic wants to merge 1 commit intomodelcontextprotocol:mainfrom
StreamableHTTPTransport#268Conversation
25830a2 to
5f26e0d
Compare
5f26e0d to
ef63429
Compare
| end | ||
|
|
||
| def close | ||
| @reaper_thread&.kill |
There was a problem hiding this comment.
What happens to the mutex if the reaper thread had a lock on it and was busy reaping?
There was a problem hiding this comment.
As far as I could verify, In MRI Ruby, this should be safe. Ruby runs ensure clauses even when a thread is terminated via Thread#kill, and Mutex#synchronize releases the lock in its ensure path.
So if the reaper is killed while holding the mutex, the thread will unwind and the lock should be released as part of that process. As a result, close's @mutex.synchronize is not expected to deadlock due to the mutex being left locked by Thread#kill.
## Motivation and Context The MCP specification recommends expiring session IDs to reduce session hijacking risks: https://modelcontextprotocol.io/specification/latest/basic/security_best_practices#session-hijacking Several other SDKs (Go, C#, PHP, Python) already implement session expiry controls, but the Ruby SDK did not. Sessions persisted indefinitely unless explicitly deleted or a stream error occurred, leaving abandoned sessions to accumulate in memory. This adds a `session_idle_timeout:` option to `StreamableHTTPTransport#initialize`. When set, sessions that receive no HTTP requests for the specified duration (in seconds) are automatically expired. Expired sessions return 404 on subsequent requests (GET and POST), matching the MCP specification's behavior for terminated sessions. Each request resets the idle timer, so actively used sessions are not interrupted. A background reaper thread periodically cleans up expired sessions to handle orphaned sessions that receive no further requests. The reaper only starts when `session_idle_timeout` is configured. The default is `nil` (no expiry) for backward compatibility, consistent with the Python SDK's approach. The Python SDK recommends 1800 seconds (30 minutes) for most deployments: modelcontextprotocol/python-sdk#2022 Resolves modelcontextprotocol#265. ## How Has This Been Tested? Added tests for session expiry, idle timeout reset, reaper thread lifecycle, input validation, and default behavior in `streamable_http_transport_test.rb`. All existing tests continue to pass. ## Breaking Change None. The default value of `session_idle_timeout` is `nil`, which preserves the existing behavior of sessions never expiring. The new `last_active_at` field in the internal session hash is not part of the public API. Existing code that instantiates `StreamableHTTPTransport.new(server)` or `StreamableHTTPTransport.new(server, stateless: true)` continues to work without changes.
ef63429 to
49fc501
Compare
|
@atesgoral Thanks for the review. It looks like the approval was dismissed when I resolved a previous conflict. Could you take another look when you have a moment? |
Motivation and Context
The MCP specification recommends expiring session IDs to reduce session hijacking risks: https://modelcontextprotocol.io/specification/latest/basic/security_best_practices#session-hijacking
Several other SDKs (Go, C#, PHP, Python) already implement session expiry controls, but the Ruby SDK did not. Sessions persisted indefinitely unless explicitly deleted or a stream error occurred, leaving abandoned sessions to accumulate in memory.
This adds a
session_idle_timeout:option toStreamableHTTPTransport#initialize. When set, sessions that receive no HTTP requests for the specified duration (in seconds) are automatically expired. Expired sessions return 404 on subsequent requests (GET and POST), matching the MCP specification's behavior for terminated sessions. Each request resets the idle timer, so actively used sessions are not interrupted.A background reaper thread periodically cleans up expired sessions to handle orphaned sessions that receive no further requests. The reaper only starts when
session_idle_timeoutis configured.The default is
nil(no expiry) for backward compatibility, consistent with the Python SDK's approach. The Python SDK recommends 1800 seconds (30 minutes) for most deployments: modelcontextprotocol/python-sdk#2022Resolves #265.
How Has This Been Tested?
Added tests for session expiry, idle timeout reset, reaper thread lifecycle, input validation, and default behavior in
streamable_http_transport_test.rb. All existing tests continue to pass.Breaking Change
None. The default value of
session_idle_timeoutisnil, which preserves the existing behavior of sessions never expiring. The newlast_active_atfield in the internal session hash is not part of the public API. Existing code that instantiatesStreamableHTTPTransport.new(server)orStreamableHTTPTransport.new(server, stateless: true)continues to work without changes.Types of changes
Checklist