modelcontextprotocol / modelcontextprotocol/java-sdk

Stateful streamable server calls listRoots() unconditionally on roots/list_changed, blocking a servlet thread until requestTimeout when no roots consumer is registered

Aperta
#1,061 2 commenti 1 reazione 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

bug P2 ready for work
Lingua principale
Java
Stelle
3.7k
Fork
1.1k
Merge medio
1g 15h
PR unite (30g)
9

Descrizione

Stateful streamable server calls listRoots() unconditionally on roots/list_changed, blocking a servlet thread until requestTimeout when no roots consumer is registered

Bug description

When a McpAsyncServer / McpSyncServer uses the streamable HTTP transport
(HttpServletStreamableServerTransportProvider) and a client that advertises
capabilities.roots.listChanged sends a notifications/roots/list_changed
message, the server issues a server→client roots/list request back to the
client — even when the application registered no rootsChangeConsumer.

The transport handles the incoming notification by blocking the servlet
(request) thread
on that round-trip. If the client does not answer roots/list
(many don't — it's a fire-and-forget notification from their side), the thread
stays parked for the full requestTimeout and then fails with a
TimeoutException. Every such notification pins one HTTP worker thread for the
whole timeout window; under multiple clients this degrades the server's request
pool.

This is the stateful counterpart of the already-merged stateless fix in
#835
. #835 made roots/list_changed a no-op for
HttpServletStatelessServerTransport; the stateful streamable path
(McpAsyncServer.asyncRootsListChangedNotificationHandler) was not touched and
still performs the unconditional round-trip — and here it doesn't just log a
warning, it blocks a thread.

Environment

  • io.modelcontextprotocol.sdk:mcp-core 2.0.0 (also present on main)
  • Transport: Streamable HTTP, HttpServletStreamableServerTransportProvider
  • Server: McpServer.sync(...), servlet container (Jetty / Tomcat)
  • Client: any client that declares roots.listChanged and sends the
    notification without answering the resulting roots/list (e.g. Claude Desktop
    via mcp-remote)

Root cause

1. The notification handler calls listRoots() unconditionally.

McpAsyncServer.prepareNotificationHandlers substitutes a logging consumer when
none is registered (mcp-core 2.0.0, lines 199–201):

if (Utils.isEmpty(rootsChangeConsumers)) {
    rootsChangeConsumers = List.of((exchange, roots) -> Mono.fromRunnable(() -> logger
        .warn("Roots list changed notification, but no consumers provided. Roots list changed: {}", roots)));
}

and asyncRootsListChangedNotificationHandler then calls
exchange.listRoots() regardless (line 317):

return (exchange, params) -> exchange.listRoots()          // <-- always sent, even to log-and-drop
    .flatMap(listRootsResult -> Flux.fromIterable(rootsChangeConsumers)
        .flatMap(consumer -> Mono.defer(() -> consumer.apply(exchange, listRootsResult.roots())))
        ...

So a roots/list request goes out to the client to fetch a value that is only
logged.

2. The streamable transport blocks the servlet thread on the notification.

HttpServletStreamableServerTransportProvider.doPost handles a notification by
.block()-ing (mcp-core 2.0.0, line 505):

else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) {
    session.accept(jsonrpcNotification)
        .contextWrite(ctx -> ctx.put(McpTransportContext.KEY, transportContext))
        .block();                                          // <-- request thread parks here
    response.setStatus(HttpServletResponse.SC_ACCEPTED);
}

3. The server→client request times out after requestTimeout.

McpStreamableServerSession$McpStreamableServerSessionStream.sendRequest
(lines 404–412):

return Mono.<McpSchema.JSONRPCResponse>create(sink -> {
    this.pendingResponses.put(requestId, sink);
    ...
    this.transport.sendMessage(jsonrpcRequest, messageId).subscribe(v -> {}, sink::error);
}).timeout(requestTimeout)                                 // <-- no answer → TimeoutException

Because McpServer.sync(...) defaults requestTimeout to 10s (line 945) — and
higher if the app raised it — the thread is pinned for that entire window and
then logs:

ERROR ... HttpServletStreamableServerTransportProvider: Error handling message:
  java.util.concurrent.TimeoutException: Did not observe any item or terminal
  signal within 10000ms in 'source(MonoCreate)'

Steps to reproduce

