Skip to content

Suppress ExecutionContext flow in BeginErrorReadLine #1498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ericstj wants to merge 3 commits into from
+49 −1
Closed

Suppress ExecutionContext flow in BeginErrorReadLine #1498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8807e95
Suppress ExecutionContext flow in BeginErrorReadLine
ericstj Apr 2, 2026
c00865c
Merge branch 'main' into suppress-ec-flow-stderr
ericstj Apr 6, 2026
acec346
Fix bad merge
ericstj Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/ModelContextProtocol.Core/Client/StdioClientTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,13 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =

LogTransportProcessStarted(logger, endpointName, process.Id);

process.BeginErrorReadLine();
// Suppress ExecutionContext flow so the Process's internal async
// stderr reader thread doesn't capture the caller's ambient context
// (e.g. AsyncLocal values from test infrastructure or HTTP request state).
using (ExecutionContext.SuppressFlow())

@halter73 halter73 Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is undesirable—the stderr handler is a long-lived background I/O callback that shouldn't inherit ambient state from whichever call site happened to create the transport.

While I kind of agree with this, I also don't want to be muddying every bit of code that kicks off a potentially longish running task with ExecutionContext.SuppressFlow(). And if we do, I would like some clear cut guidance on what counts as a long running thread that shouldn't be allow to hold on to the execution context.

{
process.BeginErrorReadLine();
}

return new StdioClientSessionTransport(_options, process, endpointName, stderrRollingLog, errorHandler, _loggerFactory);
}
Expand Down
42 changes: 42 additions & 0 deletions tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Text.Json;
using System.Threading;

namespace ModelContextProtocol.Tests.Transport;

Expand Down Expand Up @@ -60,6 +61,47 @@ public async Task CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked()
}

[Fact(Skip = "Platform not supported by this test.", SkipUnless = nameof(IsStdErrCallbackSupported))]
public async Task CreateAsync_StdErrCallback_DoesNotCaptureCallerAsyncLocal()
{
var asyncLocal = new AsyncLocal<string>();
asyncLocal.Value = "caller-context";

string? capturedValue = "not-set";
var received = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);

StdioClientTransport transport = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
new(new()
{
Command = "cmd",
Arguments = ["/c", "echo test >&2 & exit /b 1"],
StandardErrorLines = _ =>
{
capturedValue = asyncLocal.Value;
received.TrySetResult(true);
}
}, LoggerFactory) :
new(new()
{
Command = "sh",
Arguments = ["-c", "echo test >&2; exit 1"],
StandardErrorLines = _ =>
{
capturedValue = asyncLocal.Value;
received.TrySetResult(true);
}
}, LoggerFactory);

await Assert.ThrowsAnyAsync<IOException>(() => McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));

// Wait for the stderr callback to fire.
await received.Task.WaitAsync(TestContext.Current.CancellationToken);

// The callback should NOT see the caller's AsyncLocal value because
// ExecutionContext flow is suppressed for the stderr reader thread.
Assert.Null(capturedValue);
}

[Fact(Skip= "Platform not supported by this test.", SkipUnless = nameof(IsStdErrCallbackSupported))]
public async Task CreateAsync_StdErrCallbackThrows_DoesNotCrashProcess()
{
StdioClientTransport transport = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
Expand Down
Loading