Skip to content

Fix StdioServerTransport.DisposeAsync() hang: CancellableStdinStream missing Dispose override#1276

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-flaky-test-stdio-server-transport
Open

Fix StdioServerTransport.DisposeAsync() hang: CancellableStdinStream missing Dispose override#1276
Copilot wants to merge 3 commits intomainfrom
copilot/fix-flaky-test-stdio-server-transport

Conversation

Copy link
Contributor

Copilot AI commented Feb 15, 2026 •
edited
Loading

StdioServerTransport.DisposeAsync() could hang indefinitely on both Windows and Unix. The disposal sequence cancels the shutdown token, then disposes the input reader to forcefully close the stdin handle and interrupt blocked read syscalls. However, CancellableStdinStream never overrode Dispose(bool), so the underlying Console.OpenStandardInput() stream was never actually closed — the blocked read was never interrupted, and await _readLoopCompleted would hang forever.

Original prompt

This section details on the original issue you should resolve

<issue_title>Flaky test: StdioServerTransport.DisposeAsync() can hang</issue_title>
<issue_description>```
The active test run was aborted. Reason: Test host process crashed
Data collector 'Blame' message: The specified inactivity time of 7 minutes has elapsed. Collecting hang dumps from testhost and its child processes.

Test Run Aborted.
Total tests: Unknown
Passed: 189
Skipped: 3
Total time: 7.6249 Minutes
Results File: /home/runner/work/csharp-sdk/csharp-sdk/testresults/_fv-az2210-804_2025-03-29_23_16_37.trx

The active Test Run was aborted because the host process exited unexpectedly. Please inspect the call stack above, if available, to get more information about where the exception originated from.
The test running when the crash occurred:
ModelContextProtocol.Tests.Transport.StdioServerTransportTests.Constructor_Should_Initialize_With_Valid_Parameters

This test may, or may not be the source of the crash.


https://github.com/modelcontextprotocol/csharp-sdk/actions/runs/14150315138/job/39642597376?pr=142

I think there's good reason to believe StdioServerTransportTests.Constructor_Should_Initialize_With_Valid_Parameters is the source of the hang considering Constructor_Should_Initialize_With_Valid_Parameters is the only in-proc test that tries to read from the real stdin and then cancel the read gracefully. I'm pretty sure we're cancelling the ReadLineAsync operation correctly, so we might have to look into the Unix ConsolePal.</issue_description>

<agent_instructions>Determine if this still repros, re-enabling the test and trying running the tests over and over. Also see if you can root cause the issue even if it doesn't repro. If you can figure out the problem or if it's no longer a problem, re-enable the test and fix whatever needs to be fixed, if anything. </agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@eiriktsarpalis</author><body>
Updated the title to reflect that this hands on Windows as well: https://github.com/modelcontextprotocol/csharp-sdk/pull/160#discussion_r2021359753</body></comment_new>
<comment_new><author>@halter73</author><body>
Thanks. I was just about to do the same thing. More context:

- Canceling read from stdin in StdioServerTransport.DisposeAsync is also flaky on Windows
- https://github.com/modelcontextprotocol/csharp-sdk/actions/runs/14161333333/job/39667212874</body></comment_new>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@stephentoub
Fix StdioServerTransport.DisposeAsync() hang by disposing underlying …
d727d85 
…stdin stream

CancellableStdinStream did not override Dispose(bool), so when
StreamServerTransport.DisposeAsync() disposed the StreamReader (and thus
the CancellableStdinStream wrapper), the underlying stdin stream from
Console.OpenStandardInput() was never closed. This meant the blocked
read syscall was never interrupted, causing DisposeAsync to hang.

The fix adds a Dispose(bool) override to CancellableStdinStream that
properly disposes the underlying stdinStream, allowing the forceful
close to interrupt the blocked syscall.

Also re-enables the previously-skipped test
Constructor_Should_Initialize_With_Valid_Parameters.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix flaky test in StdioServerTransport.DisposeAsync() Fix StdioServerTransport.DisposeAsync() hang: CancellableStdinStream missing Dispose override Feb 15, 2026
Copilot AI requested a review from stephentoub February 15, 2026 12:43
@stephentoub stephentoub marked this pull request as ready for review February 15, 2026 12:47
@stephentoub stephentoub enabled auto-merge (squash) February 15, 2026 12:47
@stephentoub
Copy link
Contributor

stephentoub commented Feb 15, 2026

@copilot there are CI failures https://github.com/modelcontextprotocol/csharp-sdk/actions/runs/22035836303/job/63668727308?pr=1276#step:7:773

Copy link
Contributor Author

Copilot AI commented Feb 15, 2026

@copilot there are CI failures https://github.com/modelcontextprotocol/csharp-sdk/actions/runs/22035836303/job/63668727308?pr=1276#step:7:773

The CI failure is StdioClientTransportTests.CreateAsync_ValidProcessInvalidServer_StdErrCallbackInvoked on macOS net8.0 — an Assert.InRange() where the stderr callback count was 0 (timing issue with stderr delivery). This is a pre-existing flaky test unrelated to this PR's changes, which only touch StdioServerTransport.cs and StdioServerTransportTests.cs. The failing test file (StdioClientTransportTests.cs) has no modifications in this PR.

Copilot AI requested a review from stephentoub February 15, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephentoub stephentoub Awaiting requested review from stephentoub

@halter73 halter73 Awaiting requested review from halter73

@eiriktsarpalis eiriktsarpalis Awaiting requested review from eiriktsarpalis

At least 1 approving review is required to merge this pull request.

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flaky test: StdioServerTransport.DisposeAsync() can hang

2 participants

@stephentoub