Skip to content

Commit ed31001

Browse files
committed
Drop a stream whose reader stalls instead of blocking its writers
## Motivation and Context The Streamable HTTP transport writes a notification to one SSE stream after another, on the thread that sent it, and a write to a stream had no time limit. A client that opens a `subscriptions/listen` stream, or the GET stream of a session, and stops reading fills its socket buffers after a few hundred kilobytes of notifications; the next write to it then blocks for as long as the client keeps the connection. Against Puma, the thread calling `notify_tools_list_changed` stayed in that write, the streams after the stalled one received nothing more, and every later notification blocked behind it, until the client read again. The keepalive of the stalled stream blocks the same way while holding the stream's write mutex. The frames the transport pushes onto a stream on its own (a notification, a server-to-client request, a listen acknowledgement, a keepalive) now get `stream_write_timeout:` seconds, 10 by default. A write still unfinished then fails as a write to a broken stream does, with `StreamWriteTimeoutError`, an `IOError`, so the existing paths drop and close the stream and delivery moves on. The stream is closed, or its close abandoned, before that error leaves the write, so that no writer waiting on the write mutex of a listen stream puts a frame after the one cut short. A write blocks only once the peer is already far behind; a reader that slow for the whole timeout is dropped like one that stopped. The response of a request is not bounded: it is written on that request's own thread and may be large. `stream_write_timeout: nil` keeps the previous behavior. The deadline interrupts the write with the exception `Timeout` raises on its own and becomes `StreamWriteTimeoutError` only where it leaves the write, so a stream implementation that rescues `IOError` inside its `write` cannot swallow it. The entry of a listen stream is marked closed under the same hold of its write mutex. Closing a stream gets the same number of seconds, since a close can block on the same full buffers, and one that has not finished by then is abandoned and not tried again. The closing result of a teardown skips an entry marked that way. The bound is per stream and the streams are still written in turn: each stalled stream holds a notification back for that long once, before it is dropped, and for up to twice that long when closing it blocks as well, so the waits of several stalled streams add up. ## How Has This Been Tested? `test/mcp/server/transports/streamable_http_transport_test.rb`. Checked by hand against Puma with a client that never reads: with the previous library the sender blocks in the stalled write and the other subscriber receives nothing more; now the sender keeps going and the other subscriber keeps receiving. With the deadline raised as an `IOError`, a stream that rescues `IOError` and retries held the sender for the length of its block, and a close that blocks held it twice over. ## Breaking Changes A stream whose peer does not accept a pushed frame within 10 seconds is closed, where the write used to wait without limit. For the GET stream of a session this ends the session, as any failed write to it does.
1 parent 2b6c13e commit ed31001

3 files changed

Lines changed: 303 additions & 7 deletions

File tree

‎docs/_server/subscriptions.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@ Concurrent streams are capped by `max_listen_subscriptions:` (default 1000; pass
9898
comment frame every `listen_keepalive_interval:` seconds (default 15) so a dropped connection frees its slot; pass `listen_keepalive_interval: nil`
9999
when an upstream proxy already keeps the stream alive.
100100

101+
A notification is written to one stream after another, so a client that stops reading its stream must not hold the others up.
102+
A notification, an acknowledgement or a keepalive that has not been written within `stream_write_timeout:` seconds (default 10) ends that stream,
103+
as a failed write does, and delivery moves on; pass `stream_write_timeout: nil` to wait without limit. The same bound applies to
104+
the notifications and server-to-client requests written to the streams of a session.
105+
The bound is per stream: each stalled stream still holds a notification back for that long once, before it is dropped,
106+
and for up to twice that long when closing it blocks as well, so the waits of several stalled streams add up.
107+
A reader too slow to take a frame in that time is dropped like one that stopped.
108+
101109
The resource URIs one request names in `resourceSubscriptions` are held for the life of the stream, so the total byte length
102110
of the distinct URIs is bounded by `max_resource_subscription_bytes:` (default 64 KiB, about a thousand ordinary URIs;
103111
pass `nil` to remove the bound). The number of distinct URIs is bounded as well, at 1024, and that bound always applies,

‎lib/mcp/server/transports/streamable_http_transport.rb‎

Lines changed: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require "json"
44
require "set"
5+
require "timeout"
56
require_relative "../../result_type"
67
require_relative "../../transport"
78

@@ -188,6 +189,13 @@ class InvalidJsonError < StandardError; end
188189
# @param server_to_client_request_timeout [Numeric] seconds a server-to-client request waits for its
189190
# response before the transport stops waiting and raises `MCP::Server::RequestTimeoutError`.
190191
# Defaults to `DEFAULT_SERVER_TO_CLIENT_REQUEST_TIMEOUT` (600); individual calls override it with `timeout:`.
192+
# @param stream_write_timeout [Numeric, nil] seconds a notification, a server-to-client request, a listen
193+
# acknowledgement or a keepalive may take to be written to an SSE stream before the stream is dropped as stalled.
194+
# These writes are made on behalf of every stream in turn, so a peer that stops reading would otherwise hold
195+
# the writer, and the streams after it, for as long as its buffers stay full. The response of a request is not bounded:
196+
# it is written on that request's own thread and may be large. The bound is per stream, so several stalled streams delay
197+
# a notification by the sum of their waits, once each. Closing a stream is given the same number of seconds, once.
198+
# Defaults to `DEFAULT_STREAM_WRITE_TIMEOUT` (10); pass `nil` to wait without limit.
191199
# @param token_verifier [#verify, nil] enables built-in OAuth 2.1 bearer enforcement: anything responding to
192200
# `verify(token) -> MCP::Server::OAuth::AccessToken | nil` (see `MCP::Server::OAuth::TokenVerifier` for
193201
# the contract and the built-in JWT/introspection implementations). Every POST, GET, and DELETE is verified
@@ -227,6 +235,7 @@ def initialize(
227235
listen_keepalive_interval: DEFAULT_LISTEN_KEEPALIVE_INTERVAL,
228236
serve_subscriptions_listen: true,
229237
server_to_client_request_timeout: DEFAULT_SERVER_TO_CLIENT_REQUEST_TIMEOUT,
238+
stream_write_timeout: DEFAULT_STREAM_WRITE_TIMEOUT,
230239
token_verifier: nil,
231240
required_scopes: [],
232241
resource_metadata: nil,
@@ -325,6 +334,15 @@ def initialize(
325334

326335
@server_to_client_request_timeout = server_to_client_request_timeout
327336

337+
unless stream_write_timeout.nil? || (stream_write_timeout.is_a?(Numeric) && stream_write_timeout.positive?)
338+
raise ArgumentError, "stream_write_timeout must be a positive number or nil"
339+
end
340+
341+
@stream_write_timeout = stream_write_timeout
342+
343+
# Keyed by identity and weak, so a stream is not kept alive for having been recorded here.
344+
@streams_with_abandoned_close = ObjectSpace::WeakMap.new
345+
328346
if token_verifier
329347
@oauth_authenticator = OAuth::Authenticator.new(
330348
token_verifier: token_verifier,
@@ -358,6 +376,14 @@ def initialize(
358376
REQUIRED_POST_ACCEPT_TYPES_JSON = ["application/json"].freeze
359377
REQUIRED_GET_ACCEPT_TYPES = ["text/event-stream"].freeze
360378
STREAM_WRITE_ERRORS = [IOError, Errno::EPIPE, Errno::ECONNRESET].freeze
379+
380+
# Raised from a write the transport pushes onto an SSE stream when it has not finished within `stream_write_timeout:` seconds.
381+
# An `IOError`, so every path that drops a stream on a failed write drops this one the same way.
382+
class StreamWriteTimeoutError < IOError; end
383+
384+
# A write blocks only once the peer's socket buffers are full, which takes a reader already hundreds of kilobytes behind;
385+
# one that still has not taken the frame after this long is dropped whether it stopped reading or is only that slow.
386+
DEFAULT_STREAM_WRITE_TIMEOUT = 10
361387
SESSION_REAP_INTERVAL = 60
362388
# Seconds `close` waits for a reap in progress to finish closing its streams before it kills the reaper.
363389
REAPER_JOIN_TIMEOUT = 5
@@ -633,7 +659,7 @@ def deliver_broadcast_notification(notification)
633659
# Writes a notification to an SSE stream without holding `@mutex`. On a write error,
634660
# drops the broken stream and returns false; on success returns true.
635661
def write_notification(stream, notification, session_id, related_request_id)
636-
send_to_stream(stream, notification)
662+
push_to_stream(stream, notification)
637663
true
638664
rescue *STREAM_WRITE_ERRORS => e
639665
MCP.configuration.exception_reporter.call(e, { session_id: session_id, error: "Failed to send notification" })
@@ -736,7 +762,7 @@ def send_request(method, params = nil, session_id: nil, related_request_id: nil,
736762
sent = false
737763
if stream
738764
begin
739-
send_to_stream(stream, request)
765+
push_to_stream(stream, request)
740766
sent = true
741767
rescue *STREAM_WRITE_ERRORS
742768
drop_broken_stream(session_id, stream, related_request_id)
@@ -880,6 +906,41 @@ def send_ping_to_stream(stream)
880906
stream.flush
881907
end
882908

909+
# `send_to_stream` and `send_ping_to_stream` for the frames the transport pushes onto a stream on its own: a notification,
910+
# a server-to-client request, a listen acknowledgement, a keepalive. One thread writes them to stream after stream,
911+
# and a keepalive write holds its stream's write mutex, so a peer that stopped reading would stall the notifications of
912+
# every other client, and each thread that sends one, for as long as its buffers stay full.
913+
# The write is given `stream_write_timeout:` seconds; past that it fails as a write to a broken stream does, and the caller drops the stream.
914+
#
915+
# The deadline may cut a frame short, so the stream is closed here, before the error leaves: a listen stream is written under its write mutex,
916+
# which the error releases on its way out, ahead of the caller's removal of the entry, and a writer waiting on that mutex would otherwise put
917+
# its frame after the cut one. Closed first, every later write fails as on any closed stream. The entry of a listen stream, passed as `subscription`,
918+
# is marked closed as well, still under its write mutex, so those writers skip it even when the close itself had to be abandoned.
919+
#
920+
#
921+
# The deadline interrupts the write with the exception `Timeout` raises on its own, which no `rescue` of an `IOError` or a `StandardError` inside
922+
# a stream implementation catches, and becomes `StreamWriteTimeoutError` only here, where it leaves the write.
923+
def push_to_stream(stream, data, subscription: nil)
924+
within_stream_write_timeout(stream, subscription) { send_to_stream(stream, data) }
925+
end
926+
927+
def push_ping_to_stream(stream, subscription: nil)
928+
within_stream_write_timeout(stream, subscription) { send_ping_to_stream(stream) }
929+
end
930+
931+
def within_stream_write_timeout(stream, subscription, &block)
932+
return yield unless @stream_write_timeout
933+
934+
begin
935+
Timeout.timeout(@stream_write_timeout, &block)
936+
rescue Timeout::Error
937+
subscription[:closed] = true if subscription
938+
close_stream_safely(stream)
939+
940+
raise StreamWriteTimeoutError, "SSE stream write did not finish within #{@stream_write_timeout} seconds"
941+
end
942+
end
943+
883944
# Serves one request of the stateless modern lifecycle (MCP 2026-07-28, SEP-2575):
884945
# a single POST/JSON exchange with no session. The modern path never consults
885946
# `@stateless`, `@sessions`, or `@enable_json_response`, and never issues or accepts
@@ -1284,7 +1345,7 @@ def listen_sse_body(request_id, honored, acknowledgement, auth_info = nil)
12841345
acknowledged = subscription[:write_mutex].synchronize do
12851346
next false if subscription[:closed]
12861347

1287-
send_to_stream(stream, acknowledgement)
1348+
push_to_stream(stream, acknowledgement, subscription: subscription)
12881349

12891350
# Set on the entry itself, not through the registry: a concurrent close may already have cleared the registry
12901351
# while its result write waits on this mutex, and that write must still find the stream acknowledged.
@@ -1365,7 +1426,7 @@ def send_listen_keepalive_ping(subscription_key)
13651426
subscription[:write_mutex].synchronize do
13661427
next if subscription[:closed]
13671428

1368-
send_ping_to_stream(subscription[:stream])
1429+
push_ping_to_stream(subscription[:stream], subscription: subscription)
13691430
end
13701431
end
13711432

@@ -1507,7 +1568,7 @@ def deliver_to_listen_subscriptions(method, params)
15071568
subscription[:write_mutex].synchronize do
15081569
next if subscription[:closed]
15091570

1510-
send_to_stream(subscription[:stream], notification)
1571+
push_to_stream(subscription[:stream], notification, subscription: subscription)
15111572
end
15121573
rescue *STREAM_WRITE_ERRORS => e
15131574
MCP.configuration.exception_reporter.call(
@@ -1597,6 +1658,10 @@ def teardown_listen_subscriptions(timeout: LISTEN_TEARDOWN_TIMEOUT)
15971658
# The stream is left open for the caller to close.
15981659
def write_listen_closing_result(subscription)
15991660
subscription[:write_mutex].synchronize do
1661+
# Already marked by a write that ran into its deadline: the last frame on the stream may be cut short,
1662+
# and the stream may still be open because its close was abandoned, so the result must not follow it.
1663+
next if subscription[:closed]
1664+
16001665
subscription[:closed] = true
16011666

16021667
# A stream whose acknowledgement was never written gets no result either: SEP-2575 makes
@@ -1905,8 +1970,21 @@ def cleanup_and_collect_stream(session_id, streams_to_close)
19051970
removed[:post_request_streams]&.each_value { |stream| streams_to_close << stream }
19061971
end
19071972

1973+
# A close can block on the same full buffers a write does (a TLS stream writes its closing alert, a buffered one flushes),
1974+
# and it runs on the threads that deliver to every stream, in places under a stream's write mutex,
1975+
# so it gets `stream_write_timeout:` seconds as well. One that has not finished by then is abandoned,
1976+
# and not tried again: the path that drops a stream after its write timed out closes it a second time,
1977+
# which would wait out the same block once more.
19081978
def close_stream_safely(stream)
1909-
stream&.close
1979+
return unless stream
1980+
return stream.close unless @stream_write_timeout
1981+
return if @streams_with_abandoned_close.key?(stream)
1982+
1983+
begin
1984+
Timeout.timeout(@stream_write_timeout) { stream.close }
1985+
rescue Timeout::Error
1986+
@streams_with_abandoned_close[stream] = true
1987+
end
19101988
rescue StandardError
19111989
# Ignore close-related errors from already closed/broken streams.
19121990
end
@@ -2728,7 +2806,7 @@ def detach_get_stream(session)
27282806
def send_keepalive_ping(session_id, stream)
27292807
return unless session_holds_stream?(session_id, stream)
27302808

2731-
send_ping_to_stream(stream)
2809+
push_ping_to_stream(stream)
27322810
rescue *STREAM_WRITE_ERRORS => e
27332811
MCP.configuration.exception_reporter.call(
27342812
e,

0 commit comments

Comments
 (0)