-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix: memory leak and exit problems #586
New issue
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
base: main
Are you sure you want to change the base?
Changes from all commits
dfa7c6a
1917066
9cd4572
fbe6b06
0090583
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -32,6 +32,7 @@ async def handle_sse(request): | |||||
""" | ||||||
|
||||||
import logging | ||||||
from collections.abc import Callable | ||||||
from contextlib import asynccontextmanager | ||||||
from typing import Any | ||||||
from urllib.parse import quote | ||||||
|
@@ -43,7 +44,7 @@ async def handle_sse(request): | |||||
from sse_starlette import EventSourceResponse | ||||||
from starlette.requests import Request | ||||||
from starlette.responses import Response | ||||||
from starlette.types import Receive, Scope, Send | ||||||
from starlette.types import Message, Receive, Scope, Send | ||||||
|
||||||
import mcp.types as types | ||||||
|
||||||
|
@@ -79,7 +80,13 @@ def __init__(self, endpoint: str) -> None: | |||||
logger.debug(f"SseServerTransport initialized with endpoint: {endpoint}") | ||||||
|
||||||
@asynccontextmanager | ||||||
async def connect_sse(self, scope: Scope, receive: Receive, send: Send): | ||||||
async def connect_sse( | ||||||
self, | ||||||
scope: Scope, | ||||||
receive: Receive, | ||||||
send: Send, | ||||||
callback: Callable[[], None] | None = None, | ||||||
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. The callback function only is called when client connection close so the naming is not full-meaning . And I don't think it should callback here because connect_sse yield |
||||||
): | ||||||
if scope["type"] != "http": | ||||||
logger.error("connect_sse received non-HTTP request") | ||||||
raise ValueError("connect_sse can only handle HTTP requests") | ||||||
|
@@ -120,9 +127,19 @@ async def sse_writer(): | |||||
} | ||||||
) | ||||||
|
||||||
async def client_close_handler(message: Message) -> None: | ||||||
await read_stream_writer.aclose() | ||||||
await write_stream_reader.aclose() | ||||||
del self._read_stream_writers[session_id] | ||||||
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.
Suggested change
|
||||||
if callback: | ||||||
callback() | ||||||
logger.debug(f"Closed SSE session with ID: {session_id}") | ||||||
|
||||||
async with anyio.create_task_group() as tg: | ||||||
response = EventSourceResponse( | ||||||
content=sse_stream_reader, data_sender_callable=sse_writer | ||||||
content=sse_stream_reader, | ||||||
data_sender_callable=sse_writer, | ||||||
client_close_handler_callable=client_close_handler, # type: ignore[arg-type] | ||||||
) | ||||||
logger.debug("Starting SSE response task") | ||||||
tg.start_soon(response, scope, receive, send) | ||||||
|
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.
Is this needed?
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.
Yes,
sse-starlette
supportsclient_close_handler_callable
since version 2.3.0.