Skip to content

Close WebMvcStreamableServerTransportProvider transports on disconnect - #6871

Open
lxq19991111 wants to merge 2 commits into
spring-projects:mainfrom
lxq19991111:fix-streamable-transport-lifecycle
Open

lxq19991111 wants to merge 2 commits into
spring-projects:mainfrom
lxq19991111:fix-streamable-transport-lifecycle

Conversation

@lxq19991111

@lxq19991111 lxq19991111 commented Aug 27, 2026 •

Copy link
Copy Markdown

Fixes #6860

Makes the WebMVC Streamable HTTP SSE transport close when the SSE response completes, times out, or errors, and routes SSE write failures through the transport close() path instead of only reporting the error to the SseBuilder.

It mirrors modelcontextprotocol/java-sdk#1021: listening streams close on timeout/error as well as completion, replay and streaming responses close on lifecycle events, and every lifecycle event uses the same idempotent close() (no separate markClosed() flag).

Key design point: a write failure closes only the current HTTP/SSE transport and does NOT remove the logical MCP session. Removing the session on a request-specific write failure would break the next request carrying the same mcp-session-id and Last-Event-ID replay, so session eviction is intentionally left to DELETE / keep-alive eviction.

Testing

Rewrote both tests without reflection, using a package-private transport factory seam (the provider exposes a RouterFunction, so the SDK-style servlet entry-point mocking is not applicable here):

  • sseLifecycleCallbacksCloseStreamOnCompleteTimeoutAndError asserts all three lifecycle callbacks trigger the close callback.

  • sseWriteFailureClosesOnlyCurrentTransportWithoutRemovingSession initializes a session over the router, triggers an SSE write failure, verifies the transport is completed, and then deletes the session with the same id, asserting 200 to prove the session was retained.

    mvn -pl mcp/transport/mcp-spring-webmvc -am \
        -Dtest=WebMvcStreamableServerTransportProviderTests \
        -Dsurefire.failIfNoSpecifiedTests=false test
    

Module unit tests pass, and WebMcpStreamableAsyncServerTransportIT, WebMcpStreamableSyncServerTransportIT, and WebMvcStreamableIT pass.

@lxq19991111 lxq19991111 changed the title Close WebMVC Streamable transports on disconnect Close WebMvcStreamableServerTransportProvider transports on disconnect Aug 27, 2026
@lxq19991111
lxq19991111 force-pushed the fix-streamable-transport-lifecycle branch from 2858e30 to b10647a Compare August 28, 2026 01:33
@lxq19991111
lxq19991111 force-pushed the fix-streamable-transport-lifecycle branch from b10647a to e4325f5 Compare August 31, 2026 15:20
Register SSE lifecycle cleanup so a client that disconnects without a clean DELETE no longer leaves the listening stream, replay stream, or streaming-response transport dangling. Route every lifecycle event through the same idempotent transport close() instead of a separate markClosed() flag, and route SSE write failures through the close path so a write failure closes only the current transport without removing the logical MCP session.

Closes spring-projects#6860

Signed-off-by: lxq19991111 <15060002560@163.com>
An SSE write failure only closed the current WebMVC session transport and
then let the send complete normally, so callers kept writing to a stream
that no longer leads anywhere.

Surface the failure as a McpTransportException instead. The transport
still closes only the current connection and the logical MCP session is
preserved, so a client can still delete or reconnect with the same
session id and Last-Event-ID after a transient write failure.

This matches what modelcontextprotocol/java-sdk#1130 settled on upstream.
Broadcasting is unaffected: notifyClients logs and swallows per-session
failures, and the keep-alive scheduler completes ping failures, so
propagating a single write failure cannot break other sessions.

The write-failure test now asserts the propagated exception rather than
verifying that the SseBuilder was completed on the mock. That assertion
was a false positive: DefaultSseBuilder sets sendFailed on the first
failed write, after which complete() returns immediately, so the mock
verified a call that cannot happen on a real connection.

Signed-off-by: lxq19991111 <15060002560@163.com>
@lxq19991111

Copy link
Copy Markdown
Author

Pushed a follow-up commit propagating SSE write failures: the transport still closes only the current connection and keeps the logical session, but sendMessage now surfaces an McpTransportException instead of completing normally, so callers stop writing to a stream that no longer leads anywhere. This matches what modelcontextprotocol/java-sdk#1130 settled on upstream. Broadcasting is unaffected: notifyClients logs and swallows per-session failures, and the keep-alive scheduler completes ping failures, so a single write failure cannot break other sessions.

Two notes on how this relates to modelcontextprotocol/java-sdk#1130 ("Streamable HTTP: Add session sweeping and close hanging streams"). It landed after the v2.0.1 tag, and it only changes HttpServletStreamableServerTransportProvider in mcp-core, not the WebMVC provider maintained here, so upgrading the MCP SDK alone does not fix DELETE-less disconnects for spring-ai-starter-mcp-server-webmvc users, and this PR is still needed. When Spring AI moves to a release containing that work, the WebMVC transport can be aligned with the new releaseTransport() and response-stream APIs separately.

The write-failure test now asserts the propagated exception and no longer verifies that the SseBuilder was completed on the mock. That assertion was a false positive: DefaultSseBuilder sets sendFailed on the first failed write, after which complete() returns immediately, so the mock verified a call that cannot happen on a real connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] WebMvcStreamableServerTransportProvider: session and socket leak when clients disconnect without DELETE

2 participants