fix: cleanup zombie processes for child process client (#156)
* feat(client): cleanup of zombie processes in child process client Using process wrap library, Process Group for Unix and Job Object for Windows * fix: install extra dep based on feature, update examples take owned command Install process-wrap if transport-child-process feature is enabled. Update examples to take ownership of the command instead of mutable reference * fix: update other examples, comments and readme to take ownership of command Updated more examples, comments and readme to take command instead of mutable ref. Also small fix in cargo toml of example to use local path. * refactor: add configure command ext Added configure command ext to tokio process command so that you can use .configure() to use inline commands for mcp stdio client. Added warning if start kill process fails
This commit is contained in:
parent
68ddea3ad5
commit
64995cc40c
6 changed files with 115 additions and 77 deletions
|
|
@ -45,6 +45,8 @@ url = { version = "2.4", optional = true }
|
|||
# For tower compatibility
|
||||
tower-service = { version = "0.3", optional = true }
|
||||
|
||||
# for child process transport
|
||||
process-wrap = { version = "8.2", features = ["tokio1"], optional = true}
|
||||
|
||||
# for ws transport
|
||||
# tokio-tungstenite ={ version = "0.26", optional = true }
|
||||
|
|
@ -89,7 +91,11 @@ transport-streamable-http-client = [
|
|||
|
||||
transport-async-rw = ["tokio/io-util", "tokio-util/codec"]
|
||||
transport-io = ["transport-async-rw", "tokio/io-std"]
|
||||
transport-child-process = ["transport-async-rw", "tokio/process"]
|
||||
transport-child-process = [
|
||||
"transport-async-rw",
|
||||
"tokio/process",
|
||||
"dep:process-wrap",
|
||||
]
|
||||
transport-sse-server = [
|
||||
"transport-async-rw",
|
||||
"axum",
|
||||
|
|
|
|||
|
|
@ -58,15 +58,13 @@
|
|||
//!
|
||||
//! ```rust
|
||||
//! use anyhow::Result;
|
||||
//! use rmcp::{model::CallToolRequestParam, service::ServiceExt, transport::TokioChildProcess};
|
||||
//! use rmcp::{model::CallToolRequestParam, service::ServiceExt, transport::{TokioChildProcess, ConfigureCommandExt}};
|
||||
//! use tokio::process::Command;
|
||||
//!
|
||||
//! async fn client() -> Result<()> {
|
||||
//! let service = ()
|
||||
//! .serve(TokioChildProcess::new(
|
||||
//! Command::new("uvx").arg("mcp-server-git"),
|
||||
//! )?)
|
||||
//! .await?;
|
||||
//! let service = ().serve(TokioChildProcess::new(Command::new("uvx").configure(|cmd| {
|
||||
//! cmd.arg("mcp-server-git");
|
||||
//! }))?).await?;
|
||||
//!
|
||||
//! // Initialize
|
||||
//! let server_info = service.peer_info();
|
||||
|
|
|
|||
|
|
@ -84,7 +84,7 @@ pub use worker::WorkerTransport;
|
|||
pub mod child_process;
|
||||
#[cfg(feature = "transport-child-process")]
|
||||
#[cfg_attr(docsrs, doc(cfg(feature = "transport-child-process")))]
|
||||
pub use child_process::TokioChildProcess;
|
||||
pub use child_process::{ConfigureCommandExt, TokioChildProcess};
|
||||
|
||||
#[cfg(feature = "transport-io")]
|
||||
#[cfg_attr(docsrs, doc(cfg(feature = "transport-io")))]
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
use process_wrap::tokio::{TokioChildWrapper, TokioCommandWrap};
|
||||
use tokio::{
|
||||
io::AsyncRead,
|
||||
process::{ChildStdin, ChildStdout},
|
||||
|
|
@ -7,29 +8,41 @@ use super::{IntoTransport, Transport};
|
|||
use crate::service::ServiceRole;
|
||||
|
||||
pub(crate) fn child_process(
|
||||
mut child: tokio::process::Child,
|
||||
) -> std::io::Result<(tokio::process::Child, (ChildStdout, ChildStdin))> {
|
||||
if child.stdin.is_none() {
|
||||
return Err(std::io::Error::other("std in was taken"));
|
||||
}
|
||||
if child.stdout.is_none() {
|
||||
return Err(std::io::Error::other("std out was taken"));
|
||||
}
|
||||
let child_stdin = child.stdin.take().expect("already checked");
|
||||
let child_stdout = child.stdout.take().expect("already checked");
|
||||
mut child: Box<dyn TokioChildWrapper>,
|
||||
) -> std::io::Result<(Box<dyn TokioChildWrapper>, (ChildStdout, ChildStdin))> {
|
||||
let child_stdin = match child.inner_mut().stdin().take() {
|
||||
Some(stdin) => stdin,
|
||||
None => return Err(std::io::Error::other("std in was taken")),
|
||||
};
|
||||
let child_stdout = match child.inner_mut().stdout().take() {
|
||||
Some(stdout) => stdout,
|
||||
None => return Err(std::io::Error::other("std out was taken")),
|
||||
};
|
||||
Ok((child, (child_stdout, child_stdin)))
|
||||
}
|
||||
|
||||
pub struct TokioChildProcess {
|
||||
child: tokio::process::Child,
|
||||
child: ChildWithCleanup,
|
||||
child_stdin: ChildStdin,
|
||||
child_stdout: ChildStdout,
|
||||
}
|
||||
|
||||
pub struct ChildWithCleanup {
|
||||
inner: Box<dyn TokioChildWrapper>,
|
||||
}
|
||||
|
||||
impl Drop for ChildWithCleanup {
|
||||
fn drop(&mut self) {
|
||||
if let Err(e) = self.inner.start_kill() {
|
||||
tracing::warn!("Failed to kill child process: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// we hold the child process with stdout, for it's easier to implement AsyncRead
|
||||
pin_project_lite::pin_project! {
|
||||
pub struct TokioChildProcessOut {
|
||||
child: tokio::process::Child,
|
||||
child: ChildWithCleanup,
|
||||
#[pin]
|
||||
child_stdout: ChildStdout,
|
||||
}
|
||||
|
|
@ -46,14 +59,18 @@ impl AsyncRead for TokioChildProcessOut {
|
|||
}
|
||||
|
||||
impl TokioChildProcess {
|
||||
pub fn new(child: &mut tokio::process::Command) -> std::io::Result<Self> {
|
||||
child
|
||||
.kill_on_drop(true)
|
||||
pub fn new(mut command: tokio::process::Command) -> std::io::Result<Self> {
|
||||
command
|
||||
.stdin(std::process::Stdio::piped())
|
||||
.stdout(std::process::Stdio::piped());
|
||||
let (child, (child_stdout, child_stdin)) = child_process(child.spawn()?)?;
|
||||
let mut command_wrap = TokioCommandWrap::from(command);
|
||||
#[cfg(unix)]
|
||||
command_wrap.wrap(process_wrap::tokio::ProcessGroup::leader());
|
||||
#[cfg(windows)]
|
||||
command_wrap.wrap(process_wrap::tokio::JobObject);
|
||||
let (child, (child_stdout, child_stdin)) = child_process(command_wrap.spawn()?)?;
|
||||
Ok(Self {
|
||||
child,
|
||||
child: ChildWithCleanup { inner: child },
|
||||
child_stdin,
|
||||
child_stdout,
|
||||
})
|
||||
|
|
@ -82,3 +99,14 @@ impl<R: ServiceRole> IntoTransport<R, std::io::Error, ()> for TokioChildProcess
|
|||
)
|
||||
}
|
||||
}
|
||||
|
||||
pub trait ConfigureCommandExt {
|
||||
fn configure(self, f: impl FnOnce(&mut Self)) -> Self;
|
||||
}
|
||||
|
||||
impl ConfigureCommandExt for tokio::process::Command {
|
||||
fn configure(mut self, f: impl FnOnce(&mut Self)) -> Self {
|
||||
f(&mut self);
|
||||
self
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use rmcp::{
|
|||
ServiceExt,
|
||||
service::QuitReason,
|
||||
transport::{
|
||||
SseServer, StreamableHttpClientTransport, TokioChildProcess,
|
||||
ConfigureCommandExt, SseServer, StreamableHttpClientTransport, TokioChildProcess,
|
||||
streamable_http_server::axum::StreamableHttpServer,
|
||||
},
|
||||
};
|
||||
|
|
@ -59,9 +59,10 @@ async fn test_with_js_server() -> anyhow::Result<()> {
|
|||
.spawn()?
|
||||
.wait()
|
||||
.await?;
|
||||
let transport = TokioChildProcess::new(
|
||||
tokio::process::Command::new("node").arg("tests/test_with_js/server.js"),
|
||||
)?;
|
||||
let transport =
|
||||
TokioChildProcess::new(tokio::process::Command::new("node").configure(|cmd| {
|
||||
cmd.arg("tests/test_with_js/server.js");
|
||||
}))?;
|
||||
|
||||
let client = ().serve(transport).await?;
|
||||
let resources = client.list_all_resources().await?;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
use rmcp::{
|
||||
ServiceExt,
|
||||
transport::{SseServer, TokioChildProcess},
|
||||
transport::{ConfigureCommandExt, SseServer, TokioChildProcess},
|
||||
};
|
||||
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
|
||||
mod common;
|
||||
|
|
@ -54,11 +54,9 @@ async fn test_with_python_server() -> anyhow::Result<()> {
|
|||
.spawn()?
|
||||
.wait()
|
||||
.await?;
|
||||
let transport = TokioChildProcess::new(
|
||||
tokio::process::Command::new("uv")
|
||||
.arg("run")
|
||||
.arg("tests/test_with_python/server.py"),
|
||||
)?;
|
||||
let transport = TokioChildProcess::new(tokio::process::Command::new("uv").configure(|cmd| {
|
||||
cmd.arg("run").arg("tests/test_with_python/server.py");
|
||||
}))?;
|
||||
|
||||
let client = ().serve(transport).await?;
|
||||
let resources = client.list_all_resources().await?;
|
||||
|
|
|
|||
Loading…
Reference in a new issue