| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -18,6 +18,7 @@ | |
| import mcp_types as types | ||
| from mcp_types.version import MODERN_PROTOCOL_VERSIONS | ||
|
|
||
| from mcp.shared.direct_dispatcher import DirectDispatcher | ||
| from mcp.shared.dispatcher import CallOptions | ||
| from mcp.shared.exceptions import MCPError | ||
| from mcp.shared.subscriptions import ( | ||
| Expand Down Expand Up | @@ -241,42 +242,51 @@ async def listen( | |
| data = request.model_dump(by_alias=True, mode="json", exclude_none=True) | ||
| opts: CallOptions = {"request_id": request_id} | ||
| session._stamp(data, opts) # pyright: ignore[reportPrivateUsage] | ||
| dispatcher = session._dispatcher # pyright: ignore[reportPrivateUsage] | ||
| driver_scope = anyio.CancelScope() | ||
| driver_done = anyio.Event() | ||
|
|
||
| async def drive() -> None: | ||
| # Deliberately no result timeout: the response arrives when the stream ends. | ||
| with driver_scope: | ||
| try: | ||
| await session._dispatcher.send_raw_request( # pyright: ignore[reportPrivateUsage] | ||
| data["method"], data.get("params"), opts | ||
| ) | ||
| except MCPError as error: | ||
| route.settle("lost", error=error) | ||
| return | ||
| except ValueError as error: | ||
| # A raw request id collided with our minted listen id: fail this subscription | ||
| # and release the route in this same slice, so it cannot consume the raw caller's ack. | ||
| session._unregister_listen_route(request_id) # pyright: ignore[reportPrivateUsage] | ||
| route.settle("lost", error=MCPError(types.INTERNAL_ERROR, str(error))) | ||
| return | ||
| # A result, whatever its body, is the spec's graceful close; with no prior ack | ||
| # it opens the subscription already closed. | ||
| route.set_acked(types.SubscriptionFilter()) | ||
| route.settle("graceful") | ||
| try: | ||
| with driver_scope: | ||
| try: | ||
| await dispatcher.send_raw_request(data["method"], data.get("params"), opts) | ||
| except MCPError as error: | ||
| route.settle("lost", error=error) | ||
| return | ||
| except ValueError as error: | ||
| # A raw request id collided with our minted listen id: fail this subscription | ||
| # and release the route in this same slice, so it cannot consume the raw caller's ack. | ||
| session._unregister_listen_route(request_id) # pyright: ignore[reportPrivateUsage] | ||
| route.settle("lost", error=MCPError(types.INTERNAL_ERROR, str(error))) | ||
| return | ||
| # A result, whatever its body, is the spec's graceful close; with no prior ack | ||
| # it opens the subscription already closed. | ||
| route.set_acked(types.SubscriptionFilter()) | ||
| route.settle("graceful") | ||
| finally: | ||
| driver_done.set() | ||
|
|
||
| # Register the demux route before the request is written so the ack cannot race it. | ||
| route = session._register_listen_route(request_id) # pyright: ignore[reportPrivateUsage] | ||
| try: | ||
| task_group.start_soon(drive) | ||
| with anyio.fail_after(session._session_read_timeout_seconds): # pyright: ignore[reportPrivateUsage] | ||
| await route.acked.wait() | ||
| if route.honored is None: | ||
| # Only reachable on failure paths: a graceful no-ack result acked an empty filter in drive(). | ||
| if route.error is not None: | ||
| raise route.error | ||
| raise SubscriptionLost(f"subscription {request_id!r} ended before it was acknowledged") | ||
| yield Subscription(route, request_id, route.honored, on_event) | ||
| try: | ||
| with anyio.fail_after(session._session_read_timeout_seconds): # pyright: ignore[reportPrivateUsage] | ||
| await route.acked.wait() | ||
| if route.honored is None: | ||
| # Only reachable on failure paths: a graceful no-ack result acked an empty filter in drive(). | ||
| if route.error is not None: | ||
| raise route.error | ||
| raise SubscriptionLost(f"subscription {request_id!r} ended before it was acknowledged") | ||
| yield Subscription(route, request_id, route.honored, on_event) | ||
| finally: | ||
| route.settle("local") | ||
| driver_scope.cancel() | ||
| # Only direct drivers own handler cleanup; remote courtesy writes remain session-owned. | ||
| if isinstance(dispatcher, DirectDispatcher): | ||
| with anyio.move_on_after(5, shield=True): | ||
|
Comment thread
Copy link
Copy Markdown
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityP2: When direct handler cleanup outlives this five-second scope, drive remains a child of ClientSession._task_group, so closing the session still waits indefinitely for that shielded handler. Bound or detach the outstanding driver during session shutdown as well, so the timeout actually prevents an uncooperative cleanup from blocking client exit. Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/subscriptions.py, line 289:
<comment>When direct handler cleanup outlives this five-second scope, `drive` remains a child of `ClientSession._task_group`, so closing the session still waits indefinitely for that shielded handler. Bound or detach the outstanding driver during session shutdown as well, so the timeout actually prevents an uncooperative cleanup from blocking client exit.</comment>
<file context>
@@ -241,42 +242,51 @@ async def listen(
+ driver_scope.cancel()
+ # Only direct drivers own handler cleanup; remote courtesy writes remain session-owned.
+ if isinstance(dispatcher, DirectDispatcher):
+ with anyio.move_on_after(5, shield=True):
+ await driver_done.wait()
finally:
</file context>
Sorry, something went wrong.
All reactions
|
||
| await driver_done.wait() | ||
| finally: | ||
| route.settle("local") | ||
| driver_scope.cancel() | ||
| session._unregister_listen_route(request_id) # pyright: ignore[reportPrivateUsage] | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟣 pre-existing, not blocking: Users driving an in-process server over memory streams (modern protocol, JSON-RPC framed) keep the re-listen slot race the PR fixes only for DirectDispatcher. The isinstance(dispatcher, DirectDispatcher) gate at src/mcp/client/subscriptions.py:288 skips the wait for every JSONRPCDispatcher, including in-memory ones where the handler cleanup is just as local and cheap to await. A sequential re-listen against max_subscriptions=1 can be rejected with "Subscription limit reached". Fix: gate on transport locality or on a dispatcher capability (e.g. an attribute meaning 'handler runs in-process'), not on the concrete DirectDispatcher class, so in-memory stream sessions get the same bounded wait.
Extended reasoning...A small fix can ride a push you are already making; otherwise a short reply is enough.
Population: tests and apps using in-memory JSON-RPC sessions (e.g. mcp.shared.memory helpers or ClientSession(dispatcher=JSONRPCDispatcher(...)) over anyio memory streams) with a 2026-07-28 server.
Caller exits a listen block; :285-286 settle and cancel driver_scope; :288 isinstance check is False; exit returns at once.
The courtesy notifications/cancelled is written by the drive task later; the server's ListenHandler finally at src/mcp/server/subscriptions.py:236-239 releases the slot only when the server processes that cancel.
Caller immediately re-listens; server at src/mcp/server/subscriptions.py:193-194 still counts the old stream and raises MCPError "Subscription limit reached".
The dismissing finder cited mode='legacy', which cannot listen at all (ListenNotSupportedError at :224-225), so its population statement was wrong; the real population is modern-protocol in-memory stream sessions.
Base behaved the same, but this PR is the deliberate design decision on exit semantics and picks a class check rather than a locality check.
Remedy: key the wait on an in-process…
Verification: pre-existing. Trigger: an in-memory JSON-RPC session (e.g. JSONRPCDispatcher over anyio memory streams / mcp.shared.memory.create_client_server_memory_streams) against a ListenHandler at its max_subscriptions cap, sequentially re-listening after exit, on the trio backend. Mechanism verified: src/mcp/client/subscriptions.py:285-290 settles the route, cancels driver_scope, and waits on…
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.