Skip to content

Refactor transports to help enable graceful shutdown#142

Merged
halter73 merged 12 commits into
modelcontextprotocol:mainfrom
halter73:gracefulshutdown
Mar 31, 2025
Merged

Refactor transports to help enable graceful shutdown#142
halter73 merged 12 commits into
modelcontextprotocol:mainfrom
halter73:gracefulshutdown

Conversation

@halter73
Copy link
Copy Markdown
Contributor

@halter73 halter73 commented Mar 29, 2025

I took this over from #122

  • This also paves the way for better multi-session support
  • We should definitely rethink names for the transport API. But for now, I kept the names similar as possible, so we can focus on the API shape.

stephentoub and others added 5 commits March 27, 2025 18:13
@stephentoub
Enable graceful shutdown of servers
4ae26c0
@halter73
Merge branch 'main' into gracefulshutdown
2ef38ff 
# Conflicts:
#	src/ModelContextProtocol/Client/McpClient.cs
#	src/ModelContextProtocol/Protocol/Transport/StdioServerTransport.cs
@halter73
Call IMcpServer.RunAsync in AspNetCoreSseServer
fbe1dbe
@halter73
Refactor transports to enable graceful shutdown
4b7aabb 
- This also paves the way for better multi-session support
- We should definitely rethink names for the transport API
- For now, I kept the names similar as possible, so we can
  focus on the API shape
@halter73
Merge branch 'main' into gracefulshutdown
c6e9958
@halter73 halter73 mentioned this pull request Mar 29, 2025
Closed
PederHP
PederHP reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Client/McpClient.cs Outdated
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Client/McpClientFactory.cs
@stephentoub stephentoub mentioned this pull request Mar 30, 2025
Closed
halter73 added 2 commits March 30, 2025 14:04
@halter73
Address PR feedback
1d44cec 
- Use SemaphoreSlim in McpJsonRpcEndpoint
- Dispose IClientTransport on connection failure
- Test cleanup
@halter73
Fix flaky Should_Start_In_Connected_State test
cba8523
@halter73
Copy link
Copy Markdown
Contributor Author

halter73 commented Mar 30, 2025

The faster test runs after these changes are really nice.

csharp-sdk [main] > dotnet test --filter ModelContextProtocol.Tests.ClientIntegrationTests
...
Test summary: total: 41, failed: 0, succeeded: 39, skipped: 2, duration: 96.3s
csharp-sdk [main] > checkout gracefulshutdown
csharp-sdk [main] > dotnet test --filter ModelContextProtocol.Tests.ClientIntegrationTests
...
Test summary: total: 41, failed: 0, succeeded: 39, skipped: 2, duration: 37.8s

@stephentoub
Copy link
Copy Markdown
Contributor

stephentoub commented Mar 30, 2025

Yeah, those 15-30s sleeps in the tests were killer.

stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Hosting/McpServerMultiSessionHostedService.cs Outdated
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Hosting/McpServerSingleSessionHostedService.cs Outdated
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Protocol/Transport/HttpListenerSseServerTransport.cs Outdated
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Protocol/Transport/StdioClientStreamTransport.cs
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Protocol/Transport/StdioClientTransport.cs
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Server/McpServer.cs Outdated
stephentoub
stephentoub reviewed Mar 30, 2025
View reviewed changes
Comment thread src/ModelContextProtocol/Shared/McpJsonRpcEndpoint.cs
@halter73 halter73 merged commit 2ab9db0 into modelcontextprotocol:main Mar 31, 2025
8 checks passed
This was referenced Mar 31, 2025
Closed
Closed
@cristipufu
Copy link
Copy Markdown

cristipufu commented Mar 31, 2025 •
edited
Loading

@halter73 @stephentoub how should we handle multi-nodes deployments?
eg:

  • session connects to sse on node1
  • session makes tool call request on node2
  • need to send tool call updates for session on node1

@eiriktsarpalis eiriktsarpalis mentioned this pull request Mar 31, 2025
Closed
@stephentoub stephentoub mentioned this pull request Apr 1, 2025
Merged
9 tasks
@halter73 halter73 mentioned this pull request Apr 25, 2025
Closed
This was referenced Jul 24, 2025
Closed
Merged
@jeffhandley jeffhandley added the breaking-change This issue or PR introduces a breaking change label Feb 20, 2026
@jeffhandley
Copy link
Copy Markdown
Contributor

jeffhandley commented Feb 20, 2026

Adding the breaking-change label retroactively during release notes revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub stephentoub approved these changes

@eiriktsarpalis eiriktsarpalis Awaiting requested review from eiriktsarpalis

+1 more reviewer

@PederHP PederHP PederHP left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking-change This issue or PR introduces a breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@halter73 @stephentoub @cristipufu @jeffhandley @PederHP