modelcontextprotocol / modelcontextprotocol/kotlin-sdk
onClose callbacks are synchronous, so suspending cleanup has nowhere to run
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 1.5k
- Forks
- 248
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 23
Description
Affected version / component: io.modelcontextprotocol:kotlin-sdk 0.14.0 — shared/Transport.kt, shared/AbstractTransport.kt, server/ServerSession.kt, server/Server.kt
Every close callback in the server stack is a plain () -> Unit. Transport.onClose, ServerSession.onClose, and Server.onClose all take a non-suspending block, and Server.close() invokes the registered chain synchronously. That leaves nowhere to run suspending teardown — closing pooled resources, or cancelling and then joining a child CoroutineScope — without wrapping it in runBlocking, which can stall or deadlock the close path.
The contract is synchronous at each layer. AbstractTransport composes the registered blocks as () -> Unit, and invokeOnCloseCallback() runs them inline inside runCatching, swallowing exceptions. ServerSession.onClose chains the same way, and its onClose() hook fires the chain synchronously. Server.onClose does likewise, and Server.close() calls it directly:
public suspend fun close() {
notificationService.close()
sessions.forEach { (_, session) -> session.close() }
_onClose() // synchronous; no suspension point for teardown
}
close() is itself a suspend function, so the synchronous callback is a limitation of the API contract rather than something the implementation forces.
For context, #778 (shipped in 0.13.0) made the stdio transport's own close() suspending, with a graceful drain of in-flight outbound messages, and moved message handlers to Dispatchers.Default. It did not touch the user-facing close callbacks, which are still non-suspending everywhere, and there is no awaitClose() in 0.14.0.
Suggested fix
Offer a suspending close path — for example an onClose(block: suspend () -> Unit) overload, or a suspend close hook that Server.close() and session/transport teardown await. Either lets a server with suspending resources shut down deterministically instead of bridging through runBlocking.
Workaround
Keep the onClose block limited to synchronous work (flipping flags, cancelling a scope without joining it) and move all suspending teardown into a separate suspend function that callers invoke and await after close. Alternatively, treat the callback purely as a signal that resumes a coroutine, and run the suspending cleanup outside it:
suspend fun awaitClose(session: ServerSession) = suspendCoroutine { cont ->
session.onClose { cont.resume(Unit) }
}
// awaitClose(session); mySuspendingCleanup()
Both work, but they push lifecycle management onto every caller, which is easy to get wrong.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the synchronous close chains in shared/Transport.kt, shared/AbstractTransport.kt, server/ServerSession.kt, and server/Server.kt, including Server.close(). Define a suspending close-hook contract that preserves teardown ordering and exception handling, then verify that suspending cleanup can be awaited without runBlocking at each transport, session, and server layer.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kotlin
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100