Skip to content
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.

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

Shutting down console application which uses mcpclient does not kill the stdio process #155

Closed
StefH opened this issue Mar 31, 2025 · 8 comments · Fixed by #226
Closed
Labels
bug Something isn't working

Comments

@StefH
Copy link

StefH commented Mar 31, 2025

Describe the bug
Shutting down console application with CTRL-C or just killing it and uses mcpclient does not kill the stdio process

To Reproduce
Install a dotnet tool mcpserver:

dotnet tool install --version 0.0.1-preview-02 --global mcpserver.everything.stdio

And this C# code in a console app:

using System.Collections.Generic;
using Microsoft.Extensions.Logging;
using ModelContextProtocol.Client;
using ModelContextProtocol.Configuration;
using ModelContextProtocol.Protocol.Transport;

var config = new McpServerConfig
{
    Id = "everything",
    Name = "everything",
    TransportType = TransportTypes.StdIo,
    TransportOptions = new Dictionary<string, string>
    {
        { "command", "mcpserver.everything.stdio" }
    }
};

var options = new McpClientOptions
{
    ClientInfo = new() { Name = "everything-client", Version = "1.0.0" }
};

var client = await McpClientFactory.CreateAsync(config, options, null, LoggerFactory.Create(c => c.AddConsole()));
_ = await client.ListToolsAsync();

See also https://github.com/StefH/mcptest

Expected behavior
When the application stops, is killed (using close) or using CTRL-C, it's expected that the spawned child process (the stdio server) is also killed.

This is not the case, using when manually calling DisposeAsync on the McpClient, the process is killed.

@StefH StefH added the bug Something isn't working label Mar 31, 2025
@eiriktsarpalis
Copy link
Contributor

Potentially resolved by #142? cc @halter73

@StefH
Copy link
Author

StefH commented Apr 4, 2025

@eiriktsarpalis
I did a quick check, and it seems that #142 does not yet solve this.

@stephentoub
Copy link
Contributor

How is your server implemented? The server going away at that point is up to the server.

@StefH
Copy link
Author

StefH commented Apr 4, 2025

My assumptions are:

  1. There is a dotnet tool as console - app mcpserver.everything.stdio

  2. A MCP Client is created which uses this stdio tool

  3. During creation of this client, the mcpserver.everything.stdio.exe is started:
    Image

  4. The MCP Client calls ListToolsAsync

  5. The MCP tool ends

  6. The mcpserver.everything.stdio.exe is still running:
    Image

  7. I would expect that this mcpserver.everything.stdio.exe would be killed.


Adding await using for the client:

await using var client = await McpClientFactory.CreateAsync(config, options, null, LoggerFactory.Create(c => c.AddConsole()));

Does shutdown the process, although I see an error in the logging:

fail: ModelContextProtocol.Protocol.Transport.StdioClientTransport[776939737]
      Transport shutdown failed for Client (stdio) for (everything: everything)
      System.InvalidOperationException: No process is associated with this object.
         at System.Diagnostics.Process.EnsureState(State state)
         at System.Diagnostics.Process.EnsureState(State state)
         at System.Diagnostics.Process.GetProcessHandle(Int32 access, Boolean throwIfExited)
         at System.Diagnostics.Process.WaitForExitCore(Int32 milliseconds)
         at System.Diagnostics.Process.WaitForExit(Int32 milliseconds)
         at ModelContextProtocol.Utils.ProcessHelper.KillTree(Process process, TimeSpan timeout)
         at ModelContextProtocol.Protocol.Transport.StdioClientTransport.DisposeProcess(Process process, Boolean processStarted, ILogger logger, TimeSpan shutdownTimeout, String endpointName)

@stephentoub
Copy link
Contributor

I would expect that this mcpserver.everything.stdio.exe would be killed.

To my knowledge, there's nothing one process can proactively do to say "if I'm ever killed, please also kill the following processes". The client can gracefully have code to shutdown / kill child processes it creates, and it does, but if the client is killed and thus can't run that code, it's not able to kill the server.

The server itself can have code to say "if the stdin I'm reading from ends up abruptly, I should stop processing and go away", and the implementation makes it possible to do that today. But it relies on the coding of the server not doing things like blocking forever, which is why I asked how the server is implemented.

Did you write the server? What does the code for it look like?

@StefH
Copy link
Author

StefH commented Apr 4, 2025

I did write that server, code is based on this C# SDK.
Source code can be found here.

Note that wen I use this same tool in Claude Desktop (or any other tool), these tools get shutdown/killed when Claude Desktop is also closed (via the Quit option in the taskbar).

@halter73
Copy link
Contributor

halter73 commented Apr 6, 2025

To my knowledge, there's nothing one process can proactively do to say "if I'm ever killed, please also kill the following processes".

I'm pretty sure there is stuff like this. dotnet run used to have similar problems until we fixed it.

On Windows, the child process is added to a job object that is set to
terminate the child (and its tree) upon the termination of the parent
dotnet process. On Windows 7 and Server 2008, the dotnet process cannot
already be associated with a job object for the reaping to occur. On
later Windows versions, a nested job will be created so the reaping will
still occur. After the child process exits, the job object is closed
without terminating the remaining processes in the job; this allows for
the child process to spawn additional processes that outlive the child.

On POSIX operating systems, a SIGTERM is intercepted and forwarded on to
the child process only. Like the SIGINT forwarding, it is up to the
child process to decide what to do with the SIGTERM signal (the default
is to abort).

dotnet/cli#10720

I've been meaning to implement this in the MCP C# SDK, but it hasn't been as high priority as the HTTP streaming transport. Although, I'm sure someone else could also take a look at the dotnet cli/sdk PR and do something similar for stdio servers.

@stephentoub
Copy link
Contributor

I've been meaning to implement this in the MCP C# SDK

Is it really necessary? I'd hope the client going away would cause the server's stdin to close, at which point it should automatically shut down. If that's still not happening, we should figure out why. Forcefully killing the server without giving it a chance to gracefully exit is far from ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants