Skip to content

fix: ensure openConnections.dispose() runs in closeGracefully even if the handshake logic throws an exception or fails.#867

Closed
noxymon wants to merge 1 commit intomodelcontextprotocol:mainfrom
noxymon:fix/gh-547
Closed

fix: ensure openConnections.dispose() runs in closeGracefully even if the handshake logic throws an exception or fails.#867
noxymon wants to merge 1 commit intomodelcontextprotocol:mainfrom
noxymon:fix/gh-547

Conversation

@noxymon
Copy link

@noxymon noxymon commented Mar 13, 2026

This PR ensures that internal connections in DefaultMcpTransportSession are always disposed of during the graceful shutdown process, even if the handshake logic throws an exception or fails.

Motivation and Context

The previous implementation of closeGracefully() would skip the connection disposal step if the onClose function emitted an error. This led to resource leaks in scenarios where the server was unresponsive or the handshaking handshake handshaked
wrong. This fix uses Reactor's doFinally to guarantee cleanup.

This PR closes #547.

How Has This Been Tested?

  • Implemented DefaultMcpTransportSessionTests.
  • Verified that openConnections.dispose() is called when the graceful handshake succeeds.
  • Verified that openConnections.dispose() is called when the graceful handshake fails with an exception.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the [MCP Documentation (https://modelcontextprotocol.io)
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Used Mono.defer and doFinally to ensure reliable execution of the cleanup logic regardless of the reactive signal type.

… onClose errors

- Updated DefaultMcpTransportSession#closeGracefully to use doFinally for connection disposal.
- Added DefaultMcpTransportSessionTests to verify the fix.
@noxymon noxymon closed this Mar 13, 2026
@noxymon
Copy link
Author

noxymon commented Mar 13, 2026

i think, the solution duplicated by #868

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DefaultMcpTransportSession.java closeGracefully may leak resources

1 participant