Skip to content

fix(server): release the standalone GET stream on session teardown - #967

Open
lArtiquel wants to merge 1 commit into
modelcontextprotocol:mainfrom
lArtiquel:fix-stateful-get-stream-leak
Open

fix(server): release the standalone GET stream on session teardown#967
lArtiquel wants to merge 1 commit into
modelcontextprotocol:mainfrom
lArtiquel:fix-stateful-get-stream-leak

Conversation

@lArtiquel

Copy link
Copy Markdown

Fixes #922.

handleGetRequest parks on awaitCancellation() for the lifetime of the standalone GET stream. close() only called ServerSSESession.close(), which flushes and closes the response body but never cancels that coroutine, so the ApplicationCall and its connection were never released — one leaked socket per completed session, as measured in the issue.

SessionContext already holds the ApplicationCall, so this cancels its job alongside closing the session.

Same assumption appeared a second time in the GET-replacement path, where the comment claimed session.close() cancels the old coroutine. It doesn't, so a client reconnecting its GET stream leaked the previous one the same way. Fixed and the comment corrected.

Two tests, both failing before the change: one for teardown via close(), one for a replacement stream. Both assert the handler is still suspended before teardown, otherwise they'd pass vacuously — the client-side view can't catch this, since flushAndClose() gives the client EOF while the server coroutine stays parked.

ktlintCheck, detekt, apiCheck and the JVM test suites pass.

…otocol#922)

The standalone GET handler parks on awaitCancellation() for the lifetime
of the stream. Tearing the session down only called ServerSSESession.close(),
which flushes and closes the response body but leaves that coroutine
suspended, so the ApplicationCall and its connection were never released —
one leaked socket per completed session.

Cancel the call alongside closing the session, both when the transport is
closed and when a reconnecting client replaces the standalone stream.
Copilot AI lite review requested due to automatic review settings August 23, 2026 19:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a resource leak in the stateful StreamableHttpServerTransport where the standalone GET SSE handler coroutine remained suspended on awaitCancellation() even after session teardown or stream replacement, leaving the underlying ApplicationCall/socket unreleased. It adds lifecycle tests to ensure the GET handler is still suspended before teardown and is actually released when the transport is closed or the stream is replaced.

Changes:

  • Track each GET stream’s ApplicationCall and explicitly cancel its backing Job on transport close() to release the connection.
  • On replacement of the standalone GET stream, close the old SSE session and also cancel the old call Job.
  • Add JVM tests covering transport teardown and GET-stream replacement lifecycle behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt Cancels the standalone GET stream’s call Job on session teardown and on stream replacement to prevent leaked server-side connections.
kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StatefulStreamableHttpGetStreamLifecycleTest.kt Adds regression tests asserting the GET handler remains suspended until teardown/replacement and is released afterward.
Suppressed comments (1)

kotlin-sdk-server/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StatefulStreamableHttpGetStreamLifecycleTest.kt:124

  • This test launches a standalone CoroutineScope(Dispatchers.Default) for the GET stream but never cancels it. If the test fails before transport.close() (or if the client call hangs), this coroutine can leak past the test lifecycle and cause flakiness in subsequent tests. Tie the launched coroutine to the current test coroutine context instead of creating an independent scope.
        val scope = CoroutineScope(Dispatchers.Default)
        scope.launch { openGetStream(assertNotNull(sessionId)) }


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +75 to +80
val scope = CoroutineScope(Dispatchers.Default)
scope.launch { openGetStream(sessionId) }

awaitRelease(firstReleased) shouldBe true
scope.cancel()
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stateful StreamableHttpServerTransport leaks the standalone GET SSE stream on session close

2 participants