fix: resolve resource leaks in transport session and httpclient by noxymon · Pull Request #868 · modelcontextprotocol/java-sdk

Summary

This pull request fixes #547 critical resource leaks in the MCP Java SDK. It ensures that internal connections are always disposed of during session shutdown and that HttpClient instances (and their associated SelectorManager threads) are properly
closed. Key changes include refactoring DefaultMcpTransportSession and HTTP-based transports to use doFinally for guaranteed cleanup, and implementing a robust HttpClient closure utility in Utils.

Motivation and Context

This change resolves two significant issues:

  1. Thread Leakage (HttpClient resource leak causes thread accumulation and memory exhaustion #620): Each HTTP-based transport (HttpClientStreamableHttpTransport and HttpClientSseClientTransport) was creating a new HttpClient instance but never closing it. In JDK 17+, this resulted in the accumulation of
    HttpClient-xxxx-SelectorManager threads, eventually leading to memory exhaustion and application instability.
  2. Unreliable Cleanup (DefaultMcpTransportSession.java closeGracefully may leak resources #547): In DefaultMcpTransportSession, connection disposal was being skipped if an exception occurred during the graceful shutdown handshake (onClose) with the server.

By utilizing Project Reactor's doFinally operator, we guarantee that resource cleanup occurs regardless of whether the shutdown process completes successfully, errors out, or is cancelled.

How Has This Been Tested?

  • ResourceLeakTests.java: A new functional test suite was added to verify:
    • HttpClient thread cleanup: Monitors the JVM thread list to ensure SelectorManager threads are terminated after transport closure.
    • Guaranteed connection disposal: Verifies that openConnections.dispose() is called even when the onClose logic fails with an exception.
  • DefaultMcpTransportSessionTests.java: Unit tests verifying the specific lifecycle management logic within the transport session.
  • Manual Verification: Observed thread stability during repeated client creation and destruction cycles.
  • Build Validation: Verified that all changes pass the full project test suite and adhere to the project's formatting standards via spring-javaformat:apply.

Breaking Changes

None. These are internal stability improvements that do not alter the public API or behavior beyond ensuring resource reclamation.

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

The implementation of Utils.closeHttpClient includes a reflection-based fallback to access the internal stop() method for JDK versions prior to 21 (where HttpClient does not implement AutoCloseable). This ensures that thread leakage is prevented
across the entire range of supported Java versions. For JDK 21+, it utilizes the native close() method.