fix(embed): clear stale daemon on port before starting (#843)
* fix(embed): clear stale daemon on port before starting new one (#843) When `uvx hindsight-embed@latest` resolves to a new version, the old daemon may still be bound to the port, causing EADDRINUSE. Before starting a daemon, check if the port is occupied, verify it's a hindsight process via /health, and SIGTERM it if so. * chore: remove unused signal import from test * refactor: use cross-platform port check instead of lsof-only Use socket for port check (works on all platforms), extract PID lookup into a helper with Windows (netstat) and Unix (lsof) paths, and extract kill logic into a testable static method. * refactor: reuse cross-platform helpers in stop() and stop_ui()
This commit is contained in:
Nicolò Boschi
GitHub
parent
26a64cc00e
commit
7d6c570a3a
2 changed files with 205 additions and 50 deletions
|
|
@ -101,6 +101,102 @@ class DaemonEmbedManager(EmbedManager):
|
|||
api_version = os.getenv("HINDSIGHT_EMBED_API_VERSION", __version__)
|
||||
return ["uvx", f"hindsight-api@{api_version}"]
|
||||
|
||||
@staticmethod
|
||||
def _is_port_in_use(port: int) -> bool:
|
||||
"""Check if a port is in use using a socket connection (cross-platform)."""
|
||||
import socket
|
||||
|
||||
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
|
||||
sock.settimeout(1)
|
||||
return sock.connect_ex(("127.0.0.1", port)) == 0
|
||||
|
||||
@staticmethod
|
||||
def _find_pid_on_port(port: int) -> int | None:
|
||||
"""Find the PID of the process listening on a port."""
|
||||
import platform
|
||||
|
||||
try:
|
||||
if platform.system() == "Windows":
|
||||
# Use netstat on Windows
|
||||
result = subprocess.run(
|
||||
["netstat", "-ano", "-p", "TCP"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
for line in result.stdout.splitlines():
|
||||
if f"127.0.0.1:{port}" in line and "LISTENING" in line:
|
||||
return int(line.strip().split()[-1])
|
||||
else:
|
||||
# Use lsof on macOS/Linux
|
||||
result = subprocess.run(
|
||||
["lsof", "-ti", f":{port}", "-sTCP:LISTEN"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode == 0 and result.stdout.strip():
|
||||
return int(result.stdout.strip().split()[0])
|
||||
except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError):
|
||||
pass
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _kill_process(pid: int) -> bool:
|
||||
"""Kill a process by PID and wait for it to exit. Returns True if process is gone."""
|
||||
import signal
|
||||
|
||||
try:
|
||||
os.kill(pid, signal.SIGTERM)
|
||||
for _ in range(50):
|
||||
time.sleep(0.1)
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
return True
|
||||
except OSError:
|
||||
return True # Already gone
|
||||
return False
|
||||
|
||||
def _clear_port(self, port: int) -> bool:
|
||||
"""
|
||||
Ensure the port is free before starting a daemon.
|
||||
|
||||
If the port is occupied by a hindsight daemon, stop it gracefully.
|
||||
If occupied by something else, return False.
|
||||
|
||||
Returns:
|
||||
True if port is free (or was freed), False if occupied by non-hindsight process.
|
||||
"""
|
||||
if not self._is_port_in_use(port):
|
||||
return True
|
||||
|
||||
# Port is occupied — check if it's a hindsight daemon via /health
|
||||
try:
|
||||
with httpx.Client(timeout=2) as client:
|
||||
response = client.get(f"http://127.0.0.1:{port}/health")
|
||||
if response.status_code != 200:
|
||||
logger.warning(f"Port {port} is in use by another process")
|
||||
return False
|
||||
except Exception:
|
||||
logger.warning(f"Port {port} is in use by another process")
|
||||
return False
|
||||
|
||||
# It's a hindsight daemon — find its PID and stop it
|
||||
pid = self._find_pid_on_port(port)
|
||||
if pid is None:
|
||||
logger.warning(f"Port {port} has a hindsight daemon but could not find its PID")
|
||||
return False
|
||||
|
||||
logger.info(f"Stopping existing daemon on port {port} (PID {pid})")
|
||||
if self._kill_process(pid):
|
||||
logger.info(f"Old daemon (PID {pid}) stopped")
|
||||
return True
|
||||
|
||||
logger.warning(f"Old daemon (PID {pid}) did not stop in time")
|
||||
return False
|
||||
|
||||
def _start_daemon(self, config: dict, profile: str) -> bool:
|
||||
"""Start the daemon in background."""
|
||||
paths = self._profile_manager.resolve_profile_paths(profile)
|
||||
|
|
@ -108,6 +204,11 @@ class DaemonEmbedManager(EmbedManager):
|
|||
daemon_log = paths.log
|
||||
port = paths.port
|
||||
|
||||
# Ensure port is free before starting (handles stale daemons from version upgrades)
|
||||
if not self._clear_port(port):
|
||||
logger.error(f"Cannot start daemon: port {port} is in use by a non-hindsight process")
|
||||
return False
|
||||
|
||||
# Load profile's .env file and merge with provided config
|
||||
# This fixes issue #305 where profile env vars were ignored
|
||||
profile_config = self._profile_manager.load_profile_config(profile)
|
||||
|
|
@ -502,30 +603,12 @@ class DaemonEmbedManager(EmbedManager):
|
|||
logger.debug(f"UI not running for profile '{profile}'")
|
||||
return True
|
||||
|
||||
# Find PID by port
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["lsof", "-ti", f":{ui_port}", "-sTCP:LISTEN"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode == 0 and result.stdout.strip():
|
||||
pid = int(result.stdout.strip().split()[0])
|
||||
logger.debug(f"Found UI PID {pid} on port {ui_port}")
|
||||
os.kill(pid, 15)
|
||||
|
||||
# Wait for process to exit
|
||||
for _ in range(50):
|
||||
time.sleep(0.1)
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
break
|
||||
else:
|
||||
logger.warning(f"Could not find PID for UI port {ui_port}")
|
||||
except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError) as e:
|
||||
logger.warning(f"Could not find/kill UI by port: {e}")
|
||||
pid = self._find_pid_on_port(ui_port)
|
||||
if pid is not None:
|
||||
logger.debug(f"Found UI PID {pid} on port {ui_port}")
|
||||
self._kill_process(pid)
|
||||
else:
|
||||
logger.warning(f"Could not find PID for UI port {ui_port}")
|
||||
|
||||
# Wait for health check to fail
|
||||
for _ in range(30):
|
||||
|
|
@ -572,32 +655,12 @@ class DaemonEmbedManager(EmbedManager):
|
|||
paths = self._profile_manager.resolve_profile_paths(profile)
|
||||
port = paths.port
|
||||
|
||||
# Find PID by port
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["lsof", "-ti", f":{port}", "-sTCP:LISTEN"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
if result.returncode == 0 and result.stdout.strip():
|
||||
pid = int(result.stdout.strip().split()[0])
|
||||
logger.debug(f"Found daemon PID {pid} on port {port}")
|
||||
|
||||
# Send SIGTERM
|
||||
os.kill(pid, 15)
|
||||
|
||||
# Wait for process to exit
|
||||
for _ in range(50):
|
||||
time.sleep(0.1)
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except OSError:
|
||||
break
|
||||
else:
|
||||
logger.warning(f"Could not find PID for port {port}")
|
||||
except (subprocess.TimeoutExpired, ValueError, OSError, FileNotFoundError) as e:
|
||||
logger.warning(f"Could not find/kill daemon by port: {e}")
|
||||
pid = self._find_pid_on_port(port)
|
||||
if pid is not None:
|
||||
logger.debug(f"Found daemon PID {pid} on port {port}")
|
||||
self._kill_process(pid)
|
||||
else:
|
||||
logger.warning(f"Could not find PID for port {port}")
|
||||
|
||||
# Wait for health check to fail
|
||||
for _ in range(30):
|
||||
|
|
|
|||
|
|
@ -5,9 +5,11 @@ import subprocess
|
|||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from hindsight_embed import daemon_client
|
||||
from hindsight_embed.daemon_embed_manager import DaemonEmbedManager
|
||||
|
||||
@pytest.fixture
|
||||
def config():
|
||||
|
|
@ -178,3 +180,93 @@ class TestRunCli:
|
|||
# Verify exit code
|
||||
assert exit_code == 0
|
||||
|
||||
|
||||
class TestClearPort:
|
||||
"""Tests for DaemonEmbedManager._clear_port."""
|
||||
|
||||
def test_port_free(self):
|
||||
"""Port not in use — returns True immediately."""
|
||||
manager = DaemonEmbedManager()
|
||||
with patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=False):
|
||||
assert manager._clear_port(9555) is True
|
||||
|
||||
def test_port_occupied_by_hindsight_stops_it(self):
|
||||
"""Port occupied by a hindsight daemon — kills it and returns True."""
|
||||
manager = DaemonEmbedManager()
|
||||
with (
|
||||
patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True),
|
||||
patch("httpx.Client") as mock_httpx_cls,
|
||||
patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=12345),
|
||||
patch.object(DaemonEmbedManager, "_kill_process", return_value=True),
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.__enter__ = Mock(return_value=mock_client)
|
||||
mock_client.__exit__ = Mock(return_value=False)
|
||||
mock_client.get.return_value = Mock(status_code=200)
|
||||
mock_httpx_cls.return_value = mock_client
|
||||
|
||||
assert manager._clear_port(9555) is True
|
||||
|
||||
def test_port_occupied_by_non_hindsight_returns_false(self):
|
||||
"""Port occupied by non-hindsight process — returns False."""
|
||||
manager = DaemonEmbedManager()
|
||||
with (
|
||||
patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True),
|
||||
patch("httpx.Client") as mock_httpx_cls,
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.__enter__ = Mock(return_value=mock_client)
|
||||
mock_client.__exit__ = Mock(return_value=False)
|
||||
mock_client.get.side_effect = httpx.ConnectError("Connection refused")
|
||||
mock_httpx_cls.return_value = mock_client
|
||||
|
||||
assert manager._clear_port(9555) is False
|
||||
|
||||
def test_port_occupied_health_non_200_returns_false(self):
|
||||
"""Port responds but not with 200 — treated as non-hindsight."""
|
||||
manager = DaemonEmbedManager()
|
||||
with (
|
||||
patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True),
|
||||
patch("httpx.Client") as mock_httpx_cls,
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.__enter__ = Mock(return_value=mock_client)
|
||||
mock_client.__exit__ = Mock(return_value=False)
|
||||
mock_client.get.return_value = Mock(status_code=404)
|
||||
mock_httpx_cls.return_value = mock_client
|
||||
|
||||
assert manager._clear_port(9555) is False
|
||||
|
||||
def test_pid_not_found_returns_false(self):
|
||||
"""Hindsight daemon on port but can't find PID — returns False."""
|
||||
manager = DaemonEmbedManager()
|
||||
with (
|
||||
patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True),
|
||||
patch("httpx.Client") as mock_httpx_cls,
|
||||
patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=None),
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.__enter__ = Mock(return_value=mock_client)
|
||||
mock_client.__exit__ = Mock(return_value=False)
|
||||
mock_client.get.return_value = Mock(status_code=200)
|
||||
mock_httpx_cls.return_value = mock_client
|
||||
|
||||
assert manager._clear_port(9555) is False
|
||||
|
||||
def test_kill_fails_returns_false(self):
|
||||
"""Hindsight daemon found but won't die — returns False."""
|
||||
manager = DaemonEmbedManager()
|
||||
with (
|
||||
patch.object(DaemonEmbedManager, "_is_port_in_use", return_value=True),
|
||||
patch("httpx.Client") as mock_httpx_cls,
|
||||
patch.object(DaemonEmbedManager, "_find_pid_on_port", return_value=12345),
|
||||
patch.object(DaemonEmbedManager, "_kill_process", return_value=False),
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.__enter__ = Mock(return_value=mock_client)
|
||||
mock_client.__exit__ = Mock(return_value=False)
|
||||
mock_client.get.return_value = Mock(status_code=200)
|
||||
mock_httpx_cls.return_value = mock_client
|
||||
|
||||
assert manager._clear_port(9555) is False
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue