Skip to content

Commit a735370

Browse files
committed
fix: prevent race condition in MCP server start
Make singleton enforcement atomic by checking for existing MCP server inside the register_mcp_serve_process() lock. This prevents two concurrent start commands from both passing the check and spawning duplicate processes. Additional improvements: - Use std::mem::forget() to document intentional process detachment - Kill spawned process if registration fails (cleanup on race loss) - Move singleton check comment from caller to enforcement point
1 parent cb8bb77 commit a735370

2 files changed

Lines changed: 44 additions & 19 deletions

File tree

src-tauri/src/commands/mcp.rs

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -628,14 +628,6 @@ pub async fn mcp_serve(
628628
}
629629
};
630630

631-
// If already running, don't start another one
632-
if let Ok(Some(existing)) = registry.0.get_running_mcp_serve() {
633-
return Ok(format!(
634-
"Claude Code MCP server already running (PID: {})",
635-
existing.pid
636-
));
637-
}
638-
639631
let mut cmd = create_command_with_env(&claude_path);
640632
cmd.arg("mcp").arg("serve");
641633

@@ -647,13 +639,34 @@ pub async fn mcp_serve(
647639
return Err("MCP server started but PID is unavailable".to_string());
648640
}
649641

650-
if let Err(e) = registry.0.register_mcp_serve_process(pid) {
651-
error!("Failed to register MCP server process: {}", e);
652-
return Err(e);
653-
}
642+
// Intentionally drop the child handle - MCP server runs as detached process.
643+
// We track it by PID only, allowing it to outlive the parent process.
644+
std::mem::forget(child);
654645

655-
info!("Successfully started Claude Code MCP server (PID: {})", pid);
656-
Ok(format!("Claude Code MCP server started (PID: {})", pid))
646+
// Register atomically - will fail if another instance already exists
647+
match registry.0.register_mcp_serve_process(pid) {
648+
Ok(_run_id) => {
649+
info!("Successfully started Claude Code MCP server (PID: {})", pid);
650+
Ok(format!("Claude Code MCP server started (PID: {})", pid))
651+
}
652+
Err(e) => {
653+
// Registration failed (likely already running) - kill the process we just started
654+
error!("Failed to register MCP server process: {}", e);
655+
#[cfg(unix)]
656+
{
657+
use std::process::Command;
658+
let _ = Command::new("kill").arg(pid.to_string()).spawn();
659+
}
660+
#[cfg(windows)]
661+
{
662+
use std::process::Command;
663+
let _ = Command::new("taskkill")
664+
.args(["/PID", &pid.to_string(), "/F"])
665+
.spawn();
666+
}
667+
Err(e)
668+
}
669+
}
657670
}
658671
Err(e) => {
659672
error!("Failed to start MCP server: {}", e);

src-tauri/src/process/registry.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,24 @@ impl ProcessRegistry {
157157
/// Register a long-running MCP serve process (stores PID only, no child handle)
158158
///
159159
/// NOTE: Only ONE MCP serve process should run at a time (singleton pattern).
160-
/// The caller (mcp_serve command) is responsible for checking if a process
161-
/// already exists via get_running_mcp_serve() before calling this method.
160+
/// This method enforces the singleton by checking atomically within the lock.
162161
pub fn register_mcp_serve_process(&self, pid: u32) -> Result<i64, String> {
162+
// Acquire lock first to make check-and-register atomic (prevents race conditions)
163+
let mut processes = self.processes.lock().map_err(|e| e.to_string())?;
164+
165+
// Check if MCP serve process already exists (atomic with registration)
166+
let existing_mcp = processes.values().find(|p| {
167+
matches!(p.info.process_type, ProcessType::McpServe)
168+
});
169+
170+
if let Some(existing) = existing_mcp {
171+
return Err(format!(
172+
"MCP server already running (PID: {})",
173+
existing.info.pid
174+
));
175+
}
176+
177+
// Now safe to register new MCP serve process
163178
let run_id = self.generate_id()?;
164179

165180
let process_info = ProcessInfo {
@@ -172,9 +187,6 @@ impl ProcessRegistry {
172187
model: "".to_string(),
173188
};
174189

175-
// Register without child handle (like sidecar)
176-
let mut processes = self.processes.lock().map_err(|e| e.to_string())?;
177-
178190
let process_handle = ProcessHandle {
179191
info: process_info,
180192
child: Arc::new(Mutex::new(None)),

0 commit comments

Comments
 (0)