Skip to content

Commit 2aa0755

Browse files
committed
Fix StdioServerTransport.DisposeAsync hang
- It's not just Unix, it's all platforms that can hang - Windows doesn't hang on Ctrl-C because that triggers the end of the Stream, but that doesn't help in IHostApplicationLifetime.StopAsync() scenarios https://github.com/dotnet/runtime/blob/4e9627e311806330be72b7f7d3660be699878ebd/src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.cs#L13 https://github.com/dotnet/runtime/blob/4e9627e311806330be72b7f7d3660be699878ebd/src/libraries/System.Console/src/System/ConsolePal.Windows.cs#L1149
1 parent 8bd9ee8 commit 2aa0755

File tree

4 files changed

+81
-2
lines changed

4 files changed

+81
-2
lines changed

samples/TestServerWithHosting/TestServerWithHosting.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
<PropertyGroup>
44
<OutputType>Exe</OutputType>
5-
<TargetFramework>net9.0</TargetFramework>
5+
<TargetFrameworks>net9.0;net8.0</TargetFrameworks>
66
<ImplicitUsings>enable</ImplicitUsings>
77
<Nullable>enable</Nullable>
88
<!--

src/ModelContextProtocol/Protocol/Transport/StdioServerTransport.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public StdioServerTransport(McpServerOptions serverOptions, ILoggerFactory? logg
6060
/// </para>
6161
/// </remarks>
6262
public StdioServerTransport(string serverName, ILoggerFactory? loggerFactory = null)
63-
: base(Console.OpenStandardInput(),
63+
: base(new CancellableStdinStream(Console.OpenStandardInput()),
6464
new BufferedStream(Console.OpenStandardOutput()),
6565
serverName ?? throw new ArgumentNullException(nameof(serverName)),
6666
loggerFactory)
@@ -73,4 +73,32 @@ private static string GetServerName(McpServerOptions serverOptions)
7373

7474
return serverOptions.ServerInfo?.Name ?? McpServer.DefaultImplementation.Name;
7575
}
76+
77+
// Neither WindowsConsoleStream nor UnixConsoleStream respect CancellationTokens or cancel any I/O on Dispose
78+
// WindowsConsoleStream will return an EOS on CTRL-C, but that not the only reason the shutdownToken may fire.
79+
private sealed class CancellableStdinStream(Stream stdinStream) : Stream
80+
{
81+
public override bool CanRead => true;
82+
public override bool CanSeek => false;
83+
public override bool CanWrite => false;
84+
85+
public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
86+
=> stdinStream.ReadAsync(buffer, offset, count, cancellationToken).WaitAsync(cancellationToken);
87+
88+
#if NET
89+
public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
90+
=> new(stdinStream.ReadAsync(buffer, cancellationToken).AsTask().WaitAsync(cancellationToken));
91+
#endif
92+
93+
public override long Length => throw new NotSupportedException();
94+
95+
public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
96+
97+
public override void Flush() => throw new NotSupportedException();
98+
public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException();
99+
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
100+
101+
public override void SetLength(long value) => throw new NotSupportedException();
102+
public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
103+
}
76104
}

tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
<ProjectReference Include="..\..\src\ModelContextProtocol\ModelContextProtocol.csproj" />
4848
<ProjectReference Include="..\ModelContextProtocol.TestServer\ModelContextProtocol.TestServer.csproj" />
4949
<ProjectReference Include="..\ModelContextProtocol.TestSseServer\ModelContextProtocol.TestSseServer.csproj" />
50+
<ProjectReference Include="..\..\samples\TestServerWithHosting\TestServerWithHosting.csproj" />
5051
</ItemGroup>
5152

5253
<ItemGroup>
@@ -56,6 +57,12 @@
5657
<Content Condition="$([MSBuild]::IsOSPlatform('Linux'))" Include="$([System.IO.Path]::GetFullPath('$(ArtifactsBinDir)ModelContextProtocol.TestServer\$(Configuration)'))\$(TargetFramework)\TestServer.dll">
5758
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
5859
</Content>
60+
<Content Condition="$([MSBuild]::IsOSPlatform('Windows'))" Include="$([System.IO.Path]::GetFullPath('$(ArtifactsBinDir)TestServerWithHosting\$(Configuration)'))\$(TargetFramework)\TestServerWithHosting.exe">
61+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
62+
</Content>
63+
<Content Condition="$([MSBuild]::IsOSPlatform('Linux'))" Include="$([System.IO.Path]::GetFullPath('$(ArtifactsBinDir)TestServerWithHosting\$(Configuration)'))\$(TargetFramework)\TestServerWithHosting.dll">
64+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
65+
</Content>
5966
</ItemGroup>
6067

6168
</Project>
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
using System.Diagnostics;
2+
using System.Runtime.InteropServices;
3+
4+
namespace ModelContextProtocol.Tests;
5+
6+
public class StdioServerIntegrationTests
7+
{
8+
public static bool CanSendSigInt { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
9+
internal const int SIGINT = 2;
10+
11+
[Fact(Skip = "Platform not supported by this test.", SkipUnless = nameof(CanSendSigInt))]
12+
public async Task SigInt_DisposesTestServerWithHosting_Gracefully()
13+
{
14+
using var process = new Process
15+
{
16+
StartInfo = new ProcessStartInfo
17+
{
18+
FileName = "dotnet",
19+
Arguments = "TestServerWithHosting.dll",
20+
RedirectStandardInput = true,
21+
RedirectStandardOutput = true,
22+
UseShellExecute = false,
23+
CreateNoWindow = true,
24+
}
25+
};
26+
27+
process.Start();
28+
29+
// I considered writing a similar test for windows using Ctrl-C, then saw that dotnet watch doesn't even send a Ctrl-C
30+
// signal because it's such a pain without support CREATE_NEW_PROCESS_GROUP in System.Diagnostics.Process
31+
// https://github.com/dotnet/sdk/blob/43b1c12e3362098a23ca1018503eb56516840b6a/src/BuiltInTools/dotnet-watch/Internal/ProcessRunner.cs#L277-L303
32+
// https://github.com/dotnet/runtime/issues/109432, https://github.com/dotnet/runtime/issues/44944
33+
Assert.Equal(0, kill(process.Id, SIGINT));
34+
35+
using var shutdownCts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
36+
await process.WaitForExitAsync(shutdownCts.Token);
37+
38+
Assert.True(process.HasExited);
39+
Assert.Equal(0, process.ExitCode);
40+
}
41+
42+
[DllImport("libc", SetLastError = true)]
43+
internal static extern int kill(int pid, int sig);
44+
}

0 commit comments

Comments
 (0)