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