A stateful streamable McpServer.sync(...) on
HttpServletStreamableServerTransportProvider with no rootsChangeConsumer
registered. Endpoint is /mcp below.

# 1. initialize, declaring roots.listChanged — capture the mcp-session-id from response headers
curl -sS -D - -o /dev/null -X POST http://localhost:8080/mcp \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -d '{"jsonrpc":"2.0","id":0,"method":"initialize",
       "params":{"protocolVersion":"2025-11-25",
                 "capabilities":{"roots":{"listChanged":true}},
                 "clientInfo":{"name":"roots-repro","version":"1.0.0"}}}'

SID=<mcp-session-id from above>

# 2. complete the handshake
curl -sS -X POST http://localhost:8080/mcp \
  -H 'Content-Type: application/json' -H 'Accept: application/json, text/event-stream' \
  -H "mcp-session-id: $SID" \
  -d '{"jsonrpc":"2.0","method":"notifications/initialized"}'

# 3. open the listening GET SSE stream (REQUIRED — without it the server fails fast
#    with "Stream unavailable for session" instead of stalling)
curl -sS -N http://localhost:8080/mcp -H 'Accept: text/event-stream' \
  -H "mcp-session-id: $SID" &

# 4. the trigger — this POST hangs for the full requestTimeout, then returns 500
time curl -sS -X POST http://localhost:8080/mcp \
  -H 'Content-Type: application/json' -H 'Accept: application/json, text/event-stream' \
  -H "mcp-session-id: $SID" \
  -d '{"jsonrpc":"2.0","method":"notifications/roots/list_changed"}'

On the listening stream from step 3 you will see the server push a
{"method":"roots/list",...} request; nobody answers it, and step 4 blocks for
requestTimeout seconds.

Expected behavior

notifications/roots/list_changed should be acknowledged (202) promptly. When
no rootsChangeConsumer is registered, the server should not send a
roots/list request at all — there is nothing to deliver the result to. This
matches the stateless behavior already merged in #835.

Actual behavior

The server sends roots/list, the servlet thread blocks on it, and after
requestTimeout it fails with the MonoCreate TimeoutException above. One
HTTP worker thread is consumed per notification for the full timeout.

Impact

  • One pinned HTTP worker thread per roots/list_changed, for up to
    requestTimeout; concurrent clients multiply it and degrade the request pool.
  • Recurring ERROR-level log noise.
  • Affects every client that declares roots.listChanged and doesn't answer the
    unsolicited roots/list — which is spec-legal client behavior.

Proposed fix

Skip the round-trip when there is nothing to consume — the stateful analog of
#835:

  • In asyncRootsListChangedNotificationHandler (or
    prepareNotificationHandlers), only call exchange.listRoots() when a real
    rootsChangeConsumer was registered; otherwise treat the notification as a
    no-op. Don't substitute a logging consumer that forces a client round-trip
    purely to log the result.

This also aligns with the direction of #1003 (SEP-2260, "require server requests
to be associated with a client request") and #1012 / #1053 (deprecate roots),
where an unsolicited server→client roots/list triggered by a notification is
being designed out of the spec.

Related

  • #835 (merged) — same fix for the stateless transport
    (roots/list_changed → no-op). This issue asks to extend that to the
    stateful streamable path. Its parent #777 is the stateless "Missing
    handler" symptom.
  • #1003 (SEP-2260) and #1012 / #1053 — spec work removing/deprecating
    unsolicited server→client roots requests. Directional, not a current-SDK fix.
  • #920 and #1021 — separate streamable-transport robustness issues
    (session eviction on write failure; CLOSE-WAIT thread exhaustion). Distinct
    root causes, mentioned only to disambiguate.

Workaround

A servlet filter in front of the MCP endpoint that answers
notifications/roots/list_changed with 202 Accepted and drops it, so it never
reaches the SDK handler.

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia con McpAsyncServer.asyncRootsListChangedNotificationHandler e prepareNotificationHandlers, quindi esamina HttpServletStreamableServerTransportProvider.doPost e il percorso della richiesta della sessione di stream con stato. Riproduci il problema con la sequenza curl fornita e verifica che, senza un rootsChangeConsumer, non venga inviata alcuna richiesta roots/list e che la notifica restituisca prontamente 202.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java
Ambito
api, backend
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Attiva
Chiarezza
Specificata chiaramente
Idoneità per principianti
78/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.