-
Notifications
You must be signed in to change notification settings - Fork 299
Description
Is your feature request related to a problem? Please describe.
If you want to use an stdio server, you wrap a tokio process into a service like this:
rust-sdk/examples/clients/src/git_stdio.rs
Lines 20 to 27 in fbc7ab7
let client = () | |
.serve( | |
TokioChildProcess::new(Command::new("uvx").configure(|cmd| { | |
cmd.arg("mcp-server-git"); | |
})) | |
.map_err(RmcpError::transport_creation::<TokioChildProcess>)?, | |
) | |
.await?; |
Now the service owns the process and there is generally no way (that I'm aware of) to get this ownership back. The only way of shutting down the server is to call cancel
. Based on experiments with my own server implementation, this however seems to just drop the tokio process which in turns calls SIGKILL (on Unix at least). However the protocol spec states (link):
For the stdio transport, the client SHOULD initiate shutdown by:
- First, closing the input stream to the child process (the server)
- Waiting for the server to exit, or sending
SIGTERM
if the server does not exit within a reasonable time- Sending
SIGKILL
if the server does not exit within a reasonable time afterSIGTERM
The server MAY initiate shutdown by closing its output stream to the client and exiting.
So we kinda directly jump to step 3, i.e. the last resort.
Describe the solution you'd like
I think the high-level Rust API is fine, but I think it should result in a proper shutdown with all three steps.
Describe alternatives you've considered
One can work around this by abusing step 2 in the shutdown sequence. Before you pass the tokio process to the serve
(which will use serve_client_with_ct
) one can get the PID. With that, one can use libc
or nix
to send SIGTERM. However this has two drawbacks:
- windows: It's unclear how the shutdown signal should be delivered on Windows, see Clarify stdio-transport shutdown sequence for Windows modelcontextprotocol#1090
- stdin: Not closing the stdin to the child process will result in weird interactions with tokio (if that is used for the server implementation) and potentially block the server shutdown, see Stdin can block shutdown tokio-rs/tokio#2466
Additional context
This is relevant for version 0.3.0.