fix: Do not exit MCP if the parent process is alive (#770)
closes #703 Bumped inactivity timeout to an hour and make it actually check every minute if parent is alive and working
This commit is contained in:
committed by
GitHub
parent
c6194b848d
commit
6398d32c0c
Generated
+2
@@ -662,8 +662,10 @@ dependencies = [
|
||||
"schemars",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"tracing",
|
||||
"windows-sys 0.60.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -30,3 +30,13 @@ tokio = { version = "1", features = ["full"] }
|
||||
tracing = { workspace = true }
|
||||
git2 = { workspace = true }
|
||||
clap = { version = "4", features = ["derive", "env"] }
|
||||
|
||||
[target.'cfg(windows)'.dependencies]
|
||||
windows-sys = { version = "0.60", features = [
|
||||
"Win32_Foundation",
|
||||
"Win32_System_Threading",
|
||||
"Win32_System_Diagnostics_ToolHelp",
|
||||
] }
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile = "3.8"
|
||||
|
||||
+58
-12
@@ -1,9 +1,12 @@
|
||||
mod cursor;
|
||||
mod healthcheck;
|
||||
mod output;
|
||||
mod parent;
|
||||
mod server;
|
||||
mod update_check;
|
||||
|
||||
use std::time::{Duration, SystemTime};
|
||||
|
||||
use clap::Parser;
|
||||
use fff::file_picker::FilePicker;
|
||||
use fff::frecency::FrecencyTracker;
|
||||
@@ -92,7 +95,7 @@ pub const MCP_INSTRUCTIONS: &str = concat!(
|
||||
" !generated/ - exclude generated code",
|
||||
);
|
||||
|
||||
/// FFF MCP Server -- a high performance & accuracy file finder for AI code assistants.
|
||||
/// FFF MCP Server - a high performance & accuracy file finder for AI code assistants.
|
||||
#[derive(Parser)]
|
||||
#[command(name = "fff-mcp", version = concat!(env!("CARGO_PKG_VERSION"), " (", env!("FFF_GIT_HASH"), ")"))]
|
||||
pub(crate) struct Args {
|
||||
@@ -183,11 +186,12 @@ pub(crate) struct Args {
|
||||
#[arg(long = "healthcheck")]
|
||||
pub(crate) healthcheck: bool,
|
||||
|
||||
/// Exit after this many seconds of inactivity. 0 = never exit.
|
||||
/// Timeout of inactivity after which fff mcp will be exited. Even if the parent process
|
||||
/// is alive we don't want to occupy resources on index and file watches if fff is unused
|
||||
#[arg(
|
||||
long = "idle-timeout-secs",
|
||||
env = "FFF_MCP_IDLE_TIMEOUT_SECS",
|
||||
default_value_t = 900
|
||||
default_value_t = 60 * 60
|
||||
)]
|
||||
idle_timeout_secs: u64,
|
||||
}
|
||||
@@ -343,9 +347,20 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
}
|
||||
};
|
||||
|
||||
if idle_timeout_secs > 0 {
|
||||
let parent_watcher = parent::ParentWatcher::new();
|
||||
match &parent_watcher {
|
||||
Some(watcher) => tracing::info!(
|
||||
"Watching parent process (pid {}); will exit when it dies",
|
||||
watcher.parent_pid()
|
||||
),
|
||||
None => tracing::warn!(
|
||||
"Parent process liveness detection unavailable; idle timeout will exit unconditionally"
|
||||
),
|
||||
}
|
||||
|
||||
if idle_timeout_secs > 0 || parent_watcher.is_some() {
|
||||
last_activity.store(
|
||||
std::time::SystemTime::now()
|
||||
SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0),
|
||||
@@ -354,9 +369,27 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
|
||||
let last_activity_for_watchdog = last_activity.clone();
|
||||
tokio::spawn(async move {
|
||||
let tick = std::time::Duration::from_secs(60);
|
||||
let tick = watchdog_interval();
|
||||
loop {
|
||||
tokio::time::sleep(tick).await;
|
||||
|
||||
if let Some(ref watcher) = parent_watcher {
|
||||
if !watcher.parent_alive() {
|
||||
tracing::info!(
|
||||
"Parent process (pid {}) exited, shutting down",
|
||||
watcher.parent_pid()
|
||||
);
|
||||
flush_logs_and_exit().await;
|
||||
}
|
||||
// Parent is alive: it owns our lifecycle, never exit on idle
|
||||
// Clients like Codex do not restart MCP servers @see #703
|
||||
continue;
|
||||
}
|
||||
|
||||
if idle_timeout_secs == 0 {
|
||||
continue;
|
||||
}
|
||||
|
||||
let now = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
@@ -364,12 +397,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
|
||||
let last = last_activity_for_watchdog.load(std::sync::atomic::Ordering::Relaxed);
|
||||
if now.saturating_sub(last) >= idle_timeout_secs {
|
||||
tracing::info!(
|
||||
"Exiting after {}s of inactivity (idle_timeout_secs={})",
|
||||
now.saturating_sub(last),
|
||||
idle_timeout_secs
|
||||
);
|
||||
std::process::exit(0);
|
||||
tracing::info!(?idle_timeout_secs, "Exiting due to inactivity",);
|
||||
flush_logs_and_exit().await;
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -396,3 +425,20 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Tracing appender is non blocking, to get full log give it some time before hard exit
|
||||
async fn flush_logs_and_exit() -> ! {
|
||||
tokio::time::sleep(std::time::Duration::from_millis(250)).await;
|
||||
std::process::exit(0);
|
||||
}
|
||||
|
||||
fn watchdog_interval() -> Duration {
|
||||
if cfg!(debug_assertions)
|
||||
&& let Some(milliseconds) = std::env::var("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS")
|
||||
.ok()
|
||||
.and_then(|value| value.parse().ok())
|
||||
{
|
||||
return Duration::from_millis(milliseconds);
|
||||
}
|
||||
Duration::from_secs(60)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
#[cfg(unix)]
|
||||
mod imp {
|
||||
pub struct ParentWatcher {
|
||||
ppid: u32,
|
||||
}
|
||||
|
||||
impl ParentWatcher {
|
||||
pub fn new() -> Option<Self> {
|
||||
let ppid = std::os::unix::process::parent_id();
|
||||
// ppid <= 1 means we were spawned by init and can't detect death
|
||||
(ppid > 1).then_some(Self { ppid })
|
||||
}
|
||||
|
||||
pub fn parent_pid(&self) -> u32 {
|
||||
self.ppid
|
||||
}
|
||||
|
||||
// When the parent dies the kernel reparents us, so getppid() changes.
|
||||
// Race-free and immune to PID reuse, unlike kill(ppid, 0).
|
||||
pub fn parent_alive(&self) -> bool {
|
||||
std::os::unix::process::parent_id() == self.ppid
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
mod imp {
|
||||
use windows_sys::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE, WAIT_TIMEOUT};
|
||||
use windows_sys::Win32::System::Diagnostics::ToolHelp::{
|
||||
CreateToolhelp32Snapshot, PROCESSENTRY32, Process32First, Process32Next, TH32CS_SNAPPROCESS,
|
||||
};
|
||||
use windows_sys::Win32::System::Threading::{
|
||||
GetCurrentProcessId, OpenProcess, PROCESS_SYNCHRONIZE, WaitForSingleObject,
|
||||
};
|
||||
|
||||
pub struct ParentWatcher {
|
||||
handle: HANDLE,
|
||||
ppid: u32,
|
||||
}
|
||||
|
||||
// HANDLE is a raw pointer; it is only ever used via WaitForSingleObject
|
||||
// which is thread-safe, so moving/sharing the watcher across threads is fine.
|
||||
unsafe impl Send for ParentWatcher {}
|
||||
unsafe impl Sync for ParentWatcher {}
|
||||
|
||||
impl ParentWatcher {
|
||||
pub fn new() -> Option<Self> {
|
||||
let ppid = parent_pid_of_current()?;
|
||||
let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, ppid) };
|
||||
if handle.is_null() {
|
||||
return None;
|
||||
}
|
||||
// Holding the handle pins the PID, preventing reuse for the process lifetime
|
||||
Some(Self { handle, ppid })
|
||||
}
|
||||
|
||||
pub fn parent_pid(&self) -> u32 {
|
||||
self.ppid
|
||||
}
|
||||
|
||||
pub fn parent_alive(&self) -> bool {
|
||||
unsafe { WaitForSingleObject(self.handle, 0) == WAIT_TIMEOUT }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ParentWatcher {
|
||||
fn drop(&mut self) {
|
||||
unsafe { CloseHandle(self.handle) };
|
||||
}
|
||||
}
|
||||
|
||||
fn parent_pid_of_current() -> Option<u32> {
|
||||
unsafe {
|
||||
let snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
|
||||
if snapshot == INVALID_HANDLE_VALUE {
|
||||
return None;
|
||||
}
|
||||
let mut entry: PROCESSENTRY32 = std::mem::zeroed();
|
||||
entry.dwSize = std::mem::size_of::<PROCESSENTRY32>() as u32;
|
||||
let current = GetCurrentProcessId();
|
||||
let mut found = None;
|
||||
if Process32First(snapshot, &mut entry) != 0 {
|
||||
loop {
|
||||
if entry.th32ProcessID == current {
|
||||
found = Some(entry.th32ParentProcessID);
|
||||
break;
|
||||
}
|
||||
if Process32Next(snapshot, &mut entry) == 0 {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
CloseHandle(snapshot);
|
||||
found
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub use imp::ParentWatcher;
|
||||
@@ -0,0 +1,181 @@
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::process::{Child, ChildStdin, Command, Stdio};
|
||||
use std::sync::mpsc;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
const BIN: &str = env!("CARGO_BIN_EXE_fff-mcp");
|
||||
|
||||
#[test]
|
||||
fn stays_alive_while_parent_alive_despite_idle_timeout() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("a.txt"), "hello").unwrap();
|
||||
|
||||
let mut child = Command::new(BIN)
|
||||
.arg(dir.path())
|
||||
.args([
|
||||
"--no-update-check",
|
||||
"--no-warmup",
|
||||
"--no-watch",
|
||||
"--idle-timeout-secs",
|
||||
"1",
|
||||
])
|
||||
.arg("--log-file")
|
||||
.arg(dir.path().join("test.log"))
|
||||
.env("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS", "100")
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.unwrap();
|
||||
|
||||
let mut stdin = child.stdin.take().unwrap();
|
||||
let stdout_lines = spawn_line_reader(child.stdout.take().unwrap());
|
||||
do_handshake(&mut stdin, &stdout_lines);
|
||||
|
||||
// Wait past the idle timeout and several watchdog ticks.
|
||||
std::thread::sleep(Duration::from_secs(2));
|
||||
assert!(
|
||||
child.try_wait().unwrap().is_none(),
|
||||
"fff-mcp exited on idle timeout even though its parent is alive"
|
||||
);
|
||||
|
||||
// Closing stdin ends the transport; the server must still shut down cleanly.
|
||||
drop(stdin);
|
||||
wait_for_exit(&mut child, Duration::from_secs(15));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn exits_when_parent_dies_even_without_idle_timeout() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("a.txt"), "hello").unwrap();
|
||||
let log_path = dir.path().join("test.log");
|
||||
let exit_signal = dir.path().join("exit-parent");
|
||||
|
||||
// Intermediary parent: sh backgrounds fff-mcp and waits until the handshake
|
||||
// completes before dying and orphaning it.
|
||||
let mut sh = Command::new("sh")
|
||||
.arg("-c")
|
||||
.arg(
|
||||
// Preserve stdin before POSIX shells assign /dev/null to background jobs.
|
||||
r#"exec 3<&0
|
||||
"$1" "$2" --no-update-check --no-warmup --no-watch \
|
||||
--idle-timeout-secs 0 --log-file "$3" <&3 &
|
||||
while [ ! -e "$4" ]; do sleep 0.1; done"#,
|
||||
)
|
||||
.arg("sh")
|
||||
.arg(BIN)
|
||||
.arg(dir.path())
|
||||
.arg(&log_path)
|
||||
.arg(&exit_signal)
|
||||
.env("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS", "100")
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.unwrap();
|
||||
|
||||
let mut stdin = sh.stdin.take().unwrap();
|
||||
let stdout_lines = spawn_line_reader(sh.stdout.take().unwrap());
|
||||
do_handshake(&mut stdin, &stdout_lines);
|
||||
|
||||
std::fs::write(exit_signal, "").unwrap();
|
||||
sh.wait().unwrap();
|
||||
|
||||
// We still hold the stdin write end, so the only exit path is the parent
|
||||
// liveness check. EOF on stdout means fff-mcp closed it by exiting.
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
loop {
|
||||
match stdout_lines.recv_timeout(deadline.saturating_duration_since(Instant::now())) {
|
||||
Ok(_) => continue,
|
||||
Err(mpsc::RecvTimeoutError::Disconnected) => break,
|
||||
Err(mpsc::RecvTimeoutError::Timeout) => {
|
||||
panic!("fff-mcp did not exit within 5s of its parent dying")
|
||||
}
|
||||
}
|
||||
}
|
||||
drop(stdin);
|
||||
|
||||
let logs = read_session_logs(dir.path());
|
||||
assert!(
|
||||
logs.contains("Parent process") && logs.contains("exited, shutting down"),
|
||||
"expected parent-death exit reason in logs, got:\n{}",
|
||||
logs
|
||||
);
|
||||
}
|
||||
|
||||
fn do_handshake(stdin: &mut ChildStdin, stdout_lines: &mpsc::Receiver<String>) {
|
||||
let initialize = serde_json::json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 1,
|
||||
"method": "initialize",
|
||||
"params": {
|
||||
"protocolVersion": "2024-11-05",
|
||||
"capabilities": {},
|
||||
"clientInfo": { "name": "parent-liveness-test", "version": "0.0.0" }
|
||||
}
|
||||
});
|
||||
writeln!(stdin, "{}", initialize).unwrap();
|
||||
stdin.flush().unwrap();
|
||||
|
||||
let response = stdout_lines
|
||||
.recv_timeout(Duration::from_secs(30))
|
||||
.expect("no initialize response within 30s");
|
||||
assert!(
|
||||
response.contains("\"serverInfo\""),
|
||||
"unexpected initialize response: {}",
|
||||
response
|
||||
);
|
||||
|
||||
writeln!(
|
||||
stdin,
|
||||
"{}",
|
||||
serde_json::json!({ "jsonrpc": "2.0", "method": "notifications/initialized" })
|
||||
)
|
||||
.unwrap();
|
||||
stdin.flush().unwrap();
|
||||
}
|
||||
|
||||
fn spawn_line_reader(stdout: std::process::ChildStdout) -> mpsc::Receiver<String> {
|
||||
let (tx, rx) = mpsc::channel();
|
||||
std::thread::spawn(move || {
|
||||
for line in BufReader::new(stdout).lines() {
|
||||
match line {
|
||||
Ok(line) => {
|
||||
if tx.send(line).is_err() {
|
||||
break;
|
||||
}
|
||||
}
|
||||
Err(_) => break,
|
||||
}
|
||||
}
|
||||
});
|
||||
rx
|
||||
}
|
||||
|
||||
fn wait_for_exit(child: &mut Child, timeout: Duration) {
|
||||
let deadline = Instant::now() + timeout;
|
||||
while Instant::now() < deadline {
|
||||
if child.try_wait().unwrap().is_some() {
|
||||
return;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(100));
|
||||
}
|
||||
child.kill().ok();
|
||||
panic!(
|
||||
"fff-mcp did not exit within {:?} after stdin closed",
|
||||
timeout
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn read_session_logs(dir: &std::path::Path) -> String {
|
||||
let mut combined = String::new();
|
||||
for entry in std::fs::read_dir(dir).unwrap().flatten() {
|
||||
let name = entry.file_name().to_string_lossy().to_string();
|
||||
if name.starts_with("test") && name.ends_with(".log") {
|
||||
combined.push_str(&std::fs::read_to_string(entry.path()).unwrap_or_default());
|
||||
}
|
||||
}
|
||||
combined
|
||||
}
|
||||
Reference in New Issue
Block a user