Skip to content

Commit 8dfd812

Browse files
jeffhandleyCopilot
andcommitted
Prevent shutdown hang on unanswered elicitations in client sample
Disposing SessionClientRegistry waits for each session's in-flight operation to complete. A tool call blocked in ElicitationBroker.RequestAsync never completes until a frontend posts a response, so an unanswered elicitation would stall application shutdown until the host's shutdown timeout forced the process down. - Add ElicitationBroker.CancelAll() to release every pending elicitation. - Register an ApplicationStopping callback that cancels pending elicitations before the singleton registry is disposed. - Collect progress notifications in a ConcurrentQueue, since they are reported from the MCP session's message loop while the request thread serializes the response. - Document the disposal behavior and both cancellation paths in the README. - Cover CancelAll() with a test across multiple sessions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 6a69512 commit 8dfd812

4 files changed

Lines changed: 54 additions & 2 deletions

File tree

samples/AspNetCoreMcpClient/ElicitationBroker.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,26 @@ public void Cancel(string sessionId)
7474
}
7575
}
7676

77+
/// <summary>
78+
/// Cancels every pending elicitation. Call this during application shutdown so that MCP operations blocked on an
79+
/// unanswered elicitation unblock and the session registry can dispose its clients.
80+
/// </summary>
81+
public int CancelAll()
82+
{
83+
lock (_lock)
84+
{
85+
var pending = _pending.Values.ToArray();
86+
_pending.Clear();
87+
88+
foreach (var request in pending)
89+
{
90+
request.Completion.TrySetCanceled();
91+
}
92+
93+
return pending.Length;
94+
}
95+
}
96+
7797
private sealed class PendingRequest(Guid id, ElicitRequestParams? request)
7898
{
7999
public Guid Id { get; } = id;

samples/AspNetCoreMcpClient/Program.cs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using ModelContextProtocol;
33
using ModelContextProtocol.Client;
44
using ModelContextProtocol.Protocol;
5+
using System.Collections.Concurrent;
56
using System.Text.Json;
67

78
var builder = WebApplication.CreateBuilder(args);
@@ -63,6 +64,18 @@
6364

6465
var app = builder.Build();
6566

67+
// Disposing the registry waits for each session's in-flight operation to finish. A tool call blocked on an unanswered
68+
// elicitation would never finish, so release those requests before shutdown disposes the singleton registry.
69+
app.Lifetime.ApplicationStopping.Register(() =>
70+
{
71+
var broker = app.Services.GetRequiredService<ElicitationBroker>();
72+
var canceled = broker.CancelAll();
73+
if (canceled > 0)
74+
{
75+
app.Logger.LogInformation("Canceled {Count} pending elicitations during shutdown.", canceled);
76+
}
77+
});
78+
6679
app.MapGet("/tools", async (
6780
HttpContext context,
6881
SessionClientRegistry<McpClientConnection> registry,
@@ -89,8 +102,9 @@
89102
? arguments.Value.Deserialize<Dictionary<string, object?>>()
90103
: null;
91104

92-
var progressUpdates = new List<ProgressNotificationValue>();
93-
var progress = new InlineProgress<ProgressNotificationValue>(value => progressUpdates.Add(value));
105+
// Progress notifications arrive on the MCP session's message loop, so use a thread-safe collection.
106+
var progressUpdates = new ConcurrentQueue<ProgressNotificationValue>();
107+
var progress = new InlineProgress<ProgressNotificationValue>(progressUpdates.Enqueue);
94108
var result = await registry.ExecuteAsync(
95109
sessionId,
96110
async (connection, token) => await connection.Client.CallToolAsync(

samples/AspNetCoreMcpClient/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ When the MCP server sends an elicitation request during a tool call, `Elicitatio
4848

4949
The broker intentionally permits one pending elicitation per application session because the registry serializes that session's MCP operations.
5050

51+
Because the registry waits for a session's in-flight operation before disposing its client, a tool call blocked on an unanswered elicitation would otherwise stall `DELETE /session` and application shutdown. `DELETE /session` cancels the session's pending elicitation first, and the sample registers an `ApplicationStopping` callback that cancels all pending elicitations before the registry is disposed.
52+
5153
## Production considerations
5254

5355
- **Never trust a caller-provided session header.** Replace `X-Demo-User` with a key derived from authenticated, server-side identity or session state. Do not use access tokens or other secrets as dictionary keys.

tests/AspNetCoreMcpClient.Tests/ElicitationBrokerTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,20 @@ public async Task Cancellation_RemovesPendingRequest()
3333
await Assert.ThrowsAnyAsync<OperationCanceledException>(() => requestTask);
3434
Assert.Null(broker.GetPending("alice"));
3535
}
36+
37+
[Fact]
38+
public async Task CancelAll_ReleasesEveryPendingRequest()
39+
{
40+
var broker = new ElicitationBroker();
41+
var alice = broker.RequestAsync("alice", new() { Message = "Choose" }, TestContext.Current.CancellationToken).AsTask();
42+
var bob = broker.RequestAsync("bob", new() { Message = "Choose" }, TestContext.Current.CancellationToken).AsTask();
43+
44+
Assert.Equal(2, broker.CancelAll());
45+
46+
await Assert.ThrowsAnyAsync<OperationCanceledException>(() => alice);
47+
await Assert.ThrowsAnyAsync<OperationCanceledException>(() => bob);
48+
Assert.Null(broker.GetPending("alice"));
49+
Assert.Null(broker.GetPending("bob"));
50+
Assert.Equal(0, broker.CancelAll());
51+
}
3652
}

0 commit comments

Comments
 (0)