Skip to content

Commit 6eddb34

Browse files
committed
Isolate a raising bus unsubscribe from listen stream cleanup
The unsubscribe callable returned by a custom SubscriptionBus runs first in the stream's finally block; if it raised, the stream's slot release and buffer closes were skipped, permanently consuming a max_subscriptions slot. Run it through a logged isolation boundary, mirroring the listener isolation publish already does.
1 parent 05fb2b6 commit 6eddb34

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

src/mcp/server/subscriptions.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,19 @@ def unsubscribe() -> None:
141141
return unsubscribe
142142

143143

144+
def _safe_unsubscribe(unsubscribe: Callable[[], None]) -> None:
145+
"""Run a bus's unsubscribe callable, isolating the stream from it raising.
146+
147+
The callable comes from a custom `SubscriptionBus`; a raising one is
148+
logged and skipped so it cannot stop the stream's own cleanup from
149+
releasing its subscription slot.
150+
"""
151+
try:
152+
unsubscribe()
153+
except Exception: # fan-out boundary: a raising bus must not skip stream cleanup
154+
logger.exception("bus unsubscribe raised; continuing stream cleanup")
155+
156+
144157
def _honored_subset(requested: SubscriptionFilter) -> SubscriptionFilter:
145158
"""The subset of `requested` the server will deliver, for the ack.
146159
@@ -263,7 +276,7 @@ def deliver(event: ServerEvent) -> None:
263276
_event_to_notification(event, meta), related_request_id=subscription_id
264277
)
265278
finally:
266-
unsubscribe()
279+
_safe_unsubscribe(unsubscribe)
267280
self._streams.discard(send)
268281
send.close()
269282
recv.close()

tests/server/test_subscriptions.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,53 @@ def bad(event: ServerEvent) -> None:
314314
assert seen == [ToolsListChanged()]
315315

316316

317+
@pytest.mark.anyio
318+
async def test_raising_unsubscribe_does_not_skip_stream_cleanup() -> None:
319+
"""SDK-defined: a custom bus whose unsubscribe callable raises is logged
320+
and isolated - the stream still releases its subscription slot, closes its
321+
buffers, and returns the graceful result."""
322+
323+
class _RaisingUnsubscribeBus(InMemorySubscriptionBus):
324+
def subscribe(self, listener: Callable[[ServerEvent], None]) -> Callable[[], None]:
325+
super().subscribe(listener)
326+
327+
def unsubscribe() -> None:
328+
raise RuntimeError("boom")
329+
330+
return unsubscribe
331+
332+
handler = ListenHandler(_RaisingUnsubscribeBus(), max_subscriptions=1)
333+
session = _RecordingSession()
334+
results: list[SubscriptionsListenResult] = []
335+
336+
async with anyio.create_task_group() as tg:
337+
338+
async def run() -> None:
339+
results.append(await handler(_ctx(session), _params(tools_list_changed=True)))
340+
341+
tg.start_soon(run)
342+
await session.wait_for(1)
343+
handler.close()
344+
345+
assert results[0].meta == {SUBSCRIPTION_ID_META_KEY: 7} # the graceful result still returned
346+
347+
# The slot was released despite the raising unsubscribe: a second listen
348+
# is accepted at the cap of one.
349+
late_session = _RecordingSession()
350+
late_results: list[SubscriptionsListenResult] = []
351+
352+
async with anyio.create_task_group() as tg:
353+
354+
async def run_late() -> None:
355+
late_results.append(await handler(_ctx(late_session, request_id=8), _params(tools_list_changed=True)))
356+
357+
tg.start_soon(run_late)
358+
await late_session.wait_for(1)
359+
handler.close()
360+
361+
assert late_results[0].meta == {SUBSCRIPTION_ID_META_KEY: 8}
362+
363+
317364
@pytest.mark.anyio
318365
async def test_subscription_limit_rejects_further_streams_pre_ack() -> None:
319366
"""SDK-defined cap (mirrors the TypeScript SDK): past `max_subscriptions`,

0 commit comments

Comments
 (0)