fix: prevent service crash-loop on port-in-use at startup
serve() now kills any stale process holding the configured port before uvicorn tries to bind. Prevents the crash-loop where systemd restarts muxplex but the old process is still holding port 8088 (observed: 2075+ restarts before manual intervention). New helper _kill_stale_port_holder(port): - Runs lsof -ti :<port> to find occupying PIDs - Sends SIGTERM to all foreign PIDs (skips own PID) - Waits 1 second for the port to free - Silently swallows all errors (missing lsof, permission denied) so a broken environment never prevents startup Also adds TimeoutStopSec=10 and KillMode=mixed to the systemd unit template so the old process gets SIGKILL'd if it does not exit on SIGTERM within 10 seconds — preventing the SIGTERM-ignored zombie scenario entirely.
This commit is contained in:
@@ -162,6 +162,43 @@ def show_password() -> None:
|
|||||||
print("No password file found. Start muxplex to auto-generate one.")
|
print("No password file found. Start muxplex to auto-generate one.")
|
||||||
|
|
||||||
|
|
||||||
|
def _kill_stale_port_holder(port: int) -> None:
|
||||||
|
"""Kill any existing process on *port* to prevent EADDRINUSE crash-loops.
|
||||||
|
|
||||||
|
On service restart (``systemctl restart muxplex``), the old process may still
|
||||||
|
be holding the port in TIME_WAIT state or simply not have exited yet. Without
|
||||||
|
this guard the new process fails to bind, exits with status=1, and systemd
|
||||||
|
restarts it in an infinite loop (observed: 2075+ restarts before manual
|
||||||
|
intervention).
|
||||||
|
|
||||||
|
Uses ``lsof -ti :<port>`` to find occupants, sends SIGTERM, then waits 1 s
|
||||||
|
for the port to free. Silently swallows all errors so that a missing ``lsof``
|
||||||
|
or a permission error never prevents the server from starting.
|
||||||
|
"""
|
||||||
|
import signal
|
||||||
|
import time
|
||||||
|
|
||||||
|
try:
|
||||||
|
result = subprocess.run(
|
||||||
|
["lsof", "-ti", f":{port}"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=5,
|
||||||
|
)
|
||||||
|
if result.returncode == 0 and result.stdout.strip():
|
||||||
|
my_pid = os.getpid()
|
||||||
|
for pid_str in result.stdout.strip().split("\n"):
|
||||||
|
try:
|
||||||
|
pid = int(pid_str.strip())
|
||||||
|
if pid != my_pid:
|
||||||
|
os.kill(pid, signal.SIGTERM)
|
||||||
|
except (ValueError, ProcessLookupError, PermissionError):
|
||||||
|
pass
|
||||||
|
time.sleep(1) # Brief wait for the port to be released
|
||||||
|
except Exception:
|
||||||
|
pass # lsof not available or other error — proceed; uvicorn will fail naturally
|
||||||
|
|
||||||
|
|
||||||
def serve(
|
def serve(
|
||||||
host: str | None = None,
|
host: str | None = None,
|
||||||
port: int | None = None,
|
port: int | None = None,
|
||||||
@@ -188,6 +225,9 @@ def serve(
|
|||||||
os.environ["MUXPLEX_AUTH"] = auth
|
os.environ["MUXPLEX_AUTH"] = auth
|
||||||
os.environ["MUXPLEX_SESSION_TTL"] = str(session_ttl)
|
os.environ["MUXPLEX_SESSION_TTL"] = str(session_ttl)
|
||||||
|
|
||||||
|
# Prevent crash-loop on restart: kill any stale process holding the port
|
||||||
|
_kill_stale_port_holder(port)
|
||||||
|
|
||||||
from muxplex.main import app # noqa: PLC0415
|
from muxplex.main import app # noqa: PLC0415
|
||||||
|
|
||||||
print(f" muxplex → http://{host}:{port}")
|
print(f" muxplex → http://{host}:{port}")
|
||||||
|
|||||||
@@ -27,6 +27,8 @@ Type=simple
|
|||||||
ExecStart={exec_start}
|
ExecStart={exec_start}
|
||||||
Restart=on-failure
|
Restart=on-failure
|
||||||
RestartSec=5s
|
RestartSec=5s
|
||||||
|
TimeoutStopSec=10
|
||||||
|
KillMode=mixed
|
||||||
Environment=PATH={safe_path}
|
Environment=PATH={safe_path}
|
||||||
|
|
||||||
[Install]
|
[Install]
|
||||||
|
|||||||
@@ -1070,6 +1070,133 @@ def test_main_dispatches_to_generate_federation_key(monkeypatch):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# task: port-in-use crash-loop prevention — _kill_stale_port_holder
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_kill_stale_port_holder_exists():
|
||||||
|
"""_kill_stale_port_holder must be importable from muxplex.cli."""
|
||||||
|
from muxplex.cli import _kill_stale_port_holder # noqa: F401
|
||||||
|
|
||||||
|
|
||||||
|
def test_kill_stale_port_holder_runs_lsof(monkeypatch):
|
||||||
|
"""_kill_stale_port_holder must invoke lsof -ti :<port> to find occupying PIDs."""
|
||||||
|
import subprocess
|
||||||
|
import muxplex.cli as cli_mod
|
||||||
|
|
||||||
|
lsof_calls = []
|
||||||
|
|
||||||
|
def fake_run(cmd, **kw):
|
||||||
|
lsof_calls.append(cmd)
|
||||||
|
return type("R", (), {"returncode": 1, "stdout": "", "stderr": ""})()
|
||||||
|
|
||||||
|
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||||
|
|
||||||
|
cli_mod._kill_stale_port_holder(8088)
|
||||||
|
|
||||||
|
assert any("lsof" in str(c) for c in lsof_calls), (
|
||||||
|
"_kill_stale_port_holder must call lsof to discover port occupants"
|
||||||
|
)
|
||||||
|
assert any("8088" in str(c) for c in lsof_calls), (
|
||||||
|
"_kill_stale_port_holder must include the port number in the lsof call"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_kill_stale_port_holder_kills_foreign_pid(monkeypatch):
|
||||||
|
"""_kill_stale_port_holder must send SIGTERM to PIDs that are not our own."""
|
||||||
|
import os
|
||||||
|
import signal
|
||||||
|
import subprocess
|
||||||
|
import muxplex.cli as cli_mod
|
||||||
|
|
||||||
|
foreign_pid = 99999
|
||||||
|
killed = []
|
||||||
|
|
||||||
|
def fake_run(cmd, **kw):
|
||||||
|
return type(
|
||||||
|
"R", (), {"returncode": 0, "stdout": f"{foreign_pid}\n", "stderr": ""}
|
||||||
|
)()
|
||||||
|
|
||||||
|
def fake_kill(pid, sig):
|
||||||
|
killed.append((pid, sig))
|
||||||
|
|
||||||
|
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||||
|
monkeypatch.setattr(os, "kill", fake_kill)
|
||||||
|
monkeypatch.setattr(os, "getpid", lambda: 12345) # not the same as foreign_pid
|
||||||
|
|
||||||
|
# Patch time.sleep so test doesn't actually sleep
|
||||||
|
import time
|
||||||
|
|
||||||
|
monkeypatch.setattr(time, "sleep", lambda _: None)
|
||||||
|
|
||||||
|
cli_mod._kill_stale_port_holder(8088)
|
||||||
|
|
||||||
|
assert (foreign_pid, signal.SIGTERM) in killed, (
|
||||||
|
f"Expected SIGTERM sent to foreign PID {foreign_pid}, got: {killed}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_kill_stale_port_holder_skips_own_pid(monkeypatch):
|
||||||
|
"""_kill_stale_port_holder must NOT kill its own PID."""
|
||||||
|
import os
|
||||||
|
import subprocess
|
||||||
|
import muxplex.cli as cli_mod
|
||||||
|
|
||||||
|
my_pid = 12345
|
||||||
|
killed = []
|
||||||
|
|
||||||
|
def fake_run(cmd, **kw):
|
||||||
|
return type("R", (), {"returncode": 0, "stdout": f"{my_pid}\n", "stderr": ""})()
|
||||||
|
|
||||||
|
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||||
|
monkeypatch.setattr(os, "kill", lambda pid, sig: killed.append(pid))
|
||||||
|
monkeypatch.setattr(os, "getpid", lambda: my_pid)
|
||||||
|
|
||||||
|
import time
|
||||||
|
|
||||||
|
monkeypatch.setattr(time, "sleep", lambda _: None)
|
||||||
|
|
||||||
|
cli_mod._kill_stale_port_holder(8088)
|
||||||
|
|
||||||
|
assert my_pid not in killed, "_kill_stale_port_holder must not kill its own PID"
|
||||||
|
|
||||||
|
|
||||||
|
def test_kill_stale_port_holder_survives_lsof_not_available(monkeypatch):
|
||||||
|
"""_kill_stale_port_holder must not raise when lsof is unavailable."""
|
||||||
|
import subprocess
|
||||||
|
import muxplex.cli as cli_mod
|
||||||
|
|
||||||
|
def fake_run(cmd, **kw):
|
||||||
|
raise FileNotFoundError("lsof not found")
|
||||||
|
|
||||||
|
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||||
|
|
||||||
|
# Should not raise
|
||||||
|
cli_mod._kill_stale_port_holder(8088)
|
||||||
|
|
||||||
|
|
||||||
|
def test_serve_calls_kill_stale_port_holder(tmp_path, monkeypatch):
|
||||||
|
"""serve() must call _kill_stale_port_holder(port) before starting uvicorn."""
|
||||||
|
import muxplex.cli as cli_mod
|
||||||
|
|
||||||
|
settings_file = tmp_path / "settings.json"
|
||||||
|
monkeypatch.setattr("muxplex.settings.SETTINGS_PATH", settings_file)
|
||||||
|
|
||||||
|
killed_ports = []
|
||||||
|
monkeypatch.setattr(
|
||||||
|
cli_mod, "_kill_stale_port_holder", lambda port: killed_ports.append(port)
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("uvicorn.run"):
|
||||||
|
with patch.dict("sys.modules", {"muxplex.main": MagicMock()}):
|
||||||
|
cli_mod.serve(port=9876)
|
||||||
|
|
||||||
|
assert 9876 in killed_ports, (
|
||||||
|
"serve() must call _kill_stale_port_holder with the resolved port before uvicorn.run"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_upgrade_uses_service_module_install(monkeypatch, capsys):
|
def test_upgrade_uses_service_module_install(monkeypatch, capsys):
|
||||||
"""upgrade() must call muxplex.service.service_install."""
|
"""upgrade() must call muxplex.service.service_install."""
|
||||||
import subprocess
|
import subprocess
|
||||||
|
|||||||
@@ -541,6 +541,41 @@ def test_systemd_logs_handles_keyboard_interrupt(monkeypatch):
|
|||||||
svc._systemd_logs()
|
svc._systemd_logs()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# task: port-in-use crash-loop prevention — TimeoutStopSec in systemd unit
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_systemd_unit_template_has_timeout_stop_sec():
|
||||||
|
"""_SYSTEMD_UNIT_TEMPLATE must include TimeoutStopSec to SIGKILL stale process."""
|
||||||
|
import muxplex.service as svc
|
||||||
|
|
||||||
|
assert "TimeoutStopSec" in svc._SYSTEMD_UNIT_TEMPLATE, (
|
||||||
|
"_SYSTEMD_UNIT_TEMPLATE must include TimeoutStopSec so systemd sends SIGKILL "
|
||||||
|
"if the old process does not exit on SIGTERM within the configured time"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_systemd_install_writes_timeout_stop_sec(monkeypatch, tmp_path):
|
||||||
|
"""The written unit file must contain TimeoutStopSec."""
|
||||||
|
import muxplex.service as svc
|
||||||
|
|
||||||
|
unit_dir = tmp_path / "systemd" / "user"
|
||||||
|
unit_path = unit_dir / "muxplex.service"
|
||||||
|
|
||||||
|
monkeypatch.setattr(svc, "_SYSTEMD_UNIT_DIR", unit_dir)
|
||||||
|
monkeypatch.setattr(svc, "_SYSTEMD_UNIT_PATH", unit_path)
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
monkeypatch.setattr(subprocess, "run", lambda cmd, **kw: calls.append(list(cmd)))
|
||||||
|
monkeypatch.setattr(svc, "_prompt_host_if_localhost", lambda: None)
|
||||||
|
|
||||||
|
svc._systemd_install()
|
||||||
|
|
||||||
|
content = unit_path.read_text()
|
||||||
|
assert "TimeoutStopSec" in content, "Written unit file must contain TimeoutStopSec"
|
||||||
|
|
||||||
|
|
||||||
def test_launchd_logs_handles_keyboard_interrupt(monkeypatch):
|
def test_launchd_logs_handles_keyboard_interrupt(monkeypatch):
|
||||||
"""service logs must exit cleanly on Ctrl+C on macOS."""
|
"""service logs must exit cleanly on Ctrl+C on macOS."""
|
||||||
import muxplex.service as svc
|
import muxplex.service as svc
|
||||||
|
|||||||
Reference in New Issue
Block a user