Skip to content

Terminate the client message loop when the transport stream finishes - #275

Open
robertoscipionecom wants to merge 2 commits into
modelcontextprotocol:mainfrom
robertoscipionecom:coders-client-loop-fix
Open

Terminate the client message loop when the transport stream finishes#275
robertoscipionecom wants to merge 2 commits into
modelcontextprotocol:mainfrom
robertoscipionecom:coders-client-loop-fix

Conversation

@robertoscipionecom

@robertoscipionecom robertoscipionecom commented Aug 16, 2026

Copy link
Copy Markdown

Problem

Client.connect(transport:) runs its message handling loop as a
repeat { for try await … } while true, which treats the end of the transport stream
as transient: it exits the for, calls receive() again and starts over.

No transport behaves that way. Every transport in this repository exposes a
single-use stream that finishes only when the connection is over:

Transport finish() sites Reopens?
StdioTransport readLoop on EOF/read error, disconnect() no
HTTPClientTransport disconnect() only no
InMemoryTransport disconnect(), peer disconnect no
NetworkTransport disconnect() / isStopping — its internal reconnect reuses the same continuation without finishing it no

Once the stream has finished, receive() hands back the very same finished stream, so
the for returns immediately and the while true starts over immediately. The result is
a busy loop that saturates a core for as long as the Client object is alive — and since
the loop's Task holds a strong reference to the Client, that is forever unless someone
calls disconnect().

Measured in a shipping macOS app: six MCP clients left on dead stdio connections,
599% CPU across six spinning threads, with only one server process still running.
A sample of the process shows all six threads inside
closure #1 in Client.connect(transport:)AsyncThrowingStream.Iterator.next().

The loop already has the right exit (break, in the generic catch), but it is only
reachable when the stream throws something other than
Errno.resourceTemporarilyUnavailable. A stream that finishes cleanly never gets there.

Fix

Break out of the loop when the stream finishes, the same way the error branch already
does. The resourceTemporarilyUnavailable retry — the only legitimate reason for this
loop to repeat — is untouched. The repeat is labelled so the break inside the do
block is unambiguous to the reader.

Tests

A regression test is included: Message loop stops when the transport stream finishes.

StreamFinishingTransport hands out one single-use stream and counts how many times the
client asks for it. Once the stream finishes, a client whose loop has terminated never
asks again, so the count stays at 1.

  • Against 0.12.1 without the fix: fails — the receive count is in the thousands
    after a 100 ms wait.
  • With the fix: passes.

Full suite: 552 tests in 40 suites passing (551 before this PR, all unchanged).

The branch was not covered before, which is why the bug survived: MockTransport.receive()
creates a new stream on every call and replaces the stored continuation without finishing
the previous one, so in tests the stream never finishes while the client is still
connected.

The message handling loop treats the end of the transport stream as
transient: it exits the `for try await`, calls `receive()` again and
starts over. No transport behaves that way. Every one of them exposes a
single-use stream that finishes only when the connection is over —
StdioTransport on EOF, HTTPClientTransport and InMemoryTransport on
disconnect, NetworkTransport on disconnect (its internal reconnect
reuses the same continuation without ever finishing it).

Once the stream has finished, `receive()` hands back the very same
finished stream, so the `for` returns immediately and the `while true`
starts over immediately: a busy loop that saturates a core for as long
as the client object is alive. Measured in a shipping app: six MCP
clients left on dead connections, 599% CPU, with only one server
process still running.

Break out of the loop instead, the same way the error branch already
does. The `resourceTemporarilyUnavailable` retry — the only legitimate
reason for this loop to repeat — is untouched.
The existing suite never exercised the branch: `MockTransport.receive()`
creates a new stream on every call and replaces the stored continuation
without finishing the previous one, so in tests the stream never finishes
while the client is still connected.

`StreamFinishingTransport` hands out one single-use stream and counts how
many times the client asks for it. After the stream finishes, a client
whose loop has terminated never asks again, so the count stays at 1.

Verified against 0.12.1 without the fix: the test fails with a receive
count in the thousands after a 100 ms wait.
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.

1 participant