Skip to content

Fix: Prevent session manager shutdown on individual session crash #841

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.

Sign up for GitHub

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

Merged
ihrpr merged 11 commits into from
Jul 9, 2025
Merged

Fix: Prevent session manager shutdown on individual session crash #841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
722ff23
Fix: Prevent session manager shutdown on individual session crash
soby May 29, 2025
415fa19
only remove error sessions and not those that have not been terminated
soby May 29, 2025
be47a85
lint
soby May 29, 2025
0d477c4
step down logging from error to warning
soby Jun 2, 2025
68fc9ca
step down logging for other places subject to network closures/discon…
soby Jun 4, 2025
615336a
ruff
soby Jun 15, 2025
8359301
linter differences of opinion
soby Jun 15, 2025
ec245fd
Apply review feedback
felixweinberger Jul 8, 2025
0cb1f05
Update src/mcp/server/streamable_http_manager.py
soby Jul 9, 2025
635ab04
Merge branch 'main' into fix/session-manager-resilience
soby Jul 9, 2025
46d0452
ruff
soby Jul 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ruff
  • Loading branch information
@soby @felixweinberger
soby authored and felixweinberger committed Jul 8, 2025
commit 615336a5bf4b6018506c2617d7c21b55d721de84
10 changes: 3 additions & 7 deletions src/mcp/server/streamable_http_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,21 +251,17 @@ async def run_server(*, task_status: TaskStatus[None] = anyio.TASK_STATUS_IGNORE
# Only remove from instances if not terminated
if (
http_transport.mcp_session_id
and http_transport.mcp_session_id
in self._server_instances
and http_transport.mcp_session_id in self._server_instances
and not (
hasattr(http_transport, "_terminated")
and http_transport._terminated # pyright: ignore
hasattr(http_transport, "_terminated") and http_transport._terminated # pyright: ignore
Comment thread
felixweinberger marked this conversation as resolved.
Outdated
)
):
logger.info(
"Cleaning up crashed session "
f"{http_transport.mcp_session_id} from "
"active instances."
)
del self._server_instances[
http_transport.mcp_session_id
]
del self._server_instances[http_transport.mcp_session_id]

# Assert task group is not None for type checking
assert self._task_group is not None
Expand Down
24 changes: 9 additions & 15 deletions tests/server/test_streamable_http_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ async def send(message):
with pytest.raises(RuntimeError) as excinfo:
await manager.handle_request(scope, receive, send)

assert "Task group is not initialized. Make sure to use run()." in str(
excinfo.value
)
assert "Task group is not initialized. Make sure to use run()." in str(excinfo.value)


class TestException(Exception):
Expand Down Expand Up @@ -137,12 +135,10 @@ async def mock_receive():
# Give other tasks a chance to run. This is important for the finally block.
await anyio.sleep(0.01)

assert (
session_id not in manager._server_instances
), "Session ID should be removed from _server_instances after graceful exit"
assert (
not manager._server_instances
), "No sessions should be tracked after the only session exits gracefully"
assert session_id not in manager._server_instances, (
"Session ID should be removed from _server_instances after graceful exit"
)
assert not manager._server_instances, "No sessions should be tracked after the only session exits gracefully"


@pytest.mark.anyio
Expand Down Expand Up @@ -195,9 +191,7 @@ async def mock_receive():
# Give other tasks a chance to run to ensure the finally block executes
await anyio.sleep(0.01)

assert (
session_id not in manager._server_instances
), "Session ID should be removed from _server_instances after an exception"
assert (
not manager._server_instances
), "No sessions should be tracked after the only session crashes"
assert session_id not in manager._server_instances, (
"Session ID should be removed from _server_instances after an exception"
)
assert not manager._server_instances, "No sessions should be tracked after the only session crashes"