Skip to content

Commit 7ce0e69

Browse files
committed
fix(http): cancel local session requests on disconnect
1 parent 08e0211 commit 7ce0e69

2 files changed

Lines changed: 220 additions & 10 deletions

File tree

‎crates/rmcp/src/transport/streamable_http_server/session/local.rs‎

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
use std::{
22
collections::{HashMap, HashSet, VecDeque},
33
num::ParseIntError,
4+
pin::Pin,
45
sync::Arc,
6+
task::{Context, Poll},
57
time::{Duration, Instant},
68
};
79

810
use futures::{Stream, StreamExt};
11+
use pin_project_lite::pin_project;
912
use thiserror::Error;
1013
use tokio::sync::{
1114
mpsc::{Receiver, Sender},
@@ -17,9 +20,10 @@ use tracing::instrument;
1720
use crate::{
1821
RoleServer,
1922
model::{
20-
CancelledNotificationParam, ClientJsonRpcMessage, ClientNotification, ClientRequest,
21-
JsonRpcNotification, JsonRpcRequest, Notification, ProgressNotificationParam,
22-
ProgressToken, RequestId, ServerJsonRpcMessage, ServerNotification,
23+
CancelledNotification, CancelledNotificationParam, ClientJsonRpcMessage,
24+
ClientNotification, ClientRequest, JsonRpcNotification, JsonRpcRequest, Notification,
25+
ProgressNotificationParam, ProgressToken, RequestId, ServerJsonRpcMessage,
26+
ServerNotification,
2327
},
2428
transport::{
2529
WorkerTransport,
@@ -109,7 +113,11 @@ impl SessionManager for LocalSessionManager {
109113
let receiver = handle.establish_request_wise_channel().await?;
110114
let http_request_id = receiver.http_request_id;
111115
handle.push_message(message, http_request_id).await?;
112-
Ok(ReceiverStream::new(receiver.inner))
116+
Ok(RequestWiseResponseStream::new(
117+
ReceiverStream::new(receiver.inner),
118+
handle.clone(),
119+
http_request_id,
120+
))
113121
}
114122

115123
async fn create_standalone_stream(
@@ -426,6 +434,53 @@ pub struct StreamableHttpMessageReceiver {
426434
pub inner: Receiver<ServerSseMessage>,
427435
}
428436

437+
pin_project! {
438+
/// Cancels the in-flight session request when the request-wise response
439+
/// stream is dropped before it reaches natural completion.
440+
struct RequestWiseResponseStream {
441+
#[pin]
442+
inner: ReceiverStream<ServerSseMessage>,
443+
handle: LocalSessionHandle,
444+
http_request_id: Option<HttpRequestId>,
445+
}
446+
447+
impl PinnedDrop for RequestWiseResponseStream {
448+
fn drop(this: Pin<&mut Self>) {
449+
let this = this.project();
450+
if let Some(id) = this.http_request_id.take() {
451+
this.handle.cancel_request_wise_channel_on_disconnect(id);
452+
}
453+
}
454+
}
455+
}
456+
457+
impl RequestWiseResponseStream {
458+
fn new(
459+
inner: ReceiverStream<ServerSseMessage>,
460+
handle: LocalSessionHandle,
461+
http_request_id: Option<HttpRequestId>,
462+
) -> Self {
463+
Self {
464+
inner,
465+
handle,
466+
http_request_id,
467+
}
468+
}
469+
}
470+
471+
impl Stream for RequestWiseResponseStream {
472+
type Item = ServerSseMessage;
473+
474+
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
475+
let this = self.project();
476+
let polled = this.inner.poll_next(cx);
477+
if let Poll::Ready(None) = &polled {
478+
*this.http_request_id = None;
479+
}
480+
polled
481+
}
482+
}
483+
429484
impl LocalSessionWorker {
430485
fn unregister_resource(&mut self, resource: &ResourceKey) {
431486
let Some(http_request_id) = self.resource_router.remove(resource) else {
@@ -486,6 +541,22 @@ impl LocalSessionWorker {
486541
self.unregister_resource(&resource);
487542
}
488543
}
544+
fn remove_request_wise_channel(&mut self, id: HttpRequestId) -> Vec<RequestId> {
545+
let Some(channel) = self.tx_router.remove(&id) else {
546+
return Vec::new();
547+
};
548+
channel
549+
.resources
550+
.into_iter()
551+
.filter_map(|resource| {
552+
self.resource_router.remove(&resource);
553+
match resource {
554+
ResourceKey::McpRequestId(request_id) => Some(request_id),
555+
ResourceKey::ProgressToken(_) => None,
556+
}
557+
})
558+
.collect()
559+
}
489560
fn evict_expired_channels(&mut self) {
490561
let ttl = self.session_config.completed_cache_ttl;
491562
self.tx_router
@@ -813,6 +884,9 @@ pub enum SessionEvent {
813884
id: HttpRequestId,
814885
responder: oneshot::Sender<Result<(), SessionError>>,
815886
},
887+
CancelRequestWiseChannel {
888+
id: HttpRequestId,
889+
},
816890
Resume {
817891
last_event_id: EventId,
818892
responder: oneshot::Sender<Result<StreamableHttpMessageReceiver, SessionError>>,
@@ -914,6 +988,19 @@ impl LocalSessionHandle {
914988
.map_err(|_| SessionError::SessionServiceTerminated)?
915989
}
916990

991+
fn cancel_request_wise_channel_on_disconnect(&self, request_id: HttpRequestId) {
992+
let event = SessionEvent::CancelRequestWiseChannel { id: request_id };
993+
match self.event_tx.try_send(event) {
994+
Ok(()) | Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => {}
995+
Err(tokio::sync::mpsc::error::TrySendError::Full(event)) => {
996+
let event_tx = self.event_tx.clone();
997+
tokio::spawn(async move {
998+
let _ = event_tx.send(event).await;
999+
});
1000+
}
1001+
}
1002+
}
1003+
9171004
/// Establish a common channel for general purpose messages.
9181005
pub async fn establish_common_channel(
9191006
&self,
@@ -1186,9 +1273,24 @@ impl Worker for LocalSessionWorker {
11861273
id,
11871274
responder,
11881275
}) => {
1189-
let _handle_result = self.tx_router.remove(&id);
1276+
self.remove_request_wise_channel(id);
11901277
let _ = responder.send(Ok(()));
11911278
}
1279+
InnerEvent::FromHttpService(SessionEvent::CancelRequestWiseChannel { id }) => {
1280+
let request_ids = self.remove_request_wise_channel(id);
1281+
for request_id in request_ids {
1282+
context
1283+
.send_to_handler(ClientJsonRpcMessage::notification(
1284+
ClientNotification::CancelledNotification(
1285+
CancelledNotification::new(CancelledNotificationParam::new(
1286+
Some(request_id),
1287+
Some("client disconnected".to_owned()),
1288+
)),
1289+
),
1290+
))
1291+
.await?;
1292+
}
1293+
}
11921294
InnerEvent::FromHttpService(SessionEvent::Resume {
11931295
last_event_id,
11941296
responder,

‎crates/rmcp/tests/test_streamable_http_disconnect_cancel.rs‎

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,9 @@
55
not(feature = "local")
66
))]
77

8-
//! Regression test for #857: when a stateless streamable-HTTP client disconnects
8+
//! Regression tests for #857 and #1325: when a streamable-HTTP client disconnects
99
//! (drops the response) while a tool handler is still awaiting, the per-request
1010
//! `RequestContext::ct` should fire so the handler can cancel cooperatively.
11-
//!
12-
//! Stateless requests are one-shot (no session, no resumption), so a dropped
13-
//! response is terminal and safe to cancel — unlike the stateful/resumable path,
14-
//! where a disconnect may be recovered via `Last-Event-ID`.
1511
1612
use std::{sync::Arc, time::Duration};
1713

@@ -114,6 +110,47 @@ async fn spawn_stateless_server(json_response: bool) -> anyhow::Result<TestServe
114110
})
115111
}
116112

113+
async fn spawn_session_server() -> anyhow::Result<TestServer> {
114+
let started = Arc::new(Notify::new());
115+
let cancelled = Arc::new(Notify::new());
116+
let probe = CancelProbe {
117+
started: started.clone(),
118+
cancelled: cancelled.clone(),
119+
};
120+
121+
let server_ct = CancellationToken::new();
122+
let config = StreamableHttpServerConfig::default()
123+
// A short keep-alive lets the SSE server notice a dropped connection
124+
// quickly (hyper only observes the disconnect on its next write).
125+
.with_sse_keep_alive(Some(Duration::from_millis(100)))
126+
.with_cancellation_token(server_ct.child_token());
127+
128+
let service: StreamableHttpService<CancelProbe, LocalSessionManager> =
129+
StreamableHttpService::new(
130+
move || Ok(probe.clone()),
131+
Arc::new(LocalSessionManager::default()),
132+
config,
133+
);
134+
let router = axum::Router::new().nest_service("/mcp", service);
135+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
136+
let addr = listener.local_addr()?;
137+
tokio::spawn({
138+
let ct = server_ct.clone();
139+
async move {
140+
let _ = axum::serve(listener, router)
141+
.with_graceful_shutdown(async move { ct.cancelled_owned().await })
142+
.await;
143+
}
144+
});
145+
146+
Ok(TestServer {
147+
url: format!("http://{addr}/mcp"),
148+
server_ct,
149+
started,
150+
cancelled,
151+
})
152+
}
153+
117154
/// SSE mode: the response is a stream; dropping it (client disconnect) must fire
118155
/// the handler's cancellation token.
119156
#[tokio::test]
@@ -154,6 +191,77 @@ async fn stateless_sse_client_disconnect_cancels_request() -> anyhow::Result<()>
154191
Ok(())
155192
}
156193

194+
/// Session mode: dropping the request-wise response stream must cancel the
195+
/// in-flight request owned by the local session worker.
196+
#[tokio::test]
197+
async fn stateful_sse_client_disconnect_cancels_request() -> anyhow::Result<()> {
198+
let server = spawn_session_server().await?;
199+
let client = reqwest::Client::builder()
200+
.pool_max_idle_per_host(0)
201+
.build()?;
202+
203+
let init = client
204+
.post(&server.url)
205+
.header("Content-Type", "application/json")
206+
.header("Accept", "application/json, text/event-stream")
207+
.header("MCP-Protocol-Version", "2025-06-18")
208+
.body(r#"{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"test","version":"0.1.0"}}}"#)
209+
.send()
210+
.await?;
211+
assert!(
212+
init.status().is_success(),
213+
"initialize failed: {:?}",
214+
init.status()
215+
);
216+
let session_id = init
217+
.headers()
218+
.get("mcp-session-id")
219+
.expect("initialize response should include session id")
220+
.to_str()?
221+
.to_owned();
222+
let _ = init.text().await?;
223+
224+
let initialized = client
225+
.post(&server.url)
226+
.header("Content-Type", "application/json")
227+
.header("Accept", "application/json, text/event-stream")
228+
.header("mcp-session-id", &session_id)
229+
.header("MCP-Protocol-Version", "2025-06-18")
230+
.body(r#"{"jsonrpc":"2.0","method":"notifications/initialized"}"#)
231+
.send()
232+
.await?;
233+
assert_eq!(initialized.status(), reqwest::StatusCode::ACCEPTED);
234+
235+
let call = client
236+
.post(&server.url)
237+
.header("Content-Type", "application/json")
238+
.header("Accept", "application/json, text/event-stream")
239+
.header("mcp-session-id", &session_id)
240+
.header("MCP-Protocol-Version", "2025-06-18")
241+
.body(CALL_BODY)
242+
.send()
243+
.await?;
244+
assert!(
245+
call.status().is_success(),
246+
"tools/call failed: {:?}",
247+
call.status()
248+
);
249+
250+
tokio::time::timeout(Duration::from_secs(5), server.started.notified())
251+
.await
252+
.expect("tool handler should start");
253+
254+
drop(call);
255+
drop(client);
256+
257+
tokio::time::timeout(Duration::from_secs(10), server.cancelled.notified())
258+
.await
259+
.expect("RequestContext::ct should fire after client disconnect (stateful SSE)");
260+
261+
server.server_ct.cancel();
262+
Ok(())
263+
}
264+
157265
/// JSON-direct mode: the server holds the connection open awaiting the single
158266
/// response. A client that disconnects while the handler is running must still
159267
/// fire the handler's cancellation token.

0 commit comments

Comments
 (0)