Compare commits
2 Commits
fix/754
...
fix/issue-
| Author | SHA1 | Date | |
|---|---|---|---|
| 590b601b5c | |||
| c4aad087d4 |
75
tests/test_mcp_pid_lock.py
Normal file
75
tests/test_mcp_pid_lock.py
Normal file
@@ -0,0 +1,75 @@
|
|||||||
|
"""Tests for MCP PID file lock (#734)."""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
import tempfile
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
sys.path.insert(0, str(Path(__file__).parent.parent))
|
||||||
|
|
||||||
|
# Override MCP_DIR for testing
|
||||||
|
import tools.mcp_pid_lock as lock_mod
|
||||||
|
_test_dir = Path(tempfile.mkdtemp())
|
||||||
|
lock_mod._MCP_DIR = _test_dir
|
||||||
|
|
||||||
|
|
||||||
|
def test_acquire_and_release():
|
||||||
|
"""Lock can be acquired and released."""
|
||||||
|
pid = lock_mod.acquire_lock("test_server")
|
||||||
|
assert pid == os.getpid()
|
||||||
|
assert lock_mod.is_locked("test_server")
|
||||||
|
lock_mod.release_lock("test_server")
|
||||||
|
assert not lock_mod.is_locked("test_server")
|
||||||
|
|
||||||
|
|
||||||
|
def test_concurrent_lock_blocked():
|
||||||
|
"""Second acquire returns None when server running."""
|
||||||
|
lock_mod.acquire_lock("test_concurrent")
|
||||||
|
result = lock_mod.acquire_lock("test_concurrent")
|
||||||
|
assert result is None
|
||||||
|
lock_mod.release_lock("test_concurrent")
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_lock_cleaned():
|
||||||
|
"""Stale PID files are cleaned up."""
|
||||||
|
# Write a fake stale PID
|
||||||
|
pid_file = _test_dir / "stale.pid"
|
||||||
|
pid_file.write_text("99999999")
|
||||||
|
assert not lock_mod.is_locked("stale")
|
||||||
|
assert not pid_file.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_list_locks():
|
||||||
|
"""list_locks returns only active locks."""
|
||||||
|
lock_mod.acquire_lock("list_test")
|
||||||
|
locks = lock_mod.list_locks()
|
||||||
|
assert "list_test" in locks
|
||||||
|
assert locks["list_test"] == os.getpid()
|
||||||
|
lock_mod.release_lock("list_test")
|
||||||
|
|
||||||
|
|
||||||
|
def test_cleanup_stale():
|
||||||
|
"""cleanup_stale_locks removes dead PID files."""
|
||||||
|
(_test_dir / "dead1.pid").write_text("99999998")
|
||||||
|
(_test_dir / "dead2.pid").write_text("99999999")
|
||||||
|
count = lock_mod.cleanup_stale_locks()
|
||||||
|
assert count >= 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_force_release():
|
||||||
|
"""force_release kills process and removes lock."""
|
||||||
|
lock_mod.acquire_lock("force_test")
|
||||||
|
assert lock_mod.is_locked("force_test")
|
||||||
|
lock_mod.force_release("force_test")
|
||||||
|
assert not lock_mod.is_locked("force_test")
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
tests = [test_acquire_and_release, test_concurrent_lock_blocked,
|
||||||
|
test_stale_lock_cleaned, test_list_locks, test_cleanup_stale,
|
||||||
|
test_force_release]
|
||||||
|
for t in tests:
|
||||||
|
print(f"Running {t.__name__}...")
|
||||||
|
t()
|
||||||
|
print(" PASS")
|
||||||
|
print("\nAll tests passed.")
|
||||||
158
tools/mcp_pid_lock.py
Normal file
158
tools/mcp_pid_lock.py
Normal file
@@ -0,0 +1,158 @@
|
|||||||
|
"""
|
||||||
|
MCP PID File Lock — Prevent concurrent MCP server instances.
|
||||||
|
|
||||||
|
Uses PID files at ~/.hermes/mcp/{name}.pid to ensure only one instance
|
||||||
|
of each MCP server runs at a time. Prevents zombie accumulation (#714).
|
||||||
|
|
||||||
|
Usage:
|
||||||
|
from tools.mcp_pid_lock import acquire_lock, release_lock, is_locked
|
||||||
|
|
||||||
|
lock = acquire_lock("morrowind")
|
||||||
|
if lock:
|
||||||
|
try:
|
||||||
|
# run server
|
||||||
|
pass
|
||||||
|
finally:
|
||||||
|
release_lock("morrowind")
|
||||||
|
"""
|
||||||
|
|
||||||
|
import fcntl
|
||||||
|
import os
|
||||||
|
import signal
|
||||||
|
import time
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Optional
|
||||||
|
|
||||||
|
_MCP_DIR = Path(os.getenv("HERMES_HOME", str(Path.home() / ".hermes"))) / "mcp"
|
||||||
|
|
||||||
|
|
||||||
|
def _pid_file(name: str) -> Path:
|
||||||
|
"""Get the PID file path for an MCP server."""
|
||||||
|
_MCP_DIR.mkdir(parents=True, exist_ok=True)
|
||||||
|
return _MCP_DIR / f"{name}.pid"
|
||||||
|
|
||||||
|
|
||||||
|
def _is_process_alive(pid: int) -> bool:
|
||||||
|
"""Check if a process is running."""
|
||||||
|
try:
|
||||||
|
os.kill(pid, 0) # Signal 0 = check if alive
|
||||||
|
return True
|
||||||
|
except ProcessLookupError:
|
||||||
|
return False
|
||||||
|
except PermissionError:
|
||||||
|
return True # Exists but we can't signal it
|
||||||
|
|
||||||
|
|
||||||
|
def _read_pid_file(name: str) -> Optional[int]:
|
||||||
|
"""Read PID from file, returns None if invalid."""
|
||||||
|
path = _pid_file(name)
|
||||||
|
if not path.exists():
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
content = path.read_text().strip()
|
||||||
|
return int(content) if content else None
|
||||||
|
except (ValueError, OSError):
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _write_pid_file(name: str, pid: int):
|
||||||
|
"""Write PID to file."""
|
||||||
|
path = _pid_file(name)
|
||||||
|
path.write_text(str(pid))
|
||||||
|
|
||||||
|
|
||||||
|
def _remove_pid_file(name: str):
|
||||||
|
"""Remove PID file."""
|
||||||
|
path = _pid_file(name)
|
||||||
|
try:
|
||||||
|
path.unlink()
|
||||||
|
except FileNotFoundError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def is_locked(name: str) -> bool:
|
||||||
|
"""Check if an MCP server is already running."""
|
||||||
|
pid = _read_pid_file(name)
|
||||||
|
if pid is None:
|
||||||
|
return False
|
||||||
|
if _is_process_alive(pid):
|
||||||
|
return True
|
||||||
|
# Stale PID file
|
||||||
|
_remove_pid_file(name)
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def acquire_lock(name: str) -> Optional[int]:
|
||||||
|
"""
|
||||||
|
Acquire a PID lock for an MCP server.
|
||||||
|
|
||||||
|
Returns the PID if lock acquired, None if server already running.
|
||||||
|
"""
|
||||||
|
# Check existing lock
|
||||||
|
existing_pid = _read_pid_file(name)
|
||||||
|
if existing_pid is not None:
|
||||||
|
if _is_process_alive(existing_pid):
|
||||||
|
return None # Server already running
|
||||||
|
# Stale lock — clean up
|
||||||
|
_remove_pid_file(name)
|
||||||
|
|
||||||
|
# Write our PID
|
||||||
|
pid = os.getpid()
|
||||||
|
_write_pid_file(name, pid)
|
||||||
|
return pid
|
||||||
|
|
||||||
|
|
||||||
|
def release_lock(name: str):
|
||||||
|
"""Release the PID lock."""
|
||||||
|
# Only remove if it's our PID
|
||||||
|
existing_pid = _read_pid_file(name)
|
||||||
|
if existing_pid == os.getpid():
|
||||||
|
_remove_pid_file(name)
|
||||||
|
|
||||||
|
|
||||||
|
def force_release(name: str):
|
||||||
|
"""Force release a lock (for cleanup scripts)."""
|
||||||
|
pid = _read_pid_file(name)
|
||||||
|
if pid and _is_process_alive(pid):
|
||||||
|
try:
|
||||||
|
os.kill(pid, signal.SIGTERM)
|
||||||
|
time.sleep(0.5)
|
||||||
|
if _is_process_alive(pid):
|
||||||
|
os.kill(pid, signal.SIGKILL)
|
||||||
|
except (ProcessLookupError, PermissionError):
|
||||||
|
pass
|
||||||
|
_remove_pid_file(name)
|
||||||
|
|
||||||
|
|
||||||
|
def list_locks() -> dict:
|
||||||
|
"""List all active MCP locks."""
|
||||||
|
locks = {}
|
||||||
|
if not _MCP_DIR.exists():
|
||||||
|
return locks
|
||||||
|
|
||||||
|
for pid_file in _MCP_DIR.glob("*.pid"):
|
||||||
|
name = pid_file.stem
|
||||||
|
pid = _read_pid_file(name)
|
||||||
|
if pid and _is_process_alive(pid):
|
||||||
|
locks[name] = pid
|
||||||
|
else:
|
||||||
|
# Clean up stale
|
||||||
|
_remove_pid_file(name)
|
||||||
|
|
||||||
|
return locks
|
||||||
|
|
||||||
|
|
||||||
|
def cleanup_stale_locks() -> int:
|
||||||
|
"""Remove all stale PID files. Returns count cleaned."""
|
||||||
|
cleaned = 0
|
||||||
|
if not _MCP_DIR.exists():
|
||||||
|
return 0
|
||||||
|
|
||||||
|
for pid_file in _MCP_DIR.glob("*.pid"):
|
||||||
|
name = pid_file.stem
|
||||||
|
pid = _read_pid_file(name)
|
||||||
|
if pid is None or not _is_process_alive(pid):
|
||||||
|
_remove_pid_file(name)
|
||||||
|
cleaned += 1
|
||||||
|
|
||||||
|
return cleaned
|
||||||
Reference in New Issue
Block a user