Skip to content

Commit b4c7ded

Browse files
halter73halter73
authored
Add tests to verify McpClient.DisposeAsync doesn't hang (#1252)
1 parent 27b5eb8 commit b4c7ded

2 files changed

Lines changed: 224 additions & 0 deletions

File tree

‎tests/ModelContextProtocol.AspNetCore.Tests/MapMcpStreamableHttpTests.cs‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,139 @@ public async Task AdditionalHeaders_AreSent_InPostAndDeleteRequests()
412412
Assert.True(wasPostRequest, "POST request was not made");
413413
Assert.True(wasDeleteRequest, "DELETE request was not made");
414414
}
415+
416+
[Fact]
417+
public async Task DisposeAsync_DoesNotHang_WhenOwnsSessionIsFalse()
418+
{
419+
Assert.SkipWhen(Stateless, "Stateless mode doesn't support session management.");
420+
421+
var getResponseStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
422+
423+
Builder.Services.AddMcpServer().WithHttpTransport(ConfigureStateless).WithTools<ClaimsPrincipalTools>();
424+
425+
await using var app = Builder.Build();
426+
427+
// Track when the GET SSE response starts being written, which indicates
428+
// the server's HandleGetRequestAsync has fully initialized the SSE writer.
429+
app.Use(next =>
430+
{
431+
return async context =>
432+
{
433+
if (context.Request.Method == HttpMethods.Get)
434+
{
435+
context.Response.OnStarting(() =>
436+
{
437+
getResponseStarted.TrySetResult();
438+
return Task.CompletedTask;
439+
});
440+
}
441+
await next(context);
442+
};
443+
});
444+
445+
app.MapMcp();
446+
await app.StartAsync(TestContext.Current.CancellationToken);
447+
448+
await using var transport = new HttpClientTransport(new()
449+
{
450+
Endpoint = new("http://localhost:5000/"),
451+
TransportMode = HttpTransportMode.StreamableHttp,
452+
OwnsSession = false,
453+
}, HttpClient, LoggerFactory);
454+
455+
var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
456+
457+
// Call a tool to ensure the session is fully established
458+
var result = await client.CallToolAsync(
459+
"echo_claims_principal",
460+
new Dictionary<string, object?>() { ["message"] = "Hello!" },
461+
cancellationToken: TestContext.Current.CancellationToken);
462+
463+
Assert.NotNull(result);
464+
465+
// Wait for the GET SSE stream to be fully established on the server
466+
await getResponseStarted.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
467+
468+
// This should not hang. The issue reports that DisposeAsync hangs indefinitely
469+
// when OwnsSession is false. Use a timeout to detect the hang.
470+
await client.DisposeAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
471+
}
472+
473+
[Fact]
474+
public async Task DisposeAsync_DoesNotHang_WhenOwnsSessionIsFalse_WithUnsolicitedMessages()
475+
{
476+
Assert.SkipWhen(Stateless, "Stateless mode doesn't support session management.");
477+
478+
var getResponseStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
479+
var serverTcs = new TaskCompletionSource<McpServer>(TaskCreationOptions.RunContinuationsAsynchronously);
480+
481+
Builder.Services.AddMcpServer().WithHttpTransport(opts =>
482+
{
483+
ConfigureStateless(opts);
484+
opts.RunSessionHandler = async (context, server, cancellationToken) =>
485+
{
486+
serverTcs.TrySetResult(server);
487+
await server.RunAsync(cancellationToken);
488+
};
489+
}).WithTools<ClaimsPrincipalTools>();
490+
491+
await using var app = Builder.Build();
492+
493+
// Track when the GET SSE response starts being written, which indicates
494+
// the server's HandleGetRequestAsync has fully initialized the SSE writer.
495+
app.Use(next =>
496+
{
497+
return async context =>
498+
{
499+
if (context.Request.Method == HttpMethods.Get)
500+
{
501+
context.Response.OnStarting(() =>
502+
{
503+
getResponseStarted.TrySetResult();
504+
return Task.CompletedTask;
505+
});
506+
}
507+
await next(context);
508+
};
509+
});
510+
511+
app.MapMcp();
512+
await app.StartAsync(TestContext.Current.CancellationToken);
513+
514+
await using var transport = new HttpClientTransport(new()
515+
{
516+
Endpoint = new("http://localhost:5000/"),
517+
TransportMode = HttpTransportMode.StreamableHttp,
518+
OwnsSession = false,
519+
}, HttpClient, LoggerFactory);
520+
521+
var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
522+
523+
var result = await client.CallToolAsync(
524+
"echo_claims_principal",
525+
new Dictionary<string, object?>() { ["message"] = "Hello!" },
526+
cancellationToken: TestContext.Current.CancellationToken);
527+
Assert.NotNull(result);
528+
529+
// Wait for the GET SSE stream to be fully established on the server
530+
await getResponseStarted.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
531+
532+
// Register a handler on the client to detect when the notification is received
533+
var notificationReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
534+
await using var handlerRegistration = client.RegisterNotificationHandler("notifications/tools/list_changed", (notification, ct) =>
535+
{
536+
notificationReceived.TrySetResult();
537+
return default;
538+
});
539+
540+
// Get the server instance and send an unsolicited notification by modifying tools
541+
var server = await serverTcs.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
542+
await server.SendNotificationAsync("notifications/tools/list_changed", TestContext.Current.CancellationToken);
543+
544+
// Wait for the client to actually receive the notification
545+
await notificationReceived.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
546+
547+
// Dispose should still not hang
548+
await client.DisposeAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
549+
}
415550
}

