From 5c8d9aa7860244bb9b0e309d9a78adbe5613e4ce Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Fri, 27 Feb 2026 23:49:37 -0800 Subject: [PATCH] fix: await in-flight handlers before disposing to prevent ObjectDisposedException In stateless HTTP mode, fire-and-forget message handlers could outlive the HTTP request scope. When a connection was aborted (e.g., client timeout), ASP.NET Core disposed the request's IServiceProvider while tool handlers were still executing, causing ObjectDisposedException when accessing scoped services like DbContext. Track handler tasks in ProcessMessagesCoreAsync and await them in the finally block. Since ProcessMessagesCoreAsync is awaited by the session disposal chain (which runs inside the HTTP request handler), this keeps the request scope alive until all handlers complete. Closes #1269 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../McpSessionHandler.cs | 30 ++++- .../StatelessServerTests.cs | 106 ++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpSessionHandler.cs b/src/ModelContextProtocol.Core/McpSessionHandler.cs index b2f94fb28..f2745c1eb 100644 --- a/src/ModelContextProtocol.Core/McpSessionHandler.cs +++ b/src/ModelContextProtocol.Core/McpSessionHandler.cs @@ -173,23 +173,30 @@ public Task ProcessMessagesAsync(CancellationToken cancellationToken) private async Task ProcessMessagesCoreAsync(CancellationToken cancellationToken) { + List? pendingHandlers = null; try { await foreach (var message in _transport.MessageReader.ReadAllAsync(cancellationToken).ConfigureAwait(false)) { LogMessageRead(EndpointName, message.GetType().Name); + Task handlerTask; + // Fire and forget the message handling to avoid blocking the transport. if (message.Context?.ExecutionContext is null) { - _ = ProcessMessageAsync(); + handlerTask = ProcessMessageAsync(); } else { // Flow the execution context from the HTTP request corresponding to this message if provided. - ExecutionContext.Run(message.Context.ExecutionContext, _ => _ = ProcessMessageAsync(), null); + Task? taskFromExecutionContext = null; + ExecutionContext.Run(message.Context.ExecutionContext, _ => taskFromExecutionContext = ProcessMessageAsync(), null); + handlerTask = taskFromExecutionContext!; } + (pendingHandlers ??= []).Add(handlerTask); + async Task ProcessMessageAsync() { JsonRpcMessageWithId? messageWithId = message as JsonRpcMessageWithId; @@ -297,6 +304,25 @@ ex is OperationCanceledException && } finally { + // Wait for all in-flight message handlers to complete before disposing. + // In stateless HTTP mode, the HTTP request's IServiceProvider (and any scoped services + // like DbContext) remains alive only as long as the request handler hasn't returned. + // Since ProcessMessagesCoreAsync is awaited (indirectly) by the HTTP request handler + // via ServerRunTask, waiting here keeps the request scope alive until all handlers finish. + // This prevents ObjectDisposedException when the HTTP connection is aborted while + // tool handlers are still executing (see https://github.com/modelcontextprotocol/csharp-sdk/issues/1269). + if (pendingHandlers is not null) + { + try + { + await Task.WhenAll(pendingHandlers).ConfigureAwait(false); + } + catch + { + // Exceptions from handlers are already logged individually in ProcessMessageAsync. + } + } + // Fail any pending requests, as they'll never be satisfied. foreach (var entry in _pendingRequests) { diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/StatelessServerTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/StatelessServerTests.cs index 0f7c7e7e0..2a2cb6967 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/StatelessServerTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/StatelessServerTests.cs @@ -189,6 +189,78 @@ public async Task ScopedServices_Resolve_FromRequestScope() Assert.Equal("From request middleware!", Assert.IsType(toolContent).Text); } + [Fact] + public async Task ScopedServices_AccessibleInToolHandler_AfterConnectionAbort() + { + // Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/1269 + // Verifies that scoped services (like DbContext) remain accessible in tool handlers + // even when the HTTP connection is aborted before the handler completes. + var abortTestState = new AbortTestState(); + + Builder.Services.AddMcpServer(mcpServerOptions => + { + mcpServerOptions.ServerInfo = new Implementation + { + Name = nameof(StatelessServerTests), + Version = "73", + }; + }) + .WithHttpTransport(httpServerTransportOptions => + { + httpServerTransportOptions.Stateless = true; + }) + .WithTools(); + + Builder.Services.AddScoped(); + Builder.Services.AddSingleton(abortTestState); + + _app = Builder.Build(); + + _app.Use(next => + { + return context => + { + context.RequestServices.GetRequiredService().State = "From request middleware!"; + return next(context); + }; + }); + + _app.MapMcp(); + + await _app.StartAsync(TestContext.Current.CancellationToken); + + HttpClient.DefaultRequestHeaders.Accept.Add(new("application/json")); + HttpClient.DefaultRequestHeaders.Accept.Add(new("text/event-stream")); + + await using var client = await ConnectMcpClientAsync(); + + using var cts = new CancellationTokenSource(); + + // Start the tool call - it will block in the handler until we release ContinueToolExecution. + var callTask = client.CallToolAsync("testAbortedConnectionScope", cancellationToken: cts.Token); + + // Wait for the handler to start executing. + await abortTestState.ToolStarted.WaitAsync(TestContext.Current.CancellationToken); + + // Abort the connection by cancelling the token. This triggers context.RequestAborted + // on the server, which starts the session disposal chain. + await cts.CancelAsync(); + + // Let the handler continue - it will now try to access the scoped service. + // Before the fix, this would throw ObjectDisposedException because the request scope + // was disposed when the HTTP connection was aborted. + abortTestState.ContinueToolExecution.Release(); + + // Verify through the side channel that the handler accessed the scoped service + // without ObjectDisposedException. The client call itself was aborted, so we can't + // rely on the return value. + var result = await abortTestState.ScopeAccessResult.Task.WaitAsync(TestContext.Current.CancellationToken); + Assert.Equal("From request middleware!", result); + + // Clean up the aborted client call. + try { await callTask; } catch { } + } + [McpServerTool(Name = "testSamplingErrors")] public static async Task TestSamplingErrors(McpServer server) { @@ -249,6 +321,40 @@ public static async Task TestElicitationErrors(McpServer server) [McpServerTool(Name = "testScope")] public static string? TestScope(ScopedService scopedService) => scopedService.State; + [McpServerTool(Name = "testAbortedConnectionScope")] + public static async Task TestAbortedConnectionScope(ScopedService scopedService, AbortTestState abortTestState) + { + // Signal the test that the handler has started. + abortTestState.ToolStarted.Release(); + + // Wait for the test to abort the connection. Use CancellationToken.None so this + // handler continues executing even after the HTTP request is aborted. + await abortTestState.ContinueToolExecution.WaitAsync(CancellationToken.None); + + try + { + // Access the scoped service AFTER the connection was aborted. + // Before the fix for https://github.com/modelcontextprotocol/csharp-sdk/issues/1269, + // this would throw ObjectDisposedException because ASP.NET Core disposed + // the request's IServiceProvider while the handler was still executing. + var result = scopedService.State; + abortTestState.ScopeAccessResult.TrySetResult(result); + return result; + } + catch (Exception ex) + { + abortTestState.ScopeAccessResult.TrySetException(ex); + throw; + } + } + + public class AbortTestState + { + public SemaphoreSlim ToolStarted { get; } = new(0, 1); + public SemaphoreSlim ContinueToolExecution { get; } = new(0, 1); + public TaskCompletionSource ScopeAccessResult { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + public class ScopedService { public string? State { get; set; }