From 69a22a299e91d8224f5971598959f55347b538ab Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Thu, 19 Mar 2026 10:46:53 -0700 Subject: [PATCH 01/13] Fix process crash when testing Stderr --- .../Client/StdioClientTransport.cs | 25 +++++++++++++------ .../Transport/StdioClientTransportTests.cs | 11 ++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 24a47613b..5ae5011b4 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -151,7 +151,17 @@ public async Task ConnectAsync(CancellationToken cancellationToken = stderrRollingLog.Enqueue(data); } - _options.StandardErrorLines?.Invoke(data); + try + { + _options.StandardErrorLines?.Invoke(data); + } + catch (Exception ex) + { + // Prevent exceptions in the user callback from propagating + // to the background thread that dispatches ErrorDataReceived, + // which would crash the process. + LogStderrCallbackFailed(logger, endpointName, ex); + } LogReadStderr(logger, endpointName, data); } @@ -230,12 +240,10 @@ internal static void DisposeProcess( // Ensure all redirected stderr/stdout events have been dispatched // before disposing. Only the no-arg WaitForExit() guarantees this; - // WaitForExit(int) (as used by KillTree) does not. - // This should not hang: either the process already exited on its own - // (no child processes holding handles), or KillTree killed the entire - // process tree. If it does take too long, the test infrastructure's - // own timeout will catch it. - if (!processRunning && HasExited(process)) + // WaitForExit(int) (as used by KillTree) does not. We must call + // this whether the process exited on its own or was killed above, + // otherwise ErrorDataReceived handlers may fire after Dispose(). + if (HasExited(process)) { process.WaitForExit(); } @@ -299,6 +307,9 @@ private static string EscapeArgumentString(string argument) => [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} received stderr log: '{Data}'.")] private static partial void LogReadStderr(ILogger logger, string endpointName, string data); + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} StandardErrorLines callback failed.")] + private static partial void LogStderrCallbackFailed(ILogger logger, string endpointName, Exception exception); + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} started server process with PID {ProcessId}.")] private static partial void LogTransportProcessStarted(ILogger logger, string endpointName, int processId); diff --git a/tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs index d84ea9377..1a999fd14 100644 --- a/tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs @@ -59,6 +59,17 @@ public async Task CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked() Assert.Contains(id, sb.ToString()); } + [Fact(Skip = "Platform not supported by this test.", SkipUnless = nameof(IsStdErrCallbackSupported))] + public async Task CreateAsync_StdErrCallbackThrows_DoesNotCrashProcess() + { + StdioClientTransport transport = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + new(new() { Command = "cmd", Arguments = ["/c", "echo fail >&2 & exit /b 1"], StandardErrorLines = _ => throw new InvalidOperationException("boom") }, LoggerFactory) : + new(new() { Command = "sh", Arguments = ["-c", "echo fail >&2; exit 1"], StandardErrorLines = _ => throw new InvalidOperationException("boom") }, LoggerFactory); + + // Should throw IOException for the failed server, not crash the host process. + await Assert.ThrowsAnyAsync(() => McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + } + [Theory] [InlineData(null)] [InlineData("argument with spaces")] From ee120f6f810f4e9077e489c8dbea8f610d3a297f Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Thu, 19 Mar 2026 19:28:14 -0700 Subject: [PATCH 02/13] Clarify comment. --- .../Client/StdioClientTransport.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 5ae5011b4..d7919934c 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -238,11 +238,12 @@ internal static void DisposeProcess( process.KillTree(shutdownTimeout); } - // Ensure all redirected stderr/stdout events have been dispatched - // before disposing. Only the no-arg WaitForExit() guarantees this; - // WaitForExit(int) (as used by KillTree) does not. We must call - // this whether the process exited on its own or was killed above, - // otherwise ErrorDataReceived handlers may fire after Dispose(). + // When a process has exited either on it's own or via KillTree, we must + // call WaitForExit() to ensure all redirected stderr/stdout events have + // been dispatched, otherwise ErrorDataReceived handlers may fire after + // Dispose(). + // The HasExited check is here to avoid waiting forever on a process that + // failed to be killed. if (HasExited(process)) { process.WaitForExit(); From 2539d5ca6bd598681e58855a2af72e380b757940 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Fri, 20 Mar 2026 18:01:00 -0700 Subject: [PATCH 03/13] Fix additional crashes identified in dumps. --- .../ClientConformanceTests.cs | 10 +- .../ServerConformanceTests.cs | 10 +- .../Program.cs | 145 ++++++++++-------- 3 files changed, 94 insertions(+), 71 deletions(-) diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs index 1418574a7..29c5302f5 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs @@ -82,11 +82,13 @@ public async Task RunConformanceTest(string scenario) var process = new Process { StartInfo = startInfo }; + // Protect callbacks with try/catch to prevent ITestOutputHelper from + // throwing on a background thread if events arrive after the test completes. process.OutputDataReceived += (sender, e) => { if (e.Data != null) { - _output.WriteLine(e.Data); + try { _output.WriteLine(e.Data); } catch { } outputBuilder.AppendLine(e.Data); } }; @@ -95,7 +97,7 @@ public async Task RunConformanceTest(string scenario) { if (e.Data != null) { - _output.WriteLine(e.Data); + try { _output.WriteLine(e.Data); } catch { } errorBuilder.AppendLine(e.Data); } }; @@ -119,6 +121,10 @@ public async Task RunConformanceTest(string scenario) ); } + // Ensure all redirected stdout/stderr events have been dispatched before + // the test completes and ITestOutputHelper becomes invalid. + process.WaitForExit(); + var output = outputBuilder.ToString(); var error = errorBuilder.ToString(); var success = process.ExitCode == 0 || HasOnlyWarnings(output, error); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs index d2501456c..e6607b803 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs @@ -136,11 +136,13 @@ public async Task RunPendingConformanceTest_ServerSsePolling() var process = new Process { StartInfo = startInfo }; + // Protect callbacks with try/catch to prevent ITestOutputHelper from + // throwing on a background thread if events arrive after the test completes. process.OutputDataReceived += (sender, e) => { if (e.Data != null) { - output.WriteLine(e.Data); + try { output.WriteLine(e.Data); } catch { } outputBuilder.AppendLine(e.Data); } }; @@ -149,7 +151,7 @@ public async Task RunPendingConformanceTest_ServerSsePolling() { if (e.Data != null) { - output.WriteLine(e.Data); + try { output.WriteLine(e.Data); } catch { } errorBuilder.AppendLine(e.Data); } }; @@ -173,6 +175,10 @@ public async Task RunPendingConformanceTest_ServerSsePolling() ); } + // Ensure all redirected stdout/stderr events have been dispatched before + // the test completes and ITestOutputHelper becomes invalid. + process.WaitForExit(); + return ( Success: process.ExitCode == 0, Output: outputBuilder.ToString(), diff --git a/tests/ModelContextProtocol.ConformanceClient/Program.cs b/tests/ModelContextProtocol.ConformanceClient/Program.cs index 40dc424cb..b5e048dd0 100644 --- a/tests/ModelContextProtocol.ConformanceClient/Program.cs +++ b/tests/ModelContextProtocol.ConformanceClient/Program.cs @@ -105,85 +105,96 @@ OAuth = oauthOptions, }, loggerFactory: consoleLoggerFactory); -await using var mcpClient = await McpClient.CreateAsync(clientTransport, options, loggerFactory: consoleLoggerFactory); +try +{ + await using var mcpClient = await McpClient.CreateAsync(clientTransport, options, loggerFactory: consoleLoggerFactory); -bool success = true; + bool success = true; -switch (scenario) -{ - case "tools_call": + switch (scenario) { - var tools = await mcpClient.ListToolsAsync(); - Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); + case "tools_call": + { + var tools = await mcpClient.ListToolsAsync(); + Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); - // Call the "add_numbers" tool - var toolName = "add_numbers"; - Console.WriteLine($"Calling tool: {toolName}"); - var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary + // Call the "add_numbers" tool + var toolName = "add_numbers"; + Console.WriteLine($"Calling tool: {toolName}"); + var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary + { + { "a", 5 }, + { "b", 10 } + }); + success &= !(result.IsError == true); + break; + } + case "elicitation-sep1034-client-defaults": { - { "a", 5 }, - { "b", 10 } - }); - success &= !(result.IsError == true); - break; - } - case "elicitation-sep1034-client-defaults": - { - var tools = await mcpClient.ListToolsAsync(); - Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); - var toolName = "test_client_elicitation_defaults"; - Console.WriteLine($"Calling tool: {toolName}"); - var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary()); - success &= !(result.IsError == true); - break; - } - case "sse-retry": - { - var tools = await mcpClient.ListToolsAsync(); - Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); - var toolName = "test_reconnection"; - Console.WriteLine($"Calling tool: {toolName}"); - var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary()); - success &= !(result.IsError == true); - break; - } - case "auth/scope-step-up": - { - // Just testing that we can authenticate and list tools - var tools = await mcpClient.ListToolsAsync(); - Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); - - // Call the "test_tool" tool - var toolName = "test-tool"; - Console.WriteLine($"Calling tool: {toolName}"); - var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary + var tools = await mcpClient.ListToolsAsync(); + Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); + var toolName = "test_client_elicitation_defaults"; + Console.WriteLine($"Calling tool: {toolName}"); + var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary()); + success &= !(result.IsError == true); + break; + } + case "sse-retry": { - { "foo", "bar" }, - }); - success &= !(result.IsError == true); - break; - } - case "auth/scope-retry-limit": - { - // Try to list tools - this triggers the auth flow that always fails with 403. - // The test validates the client doesn't retry indefinitely. - try + var tools = await mcpClient.ListToolsAsync(); + Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); + var toolName = "test_reconnection"; + Console.WriteLine($"Calling tool: {toolName}"); + var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary()); + success &= !(result.IsError == true); + break; + } + case "auth/scope-step-up": { - await mcpClient.ListToolsAsync(); + // Just testing that we can authenticate and list tools + var tools = await mcpClient.ListToolsAsync(); + Console.WriteLine($"Available tools: {string.Join(", ", tools.Select(t => t.Name))}"); + + // Call the "test_tool" tool + var toolName = "test-tool"; + Console.WriteLine($"Calling tool: {toolName}"); + var result = await mcpClient.CallToolAsync(toolName: toolName, arguments: new Dictionary + { + { "foo", "bar" }, + }); + success &= !(result.IsError == true); + break; } - catch (Exception ex) + case "auth/scope-retry-limit": { - Console.WriteLine($"Expected auth failure: {ex.Message}"); + // Try to list tools - this triggers the auth flow that always fails with 403. + // The test validates the client doesn't retry indefinitely. + try + { + await mcpClient.ListToolsAsync(); + } + catch (Exception ex) + { + Console.WriteLine($"Expected auth failure: {ex.Message}"); + } + break; } - break; + default: + // No extra processing for other scenarios + break; } - default: - // No extra processing for other scenarios - break; -} -// Exit code 0 on success, 1 on failure -return success ? 0 : 1; + // Exit code 0 on success, 1 on failure + return success ? 0 : 1; +} +catch (Exception ex) +{ + // Report the error to stderr and exit with a non-zero code rather than + // crashing the process with an unhandled exception. An unhandled exception + // generates a crash dump which can abort the parent test host. + Console.Error.WriteLine($"Conformance client failed: {ex}"); + return 1; +} // Copied from ProtectedMcpClient sample // Simulate a user opening the browser and logging in From 82623d16827f5acc56c663d6b6c16515b568d81b Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Mon, 23 Mar 2026 12:16:26 -0700 Subject: [PATCH 04/13] Undo WaitForExit changes --- .../Client/StdioClientTransport.cs | 15 ++++++++------- .../ClientConformanceTests.cs | 4 ---- .../ServerConformanceTests.cs | 4 ---- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index d7919934c..9d7ca23b0 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -238,13 +238,14 @@ internal static void DisposeProcess( process.KillTree(shutdownTimeout); } - // When a process has exited either on it's own or via KillTree, we must - // call WaitForExit() to ensure all redirected stderr/stdout events have - // been dispatched, otherwise ErrorDataReceived handlers may fire after - // Dispose(). - // The HasExited check is here to avoid waiting forever on a process that - // failed to be killed. - if (HasExited(process)) + // When the process exited on its own, call WaitForExit() to flush + // any remaining ErrorDataReceived/OutputDataReceived events. + // We skip this after KillTree because the parameterless WaitForExit() + // blocks until all redirected output pipe handles are closed, which + // hangs when grandchild processes (e.g. from cmd.exe /c npx) inherited + // the handles. Late-firing events after KillTree are safe because the + // ErrorDataReceived handler is protected with try/catch. + if (!processRunning && HasExited(process)) { process.WaitForExit(); } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs index 29c5302f5..f5cc37561 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs @@ -121,10 +121,6 @@ public async Task RunConformanceTest(string scenario) ); } - // Ensure all redirected stdout/stderr events have been dispatched before - // the test completes and ITestOutputHelper becomes invalid. - process.WaitForExit(); - var output = outputBuilder.ToString(); var error = errorBuilder.ToString(); var success = process.ExitCode == 0 || HasOnlyWarnings(output, error); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs index e6607b803..c79336714 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs @@ -175,10 +175,6 @@ public async Task RunPendingConformanceTest_ServerSsePolling() ); } - // Ensure all redirected stdout/stderr events have been dispatched before - // the test completes and ITestOutputHelper becomes invalid. - process.WaitForExit(); - return ( Success: process.ExitCode == 0, Output: outputBuilder.ToString(), From db2baaa414d067b16e0a8b737ed73281dfadcefe Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Mon, 23 Mar 2026 19:02:29 -0700 Subject: [PATCH 05/13] Detach error handlers before disposing process. --- .../Client/StdioClientSessionTransport.cs | 14 +++++++----- .../Client/StdioClientTransport.cs | 22 +++++++------------ .../ClientConformanceTests.cs | 12 ++++++++-- .../ServerConformanceTests.cs | 12 ++++++++-- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs index a92093246..37344d2e7 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs @@ -10,15 +10,17 @@ internal sealed class StdioClientSessionTransport : StreamClientSessionTransport private readonly StdioClientTransportOptions _options; private readonly Process _process; private readonly Queue _stderrRollingLog; + private readonly DataReceivedEventHandler _errorHandler; private int _cleanedUp = 0; private readonly int? _processId; - public StdioClientSessionTransport(StdioClientTransportOptions options, Process process, string endpointName, Queue stderrRollingLog, ILoggerFactory? loggerFactory) : + public StdioClientSessionTransport(StdioClientTransportOptions options, Process process, string endpointName, Queue stderrRollingLog, DataReceivedEventHandler errorHandler, ILoggerFactory? loggerFactory) : base(process.StandardInput.BaseStream, process.StandardOutput.BaseStream, encoding: null, endpointName, loggerFactory) { _options = options; _process = process; _stderrRollingLog = stderrRollingLog; + _errorHandler = errorHandler; try { _processId = process.Id; } catch { } } @@ -55,6 +57,10 @@ protected override async ValueTask CleanupAsync(Exception? error = null, Cancell // so create an exception with details about that. error ??= await GetUnexpectedExitExceptionAsync(cancellationToken).ConfigureAwait(false); + // Detach the stderr handler so no further ErrorDataReceived events + // are dispatched during or after process disposal. + _process.ErrorDataReceived -= _errorHandler; + // Terminate the server process (or confirm it already exited), then build // and publish strongly-typed completion details while the process handle // is still valid so we can read the exit code. @@ -89,13 +95,11 @@ protected override async ValueTask CleanupAsync(Exception? error = null, Cancell try { // The process has exited, but we still need to ensure stderr has been flushed. - // WaitForExitAsync only waits for exit; it does not guarantee that all - // ErrorDataReceived events have been dispatched. The synchronous WaitForExit() - // (no arguments) does ensure that, so call it after WaitForExitAsync completes. #if NET await _process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); +#else + _process.WaitForExit((int)_options.ShutdownTimeout.TotalMilliseconds); #endif - _process.WaitForExit(); } catch { } diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 9d7ca23b0..40b86a4d3 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -59,6 +59,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = Process? process = null; bool processStarted = false; + DataReceivedEventHandler? errorHandler = null; string command = _options.Command; IList? arguments = _options.Arguments; @@ -136,7 +137,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = // few lines in a rolling log for use in exceptions. const int MaxStderrLength = 10; // keep the last 10 lines of stderr Queue stderrRollingLog = new(MaxStderrLength); - process.ErrorDataReceived += (sender, args) => + process.ErrorDataReceived += errorHandler = (sender, args) => { string? data = args.Data; if (data is not null) @@ -203,7 +204,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = process.BeginErrorReadLine(); - return new StdioClientSessionTransport(_options, process, endpointName, stderrRollingLog, _loggerFactory); + return new StdioClientSessionTransport(_options, process, endpointName, stderrRollingLog, errorHandler, _loggerFactory); } catch (Exception ex) { @@ -211,6 +212,11 @@ public async Task ConnectAsync(CancellationToken cancellationToken = try { + if (process is not null && errorHandler is not null) + { + process.ErrorDataReceived -= errorHandler; + } + DisposeProcess(process, processStarted, _options.ShutdownTimeout); } catch (Exception ex2) @@ -238,18 +244,6 @@ internal static void DisposeProcess( process.KillTree(shutdownTimeout); } - // When the process exited on its own, call WaitForExit() to flush - // any remaining ErrorDataReceived/OutputDataReceived events. - // We skip this after KillTree because the parameterless WaitForExit() - // blocks until all redirected output pipe handles are closed, which - // hangs when grandchild processes (e.g. from cmd.exe /c npx) inherited - // the handles. Late-firing events after KillTree are safe because the - // ErrorDataReceived handler is protected with try/catch. - if (!processRunning && HasExited(process)) - { - process.WaitForExit(); - } - // Invoke the callback while the process handle is still valid, // e.g. to read ExitCode before Dispose() invalidates it. beforeDispose?.Invoke(); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs index f5cc37561..72d075fe7 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs @@ -84,7 +84,7 @@ public async Task RunConformanceTest(string scenario) // Protect callbacks with try/catch to prevent ITestOutputHelper from // throwing on a background thread if events arrive after the test completes. - process.OutputDataReceived += (sender, e) => + DataReceivedEventHandler outputHandler = (sender, e) => { if (e.Data != null) { @@ -93,7 +93,7 @@ public async Task RunConformanceTest(string scenario) } }; - process.ErrorDataReceived += (sender, e) => + DataReceivedEventHandler errorHandler = (sender, e) => { if (e.Data != null) { @@ -102,6 +102,9 @@ public async Task RunConformanceTest(string scenario) } }; + process.OutputDataReceived += outputHandler; + process.ErrorDataReceived += errorHandler; + process.Start(); process.BeginOutputReadLine(); process.BeginErrorReadLine(); @@ -114,6 +117,8 @@ public async Task RunConformanceTest(string scenario) catch (OperationCanceledException) { process.Kill(entireProcessTree: true); + process.OutputDataReceived -= outputHandler; + process.ErrorDataReceived -= errorHandler; return ( Success: false, Output: outputBuilder.ToString(), @@ -121,6 +126,9 @@ public async Task RunConformanceTest(string scenario) ); } + process.OutputDataReceived -= outputHandler; + process.ErrorDataReceived -= errorHandler; + var output = outputBuilder.ToString(); var error = errorBuilder.ToString(); var success = process.ExitCode == 0 || HasOnlyWarnings(output, error); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs index c79336714..e538a6f3f 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs @@ -138,7 +138,7 @@ public async Task RunPendingConformanceTest_ServerSsePolling() // Protect callbacks with try/catch to prevent ITestOutputHelper from // throwing on a background thread if events arrive after the test completes. - process.OutputDataReceived += (sender, e) => + DataReceivedEventHandler outputHandler = (sender, e) => { if (e.Data != null) { @@ -147,7 +147,7 @@ public async Task RunPendingConformanceTest_ServerSsePolling() } }; - process.ErrorDataReceived += (sender, e) => + DataReceivedEventHandler errorHandler = (sender, e) => { if (e.Data != null) { @@ -156,6 +156,9 @@ public async Task RunPendingConformanceTest_ServerSsePolling() } }; + process.OutputDataReceived += outputHandler; + process.ErrorDataReceived += errorHandler; + process.Start(); process.BeginOutputReadLine(); process.BeginErrorReadLine(); @@ -168,6 +171,8 @@ public async Task RunPendingConformanceTest_ServerSsePolling() catch (OperationCanceledException) { process.Kill(entireProcessTree: true); + process.OutputDataReceived -= outputHandler; + process.ErrorDataReceived -= errorHandler; return ( Success: false, Output: outputBuilder.ToString(), @@ -175,6 +180,9 @@ public async Task RunPendingConformanceTest_ServerSsePolling() ); } + process.OutputDataReceived -= outputHandler; + process.ErrorDataReceived -= errorHandler; + return ( Success: process.ExitCode == 0, Output: outputBuilder.ToString(), From 8f6e91faee0c680343c712dd47d1768a33b5a238 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Tue, 24 Mar 2026 10:05:13 -0700 Subject: [PATCH 06/13] Fix undisposed resources --- .../McpServerBuilderExtensionsTransportsTests.cs | 4 ++-- .../Server/McpServerLoggingLevelTests.cs | 12 ++++++------ .../Server/McpServerResourceTests.cs | 12 ++++++------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs index a52746b88..849c3e9d9 100644 --- a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs +++ b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs @@ -9,7 +9,7 @@ namespace ModelContextProtocol.Tests.Configuration; public class McpServerBuilderExtensionsTransportsTests { [Fact] - public void WithStdioServerTransport_Sets_Transport() + public async Task WithStdioServerTransport_Sets_Transport() { var services = new ServiceCollection(); services.AddMcpServer().WithStdioServerTransport(); @@ -17,7 +17,7 @@ public void WithStdioServerTransport_Sets_Transport() var transportServiceType = services.FirstOrDefault(s => s.ServiceType == typeof(ITransport)); Assert.NotNull(transportServiceType); - var serviceProvider = services.BuildServiceProvider(); + await using var serviceProvider = services.BuildServiceProvider(); Assert.IsType(serviceProvider.GetRequiredService()); } diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs index 10116e70e..b4be8b3f2 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs @@ -15,7 +15,7 @@ public McpServerLoggingLevelTests() } [Fact] - public void CanCreateServerWithLoggingLevelHandler() + public async Task CanCreateServerWithLoggingLevelHandler() { var services = new ServiceCollection(); @@ -23,13 +23,13 @@ public void CanCreateServerWithLoggingLevelHandler() .WithStdioServerTransport() .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult()); - var provider = services.BuildServiceProvider(); + await using var provider = services.BuildServiceProvider(); provider.GetRequiredService(); } [Fact] - public void AddingLoggingLevelHandlerSetsLoggingCapability() + public async Task AddingLoggingLevelHandlerSetsLoggingCapability() { var services = new ServiceCollection(); @@ -37,7 +37,7 @@ public void AddingLoggingLevelHandlerSetsLoggingCapability() .WithStdioServerTransport() .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult()); - var provider = services.BuildServiceProvider(); + await using var provider = services.BuildServiceProvider(); var server = provider.GetRequiredService(); @@ -46,12 +46,12 @@ public void AddingLoggingLevelHandlerSetsLoggingCapability() } [Fact] - public void ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability() + public async Task ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapability() { var services = new ServiceCollection(); services.AddMcpServer() .WithStdioServerTransport(); - var provider = services.BuildServiceProvider(); + await using var provider = services.BuildServiceProvider(); var server = provider.GetRequiredService(); Assert.Null(server.ServerOptions.Capabilities?.Logging); } diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs index 695bbf39d..708c24c1f 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs @@ -29,7 +29,7 @@ public McpServerResourceTests() } [Fact] - public void CanCreateServerWithResource() + public async Task CanCreateServerWithResource() { var services = new ServiceCollection(); @@ -58,14 +58,14 @@ public void CanCreateServerWithResource() }; }); - var provider = services.BuildServiceProvider(); + await using var provider = services.BuildServiceProvider(); provider.GetRequiredService(); } [Fact] - public void CanCreateServerWithResourceTemplates() + public async Task CanCreateServerWithResourceTemplates() { var services = new ServiceCollection(); @@ -94,13 +94,13 @@ public void CanCreateServerWithResourceTemplates() }; }); - var provider = services.BuildServiceProvider(); + await using var provider = services.BuildServiceProvider(); provider.GetRequiredService(); } [Fact] - public void CreatingReadHandlerWithNoListHandlerSucceeds() + public async Task CreatingReadHandlerWithNoListHandlerSucceeds() { var services = new ServiceCollection(); services.AddMcpServer() @@ -117,7 +117,7 @@ public void CreatingReadHandlerWithNoListHandlerSucceeds() }] }; }); - var sp = services.BuildServiceProvider(); + await using var sp = services.BuildServiceProvider(); sp.GetRequiredService(); } From ccfd6851316dc7dc4401ff7c529c528acdd6098d Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Wed, 25 Mar 2026 10:19:13 -0700 Subject: [PATCH 07/13] Avoid StdioServerTransport where not required StdioServerTransport does not reliably close on linux, which causes a threadpool thread to be blocked on read. This can lead to threadpool starvation and application hangs. Since the tests in question do not require stdio transport, switch to using StreamServerTransport with null streams to avoid this issue. --- .../Configuration/McpServerOptionsSetupTests.cs | 5 +++-- .../McpServerResourceCapabilityIntegrationTests.cs | 10 +++++----- .../Server/McpServerLoggingLevelTests.cs | 6 +++--- .../Server/McpServerResourceTests.cs | 6 +++--- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs index 40165d58c..689aba9d0 100644 --- a/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs +++ b/tests/ModelContextProtocol.Tests/Configuration/McpServerOptionsSetupTests.cs @@ -181,9 +181,10 @@ public async Task ServerCapabilities_WithManualResourceSubscribeCapability_AndWi }; }) .WithResources() - .WithStdioServerTransport(); + .WithStreamServerTransport(Stream.Null, Stream.Null); - var options = services.BuildServiceProvider().GetRequiredService>().Value; + await using var sp = services.BuildServiceProvider(); + var options = sp.GetRequiredService>().Value; // The options should preserve the user's manually set capabilities Assert.NotNull(options.Capabilities?.Resources); diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs index f9b6217cf..1d9fe554a 100644 --- a/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs +++ b/tests/ModelContextProtocol.Tests/Configuration/McpServerResourceCapabilityIntegrationTests.cs @@ -106,9 +106,9 @@ public async Task Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerO }; }) .WithResources() - .WithStdioServerTransport(); + .WithStreamServerTransport(Stream.Null, Stream.Null); - var serviceProvider = services.BuildServiceProvider(); + await using var serviceProvider = services.BuildServiceProvider(); var mcpOptions = serviceProvider.GetRequiredService>().Value; // Verify capabilities are preserved @@ -122,15 +122,15 @@ public async Task Resources_AreExposed_WhenSubscribeCapabilitySetInAddMcpServerO } [Fact] - public void ResourcesCapability_IsCreated_WhenOnlyResourcesAreProvided() + public async Task ResourcesCapability_IsCreated_WhenOnlyResourcesAreProvided() { // Test that ResourcesCapability is created even without handlers or manual setting var services = new ServiceCollection(); var builder = services.AddMcpServer() .WithResources() - .WithStdioServerTransport(); + .WithStreamServerTransport(Stream.Null, Stream.Null); - var serviceProvider = services.BuildServiceProvider(); + await using var serviceProvider = services.BuildServiceProvider(); var mcpOptions = serviceProvider.GetRequiredService>().Value; // Resources are registered diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs index b4be8b3f2..a45d19604 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerLoggingLevelTests.cs @@ -20,7 +20,7 @@ public async Task CanCreateServerWithLoggingLevelHandler() var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport() + .WithStreamServerTransport(Stream.Null, Stream.Null) .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult()); await using var provider = services.BuildServiceProvider(); @@ -34,7 +34,7 @@ public async Task AddingLoggingLevelHandlerSetsLoggingCapability() var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport() + .WithStreamServerTransport(Stream.Null, Stream.Null) .WithSetLoggingLevelHandler(async (ctx, ct) => new EmptyResult()); await using var provider = services.BuildServiceProvider(); @@ -50,7 +50,7 @@ public async Task ServerWithoutCallingLoggingLevelHandlerDoesNotSetLoggingCapabi { var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport(); + .WithStreamServerTransport(Stream.Null, Stream.Null); await using var provider = services.BuildServiceProvider(); var server = provider.GetRequiredService(); Assert.Null(server.ServerOptions.Capabilities?.Logging); diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs index 708c24c1f..e05d47c8c 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs @@ -34,7 +34,7 @@ public async Task CanCreateServerWithResource() var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport() + .WithStreamServerTransport(Stream.Null, Stream.Null) .WithListResourcesHandler(async (ctx, ct) => { return new ListResourcesResult @@ -70,7 +70,7 @@ public async Task CanCreateServerWithResourceTemplates() var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport() + .WithStreamServerTransport(Stream.Null, Stream.Null) .WithListResourceTemplatesHandler(async (ctx, ct) => { return new ListResourceTemplatesResult @@ -104,7 +104,7 @@ public async Task CreatingReadHandlerWithNoListHandlerSucceeds() { var services = new ServiceCollection(); services.AddMcpServer() - .WithStdioServerTransport() + .WithStreamServerTransport(Stream.Null, Stream.Null) .WithReadResourceHandler(async (ctx, ct) => { return new ReadResourceResult From 5cae6fbf8113efe74beaae4f6b77e24f23f22180 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Wed, 25 Mar 2026 15:55:55 -0700 Subject: [PATCH 08/13] More test fixes --- .../McpServerBuilderExtensionsTransportsTests.cs | 13 +++++++------ .../Transport/StdioServerTransportTests.cs | 7 +++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs index 849c3e9d9..6af33d8d2 100644 --- a/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs +++ b/tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsTransportsTests.cs @@ -9,16 +9,17 @@ namespace ModelContextProtocol.Tests.Configuration; public class McpServerBuilderExtensionsTransportsTests { [Fact] - public async Task WithStdioServerTransport_Sets_Transport() + public void WithStdioServerTransport_Registers_Transport() { var services = new ServiceCollection(); services.AddMcpServer().WithStdioServerTransport(); - var transportServiceType = services.FirstOrDefault(s => s.ServiceType == typeof(ITransport)); - Assert.NotNull(transportServiceType); - - await using var serviceProvider = services.BuildServiceProvider(); - Assert.IsType(serviceProvider.GetRequiredService()); + // Verify StdioServerTransport is registered for ITransport, but don't resolve it โ€” + // doing so opens Console.OpenStandardInput() which permanently blocks a thread pool + // thread on the test host's stdin. StdioServerTransport should only be used in a + // dedicated child process, not in-process. + var transportDescriptor = services.FirstOrDefault(s => s.ServiceType == typeof(ITransport)); + Assert.NotNull(transportDescriptor); } [Fact] diff --git a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs index 22ac43d95..e47269686 100644 --- a/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/StdioServerTransportTests.cs @@ -27,8 +27,11 @@ public StdioServerTransportTests(ITestOutputHelper testOutputHelper) [Fact] public async Task Constructor_Should_Initialize_With_Valid_Parameters() { - // Act - await using var transport = new StdioServerTransport(_serverOptions); + // Use StreamServerTransport with Stream.Null rather than StdioServerTransport. + // StdioServerTransport opens Console.OpenStandardInput() which permanently + // blocks a thread pool thread on the test host's stdin. StdioServerTransport + // should only be instantiated in a dedicated child process. + await using var transport = new StreamServerTransport(Stream.Null, Stream.Null, _serverOptions.ServerInfo?.Name); // Assert Assert.NotNull(transport); From 0647e71a501d37bd25f789f7f00888b012a47038 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Wed, 25 Mar 2026 19:10:13 -0700 Subject: [PATCH 09/13] Fix the race condition between DisposeAsync and ReadMessageAsync. --- .../Client/StdioClientSessionTransport.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs index 37344d2e7..57d6bf7b3 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs @@ -47,9 +47,13 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation /// protected override async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default) { - // Only clean up once. + // Only run the full stdio cleanup once (handler detach, process kill, etc.). + // But always ensure the channel is completed so DisposeAsync's + // "await Completion" doesn't hang when the ReadMessagesAsync path + // is still mid-cleanup (e.g. blocked in WaitForExitAsync). if (Interlocked.Exchange(ref _cleanedUp, 1) != 0) { + SetDisconnected(error); return; } From a698b024611f963a074b2eae7a791739fa2656ec Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Wed, 25 Mar 2026 20:46:47 -0700 Subject: [PATCH 10/13] Fix test regression --- .../Client/StdioClientSessionTransport.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs index 57d6bf7b3..9abaffdde 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs @@ -48,12 +48,16 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation protected override async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default) { // Only run the full stdio cleanup once (handler detach, process kill, etc.). - // But always ensure the channel is completed so DisposeAsync's - // "await Completion" doesn't hang when the ReadMessagesAsync path - // is still mid-cleanup (e.g. blocked in WaitForExitAsync). + // But always call base.CleanupAsync โ€” it cancels the shutdown CTS (which + // unblocks the primary path if it's stuck in WaitForExitAsync), waits for + // the read task to complete (so the primary path can call SetDisconnected + // with full StdioClientCompletionDetails), and calls SetDisconnected itself + // as a fallback. We wrap the error in TransportClosedException so the + // fallback SetDisconnected still produces StdioClientCompletionDetails + // rather than a bare ClientCompletionDetails. if (Interlocked.Exchange(ref _cleanedUp, 1) != 0) { - SetDisconnected(error); + await base.CleanupAsync(new TransportClosedException(BuildCompletionDetails(error)), cancellationToken).ConfigureAwait(false); return; } From 8ad61e56e5752883b8d2c22b2449bcf258252406 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Thu, 26 Mar 2026 11:08:39 -0700 Subject: [PATCH 11/13] Address feedback and CI failure --- .../Client/StdioClientSessionTransport.cs | 12 ++++-------- .../Client/StdioClientTransport.cs | 3 ++- .../Client/StreamClientSessionTransport.cs | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs index 9abaffdde..f159780fd 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs @@ -48,16 +48,12 @@ public override async Task SendMessageAsync(JsonRpcMessage message, Cancellation protected override async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default) { // Only run the full stdio cleanup once (handler detach, process kill, etc.). - // But always call base.CleanupAsync โ€” it cancels the shutdown CTS (which - // unblocks the primary path if it's stuck in WaitForExitAsync), waits for - // the read task to complete (so the primary path can call SetDisconnected - // with full StdioClientCompletionDetails), and calls SetDisconnected itself - // as a fallback. We wrap the error in TransportClosedException so the - // fallback SetDisconnected still produces StdioClientCompletionDetails - // rather than a bare ClientCompletionDetails. + // If another call is already handling cleanup, cancel the shutdown token + // to unblock it (e.g. if it's stuck in WaitForExitAsync) and let it + // call SetDisconnected with full StdioClientCompletionDetails. if (Interlocked.Exchange(ref _cleanedUp, 1) != 0) { - await base.CleanupAsync(new TransportClosedException(BuildCompletionDetails(error)), cancellationToken).ConfigureAwait(false); + CancelShutdown(); return; } diff --git a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs index 40b86a4d3..24a7dba8e 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientTransport.cs @@ -137,7 +137,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = // few lines in a rolling log for use in exceptions. const int MaxStderrLength = 10; // keep the last 10 lines of stderr Queue stderrRollingLog = new(MaxStderrLength); - process.ErrorDataReceived += errorHandler = (sender, args) => + errorHandler = (sender, args) => { string? data = args.Data; if (data is not null) @@ -167,6 +167,7 @@ public async Task ConnectAsync(CancellationToken cancellationToken = LogReadStderr(logger, endpointName, data); } }; + process.ErrorDataReceived += errorHandler; // We need both stdin and stdout to use a no-BOM UTF-8 encoding. On .NET Core, // we can use ProcessStartInfo.StandardOutputEncoding/StandardInputEncoding, but diff --git a/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs index 19306349f..38df5b7e6 100644 --- a/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StreamClientSessionTransport.cs @@ -164,6 +164,21 @@ private async Task ProcessMessageAsync(string line, CancellationToken cancellati } } + /// + /// Cancels the shutdown token to signal that the transport is shutting down, + /// without performing any other cleanup. + /// + protected void CancelShutdown() + { + try + { + _shutdownCts?.Cancel(); + } + catch (ObjectDisposedException) + { + } + } + protected virtual async ValueTask CleanupAsync(Exception? error = null, CancellationToken cancellationToken = default) { LogTransportShuttingDown(Name); From bc10e412bc96258f11ad0b70491752cabf9597be Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Thu, 26 Mar 2026 23:19:38 -0700 Subject: [PATCH 12/13] Add timeout to WaitForExitAsync in GetUnexpectedExitExceptionAsync When CleanupAsync is entered from ReadMessagesAsync (stdout EOF), the cancellation token is _shutdownCts.Token which hasn't been canceled yet (base.CleanupAsync runs later). If stderr EOF hasn't been delivered by the threadpool yet, WaitForExitAsync hangs indefinitely. Use a linked CTS with ShutdownTimeout to bound the wait. The process is already dead; we're just draining stderr pipe buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Client/StdioClientSessionTransport.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs index f159780fd..907f9e062 100644 --- a/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/StdioClientSessionTransport.cs @@ -99,8 +99,14 @@ protected override async ValueTask CleanupAsync(Exception? error = null, Cancell try { // The process has exited, but we still need to ensure stderr has been flushed. + // Use a bounded wait: the process is already dead, we're just draining pipe + // buffers. If the caller's token is never canceled (e.g. _shutdownCts hasn't + // been canceled yet), an unbounded wait here can hang indefinitely when the + // threadpool is slow to deliver the stderr EOF callback. #if NET - await _process.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + timeoutCts.CancelAfter(_options.ShutdownTimeout); + await _process.WaitForExitAsync(timeoutCts.Token).ConfigureAwait(false); #else _process.WaitForExit((int)_options.ShutdownTimeout.TotalMilliseconds); #endif From 6db87b9935ed7a80dd178c7c775176a432306524 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Mon, 30 Mar 2026 10:46:34 -0700 Subject: [PATCH 13/13] Disable RuntimeAsync in CI to work around .NET 10 test host hang xUnit v3 test host hangs intermittently on .NET 10 when RuntimeAsync is enabled (the default). The hang occurs after all tests have passed, with the test host process stalling indefinitely. Setting DOTNET_RuntimeAsync=0 reliably prevents the hang. This is a temporary workaround pending a fix in the .NET runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci-build-test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci-build-test.yml b/.github/workflows/ci-build-test.yml index a7e265a89..7d473091d 100644 --- a/.github/workflows/ci-build-test.yml +++ b/.github/workflows/ci-build-test.yml @@ -61,6 +61,11 @@ jobs: run: make build CONFIGURATION=${{ matrix.configuration }} - name: ๐Ÿงช Test + # TODO: Remove DOTNET_RuntimeAsync=0 workaround once the .NET runtime fix is available. + # Tracked by: https://github.com/dotnet/runtime/issues/126325 + # xUnit v3 test host hangs intermittently on .NET 10 with RuntimeAsync enabled. + env: + DOTNET_RuntimeAsync: "0" run: make test CONFIGURATION=${{ matrix.configuration }} - name: ๐Ÿงช AOT Compatibility