‎tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpClientConformanceTests.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,95 @@ public async Task CreateAsyncWithKnownSessionIdThrows()
281281
Assert.Contains(nameof(McpClient.ResumeSessionAsync), exception.Message);
282282
}
283283

284+
[Fact]
285+
public async Task DisposeAsync_DoesNotHang_WhenOwnsSessionIsFalse_WithActiveGetStream()
286+
{
287+
var getRequestReceived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
288+
289+
Builder.Services.Configure<JsonOptions>(options =>
290+
{
291+
options.SerializerOptions.TypeInfoResolverChain.Add(McpJsonUtilities.DefaultOptions.TypeInfoResolver!);
292+
});
293+
_app = Builder.Build();
294+
295+
var echoTool = McpServerTool.Create(Echo, new() { Services = _app.Services });
296+
297+
_app.MapPost("/mcp", (JsonRpcMessage message, HttpContext context) =>
298+
{
299+
if (message is not JsonRpcRequest request)
300+
{
301+
return Results.Accepted();
302+
}
303+
304+
context.Response.Headers.Append("mcp-session-id", "hang-test-session");
305+
306+
if (request.Method == "initialize")
307+
{
308+
return Results.Json(new JsonRpcResponse
309+
{
310+
Id = request.Id,
311+
Result = JsonSerializer.SerializeToNode(new InitializeResult
312+
{
313+
ProtocolVersion = "2024-11-05",
314+
Capabilities = new() { Tools = new() },
315+
ServerInfo = new Implementation { Name = "hang-test", Version = "0.0.1" },
316+
}, McpJsonUtilities.DefaultOptions)
317+
});
318+
}
319+
320+
if (request.Method == "tools/list")
321+
{
322+
return Results.Json(new JsonRpcResponse
323+
{
324+
Id = request.Id,
325+
Result = JsonSerializer.SerializeToNode(new ListToolsResult
326+
{
327+
Tools = [echoTool.ProtocolTool]
328+
}, McpJsonUtilities.DefaultOptions),
329+
});
330+
}
331+
332+
return Results.Accepted();
333+
});
334+
335+
// GET handler that keeps the SSE stream open indefinitely (like a real MCP server)
336+
_app.MapGet("/mcp", async context =>
337+
{
338+
context.Response.Headers.ContentType = "text/event-stream";
339+
getRequestReceived.TrySetResult();
340+
await context.Response.Body.FlushAsync(TestContext.Current.CancellationToken);
341+
342+
try
343+
{
344+
await Task.Delay(Timeout.Infinite, context.RequestAborted);
345+
}
346+
catch (OperationCanceledException)
347+
{
348+
}
349+
});
350+
351+
await _app.StartAsync(TestContext.Current.CancellationToken);
352+
353+
await using var transport = new HttpClientTransport(new()
354+
{
355+
Endpoint = new("http://localhost:5000/mcp"),
356+
TransportMode = HttpTransportMode.StreamableHttp,
357+
OwnsSession = false,
358+
}, HttpClient, LoggerFactory);
359+
360+
await using (var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken))
361+
{
362+
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
363+
Assert.Single(tools);
364+
365+
// Wait for the GET SSE stream to be established on the server
366+
await getRequestReceived.Task.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
367+
368+
// Dispose should not hang even though the GET stream is actively open
369+
await client.DisposeAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
370+
}
371+
}
372+
284373
private static async Task CallEchoAndValidateAsync(McpClientTool echoTool)
285374
{
286375
var response = await echoTool.CallAsync(new Dictionary<string, object?>() { ["message"] = "Hello world!" }, cancellationToken: TestContext.Current.CancellationToken);

0 commit comments

Comments
 (0)