McpClient.DisposeAsync() Hangs indefinitely when HttpClientTransportOptions.OwnsSession is set to false

[henrydn]

Version
Issue occurs on 0.8.0-preview.1 and 0.7.0-preview.1

Describe the bug
When HttpClientTransportOptions.OwnsSession is set to false, McpClient.DisposeAsync() hangs indefinitely

To Reproduce
Steps to reproduce the behavior:

var httpClient = httpClientFactory.CreateClient("McpClient");
      
var httpTransportOptions = new HttpClientTransportOptions
{
    Endpoint = new Uri(_config.Uri),
    OwnsSession = false // set to false so that we don't delete the session when the MCP client is disposed
};

var client = await McpClient.CreateAsync(new HttpClientTransport(httpTransportOptions, httpClient), 
    new McpClientOptions(), cancellationToken: ct);

var response = await client.CallToolAsync(Name, arguments, cancellationToken: ct);

// Dispose hangs forever in StreamableHttpClientSessionTransport.DisposeAsync();
await client.DisposeAsync();

Expected behavior
I'm expecting McpClient.DisposeAsync() to return eventually

Additional context
When HttpClientTransportOptions.OwnsSession is true, DisposeAsync() doesn't hang. But then it deletes the session on the server which is not what we want

[henrydn]

A workaround for now is to set MaxReconnectionAttempts to 0. Not sure why it's needed though

var httpTransportOptions = new HttpClientTransportOptions
{
    Endpoint = new Uri(_config.Uri),
    OwnsSession = false, // set to false so that we don't delete the session when the MCP client is disposed
    MaxReconnectionAttempts = 0,
};

[halter73]

It looks like you're taking advantage of one bug to work around the other. As noted here, MaxReconnectionAttempts currently has an off by one error that prevents the GET request from being made at all when it's set to 0. Given its current behavior, you could call it MaxConnectionAttempts. [1]

As for why McpClient.DisposeAsync() hangs in the first place when OwnsSession = false, that must mean that the cancellation of the _connectionCts at the beginning of StreamableHttpClientSessionTransport.DisposeAsync() must not get respected by SendGetSseRequestWithRetriesAsync, but a quick spot check shows that we are flowing it to every async operation from the original HttpClient.SendAsync to ReadAsStreamAsync on the response body and EnumerateAsync in the parser.

csharp-sdk/src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs

Line 164 in bfd01b9

await _connectionCts.CancelAsync().ConfigureAwait(false);

I don't think there's a bug in SseParser or HttpClient that would cause it to ignore this kind of cancellation, but it's possible a DelegatingHandler could cause issues here. We already do have tests like CanResumeSessionWithMapMcpAndRunSessionHandler and DoesNotSendDeleteWhenTransportDoesNotOwnSession which show it's possible at least in some cases for DisposeAsync to complete with OwnsSession = false.

@henrydn Can you please provide a repo project that demonstrates the hang?

1: @MackinnonBuck As an aside, we might close #1178 now that we've updated the conformance tests past v0.1.11, so we might want a separate PR for MaxConnectionAttempts fix. Also, it looks like this issue might have started in v0.7.0-preview.1 when the Streamable HTTP resumability was added. And while I don't notice any issues with the code looking at it again, and Copilot seems to agree, it doesn't hurt to have another pair of eyes just in case the issue is related.

Copilot analysis

  The question is: with the default MaxReconnectionAttempts = 2, what happens? Let me think about the case
  where the server uses MapMcp() (the real server, not the test stub):

   1. Client initializes → server sends session ID
   2. ReceiveUnsolicitedMessagesAsync starts → sends GET request
   3. Server handles GET → opens SSE stream, sends priming event with event ID
   4. Priming event has no meaningful data (it's a "prime" type), so ProcessSseResponseAsync processes it
   5. The SSE stream stays open as long as the server wants to send unsolicited messages
   6. Client calls DisposeAsync() → _connectionCts.CancelAsync() → cancels the EnumerateAsync loop
   7. ProcessSseResponseAsync should throw OperationCanceledException from SseParser.EnumerateAsync

  But wait — ProcessSseResponseAsync catches IOException and HttpRequestException but NOT
  OperationCanceledException. So OperationCanceledException would propagate up through
  SendGetSseRequestWithRetriesAsync, then through ReceiveUnsolicitedMessagesAsync, and then be caught by
  the catch (OperationCanceledException) in DisposeAsync at line 179. That should work fine.