From 2fe5003981c85d2d32817f8b9168b9c2e42d5012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Yal=C3=A7=C4=B1n?= Date: Thu, 27 Aug 2026 11:12:03 +0300 Subject: [PATCH 1/3] Dispose the POST response in SseClientSessionTransport SendMessageAsync sends every message with HttpCompletionOption.ResponseHeadersRead (via McpHttpClient) but never disposed the returned HttpResponseMessage on the success path, so the underlying connection was never returned to the pool. Each JSON-RPC POST therefore opened and stranded a new connection, which was only reclaimed by GC or an idle timeout. Disposing the response returns the connection to the pool deterministically; subsequent POSTs reuse a single connection. Fixes #1840 Co-Authored-By: Claude Fable 5 --- .../Client/SseClientSessionTransport.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs index 99bdc1eb9..3d69a93ef 100644 --- a/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs @@ -90,7 +90,7 @@ public override async Task SendMessageAsync( using var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, _messageEndpoint); StreamableHttpClientSessionTransport.CopyAdditionalHeaders(httpRequestMessage.Headers, _options.AdditionalHeaders, sessionId: null, protocolVersion: null); - var response = await _httpClient.SendAsync(httpRequestMessage, message, cancellationToken).ConfigureAwait(false); + using var response = await _httpClient.SendAsync(httpRequestMessage, message, cancellationToken).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { From c8513e776cb81ad260129d015ac5585c8cd58176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Yal=C3=A7=C4=B1n?= Date: Sun, 30 Aug 2026 19:07:14 +0300 Subject: [PATCH 2/3] Add a regression test for POST response disposal SendMessageAsync_Disposes_Response_On_Success drives the SSE transport through the public HttpClientTransport ConnectAsync + SendMessageAsync path against the existing MockHttpHandler. The mocked POST response carries content that records its own disposal, and the test asserts the response has been disposed by the time SendMessageAsync returns - no sockets, no GC, no timing dependence. Fails on the parent commit (response never disposed on the success path), passes with the fix, on net10.0, net9.0, net8.0, and net472. Co-Authored-By: Claude Fable 5 --- .../Transport/HttpClientTransportTests.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs index a203797b7..9beb81631 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs @@ -147,6 +147,53 @@ public async Task SendMessageAsync_Handles_Accepted_Response() Assert.True(true); } + [Fact] + public async Task SendMessageAsync_Disposes_Response_On_Success() + { + // Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/1840 + // Every POST is sent with HttpCompletionOption.ResponseHeadersRead, so the underlying + // connection only returns to the pool once the response is consumed or disposed. The + // success path (an accepted response whose body is unused) previously did neither, + // stranding one connection per sent message until the GC finalized the response. + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(_transportOptions, httpClient, LoggerFactory); + + using var postContent = new DisposalTrackingContent("accepted"); + var firstCall = true; + mockHttpHandler.RequestHandler = (request) => + { + if (request.Method == HttpMethod.Post && request.RequestUri?.AbsoluteUri == "http://localhost:8080/sseendpoint") + { + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.Accepted, + Content = postContent + }); + } + else + { + if (!firstCall) + throw new IOException("Abort"); + else + firstCall = false; + + return Task.FromResult(new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent("event: endpoint\r\ndata: /sseendpoint\r\n\r\n") + }); + } + }; + + await using var session = await transport.ConnectAsync(TestContext.Current.CancellationToken); + await session.SendMessageAsync(new JsonRpcRequest { Method = RequestMethods.Initialize, Id = new RequestId(44) }, TestContext.Current.CancellationToken); + + Assert.True(postContent.Disposed, + "The POST response was not disposed after SendMessageAsync completed; " + + "with HttpCompletionOption.ResponseHeadersRead this strands the connection until the GC finalizes the response."); + } + [Fact] public async Task StreamableHttp_NotificationWithEmptyAcceptedJsonResponse_DoesNotLogParseFailure() { @@ -586,4 +633,15 @@ await session.SendMessageAsync( Assert.Equal("test-session", session.SessionId); } + + private sealed class DisposalTrackingContent(string content) : StringContent(content) + { + public bool Disposed { get; private set; } + + protected override void Dispose(bool disposing) + { + Disposed = true; + base.Dispose(disposing); + } + } } From 40baa3fbac37a19831f2241136f98770c3b43d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Furkan=20Yal=C3=A7=C4=B1n?= Date: Mon, 31 Aug 2026 15:24:57 +0300 Subject: [PATCH 3/3] Fix inaccurate GC-finalizer wording in regression test HttpResponseMessage and HttpContent have no finalizer, so cleanup never happens implicitly via GC finalization; only an explicit Dispose() releases the connection. Reworded the comment and assert message to say cleanup becomes nondeterministic instead of claiming GC finalizes the response. --- .../Transport/HttpClientTransportTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs index 9beb81631..1d3cdb9fa 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs @@ -154,7 +154,7 @@ public async Task SendMessageAsync_Disposes_Response_On_Success() // Every POST is sent with HttpCompletionOption.ResponseHeadersRead, so the underlying // connection only returns to the pool once the response is consumed or disposed. The // success path (an accepted response whose body is unused) previously did neither, - // stranding one connection per sent message until the GC finalized the response. + // leaving cleanup nondeterministic and potentially stranding the connection. using var mockHttpHandler = new MockHttpHandler(); using var httpClient = new HttpClient(mockHttpHandler); await using var transport = new HttpClientTransport(_transportOptions, httpClient, LoggerFactory); @@ -191,7 +191,7 @@ public async Task SendMessageAsync_Disposes_Response_On_Success() Assert.True(postContent.Disposed, "The POST response was not disposed after SendMessageAsync completed; " + - "with HttpCompletionOption.ResponseHeadersRead this strands the connection until the GC finalizes the response."); + "with HttpCompletionOption.ResponseHeadersRead this can strand the connection because cleanup is no longer deterministic."); } [Fact]