Skip to content

Commit fe07dbb

Browse files
ericstjCopilot
andcommitted
Fix race condition: pending request registered after reading loop cleanup
When a child process exits immediately, the message reading loop can detect EOF and complete its cleanup sweep of _pendingRequests before SendRequestAsync registers the initialize request's TaskCompletionSource. The TCS is then never signaled, causing McpClient.CreateAsync to hang indefinitely. Fix: Set a _messageProcessingComplete flag before the sweep, and check it in SendRequestAsync after registering the TCS. This ensures that either the sweep finds the TCS, or SendRequestAsync detects the flag and fails the TCS itself. This race was the root cause of intermittent test host hangs on .NET 9+ in CI, where short-lived child processes (echo to stderr, exit 1) would exit before the session initialization request was sent. Diagnostic evidence: - xUnit diagnostic log shows the test's async Task starts but never completes - dumpasync shows zero MCP state machines on the heap (entire chain GC'd) - Hang only on .NET 9+ (different timing vs .NET 8 avoids the race) - Always one of the StdioClientTransport stderr callback tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2c41608 commit fe07dbb

1 file changed

Lines changed: 16 additions & 0 deletions

File tree

src/ModelContextProtocol.Core/McpSessionHandler.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ internal static bool SupportsPrimingEvent(string? protocolVersion)
9191

9292
private CancellationTokenSource? _messageProcessingCts;
9393
private Task? _messageProcessingTask;
94+
private volatile bool _messageProcessingComplete;
9495

9596
/// <summary>
9697
/// Initializes a new instance of the <see cref="McpSessionHandler"/> class.
@@ -325,6 +326,9 @@ ex is OperationCanceledException &&
325326
}
326327

327328
// Fail any pending requests, as they'll never be satisfied.
329+
// Set the flag before sweeping so that any request registered concurrently
330+
// via SendRequestAsync after the sweep will see it and fail itself.
331+
_messageProcessingComplete = true;
328332
foreach (var entry in _pendingRequests)
329333
{
330334
entry.Value.TrySetException(new IOException("The server shut down unexpectedly."));
@@ -569,6 +573,18 @@ public async Task<JsonRpcResponse> SendRequestAsync(JsonRpcRequest request, Canc
569573

570574
var tcs = new TaskCompletionSource<JsonRpcMessage>(TaskCreationOptions.RunContinuationsAsynchronously);
571575
_pendingRequests[request.Id] = tcs;
576+
577+
// If message processing has already completed (transport closed), fail the request
578+
// immediately. This handles the race where the reading loop's cleanup sweep ran
579+
// before this request was registered, so it was missed by the sweep.
580+
if (_messageProcessingComplete)
581+
{
582+
if (_pendingRequests.TryRemove(request.Id, out var removed))
583+
{
584+
removed.TrySetException(new IOException("The server shut down unexpectedly."));
585+
}
586+
}
587+
572588
try
573589
{
574590
if (addTags)

0 commit comments

Comments
 (0)