Fix orphan server cleanup killing user's own llama-server (#4622)
* fix: only kill studio-managed llama-server processes, not user's own servers _kill_orphaned_servers() checked for "unsloth" anywhere in the process cmdline, which matched the user's own llama-server when serving models from unsloth/ HF repos (the model path in -m contains "unsloth"). This caused the user's server to get SIGKILLed on Studio startup, destroying their prompt cache and forcing full model re-loads. Narrow the check to only match processes whose binary path lives under ~/.unsloth/llama.cpp/ (the Studio install directory). * Address review: cover env var paths, move Path.home() inside try block - Also check LLAMA_SERVER_PATH and UNSLOTH_LLAMA_CPP_PATH so orphans from custom install locations are still cleaned up. - Move studio_dirs construction inside the try/except so a Path.home() failure (containers without HOME) does not crash the constructor. * Address reviewer feedback: proper path ancestry, /proc/pid/exe, legacy paths Changes based on 10-reviewer consensus: - Use Path.is_relative_to() instead of substring matching to prevent false positives on sibling paths like ~/.unsloth/llama.cpp-backup/. - Use /proc/<pid>/exe (symlink to real binary) instead of parsing the first cmdline token, which breaks on paths with spaces. Falls back to cmdline parsing on non-Linux or when /proc is unavailable. - Add legacy in-tree install paths (project_root/llama.cpp/ and project_root/bin/) so orphans from older setup.sh are still cleaned. - Treat LLAMA_SERVER_PATH as an exact binary match rather than widening it to its parent directory, which could match unrelated servers in shared locations like /usr/local/bin/. - Keep everything inside the try/except so Path.home() failures in containers do not crash the constructor. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: add Linux platform guard and log cleanup errors - Guard pgrep fallback with sys.platform check so it does not crash on Windows/macOS when psutil is unavailable. - Replace silent except-pass with logger.warning for observability. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
b1c3a1e857
commit
1fb9fe3304
1 changed files with 106 additions and 37 deletions
|
|
@ -1367,27 +1367,33 @@ class LlamaCppBackend:
|
|||
"""Kill orphaned llama-server processes started by studio.
|
||||
|
||||
Only kills processes whose resolved binary lives under a known
|
||||
Unsloth install directory to avoid terminating unrelated
|
||||
llama-server instances on the machine.
|
||||
Studio install directory (or matches an exact env-var override)
|
||||
to avoid terminating unrelated llama-server instances.
|
||||
|
||||
Mirrors every location that _find_llama_server_binary() can
|
||||
return from so that orphans from any supported install path
|
||||
are still cleaned up.
|
||||
|
||||
Uses psutil for cross-platform support (Linux, macOS, Windows).
|
||||
Falls back to pgrep + /proc/<pid>/exe on Linux when psutil is
|
||||
not installed.
|
||||
"""
|
||||
import os
|
||||
import signal
|
||||
import sys
|
||||
|
||||
try:
|
||||
import psutil
|
||||
except ImportError:
|
||||
return
|
||||
|
||||
try:
|
||||
# Build the same set of directories that _find_llama_server_binary
|
||||
# searches, so we only kill servers we could have started.
|
||||
# -- Build the ownership allowlist --------------------------------
|
||||
# Two kinds of matches:
|
||||
# exact_binaries -- env var overrides (exact path match only)
|
||||
# install_roots -- directory trees that are Studio-owned
|
||||
# (binary must be *under* one of these)
|
||||
install_roots: list[Path] = []
|
||||
|
||||
# ~/.unsloth/llama.cpp (primary install location)
|
||||
# Primary install dir (setup.sh / prebuilt installer)
|
||||
install_roots.append(Path.home() / ".unsloth" / "llama.cpp")
|
||||
|
||||
# Legacy: in-tree build
|
||||
# Legacy in-tree build dirs (older setup.sh versions)
|
||||
project_root = Path(__file__).resolve().parents[4]
|
||||
install_roots.append(project_root / "llama.cpp")
|
||||
|
||||
|
|
@ -1418,40 +1424,103 @@ class LlamaCppBackend:
|
|||
|
||||
my_pid = os.getpid()
|
||||
|
||||
for proc in psutil.process_iter(["pid", "name", "exe"]):
|
||||
try:
|
||||
if proc.info["pid"] == my_pid:
|
||||
# -- Enumerate processes -------------------------------------------
|
||||
# Prefer psutil (cross-platform). Fall back to pgrep + /proc on
|
||||
# Linux when psutil is not installed.
|
||||
try:
|
||||
import psutil
|
||||
|
||||
has_psutil = True
|
||||
except ImportError:
|
||||
has_psutil = False
|
||||
|
||||
if has_psutil:
|
||||
for proc in psutil.process_iter(["pid", "name", "exe"]):
|
||||
try:
|
||||
if proc.info["pid"] == my_pid:
|
||||
continue
|
||||
|
||||
name = proc.info.get("name") or ""
|
||||
if not name.lower().startswith("llama-server"):
|
||||
continue
|
||||
|
||||
exe = proc.info.get("exe")
|
||||
if not exe:
|
||||
continue
|
||||
|
||||
exe_path = Path(exe).resolve()
|
||||
|
||||
# Check ownership: exact binary match OR binary is
|
||||
# under a known install root (proper ancestry, not
|
||||
# substring).
|
||||
is_ours = exe_path in exact_binaries or any(
|
||||
exe_path.is_relative_to(root) for root in resolved_roots
|
||||
)
|
||||
if not is_ours:
|
||||
continue
|
||||
|
||||
proc.kill()
|
||||
logger.info(
|
||||
f"Killed orphaned llama-server process "
|
||||
f"(pid={proc.info['pid']})"
|
||||
)
|
||||
except (
|
||||
psutil.NoSuchProcess,
|
||||
psutil.AccessDenied,
|
||||
psutil.ZombieProcess,
|
||||
):
|
||||
pass
|
||||
else:
|
||||
# -- Fallback: pgrep + /proc/<pid>/exe (Linux only) -----------
|
||||
if sys.platform != "linux":
|
||||
return
|
||||
result = subprocess.run(
|
||||
["pgrep", "-a", "-f", "llama-server"],
|
||||
capture_output = True,
|
||||
text = True,
|
||||
timeout = 5,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
return
|
||||
|
||||
for line in result.stdout.strip().splitlines():
|
||||
parts = line.strip().split(None, 1)
|
||||
if len(parts) < 2:
|
||||
continue
|
||||
pid = int(parts[0])
|
||||
if pid == my_pid:
|
||||
continue
|
||||
|
||||
name = proc.info.get("name") or ""
|
||||
if not name.lower().startswith("llama-server"):
|
||||
continue
|
||||
# Resolve the actual executable. /proc/<pid>/exe is a
|
||||
# symlink to the real binary and avoids all cmdline-
|
||||
# parsing ambiguities (spaces in paths, argv rewriting).
|
||||
# Fall back to the first cmdline token when /proc is
|
||||
# unavailable.
|
||||
proc_exe = Path(f"/proc/{pid}/exe")
|
||||
try:
|
||||
binary = proc_exe.resolve(strict = True)
|
||||
except (OSError, ValueError):
|
||||
cmdline = parts[1]
|
||||
token = cmdline.split()[0] if cmdline.strip() else ""
|
||||
if not token:
|
||||
continue
|
||||
binary = Path(token).resolve(strict = False)
|
||||
|
||||
exe = proc.info.get("exe")
|
||||
if not exe:
|
||||
continue
|
||||
|
||||
exe_path = Path(exe).resolve()
|
||||
|
||||
# Check if this binary is one we manage
|
||||
is_ours = exe_path in exact_binaries or any(
|
||||
exe_path.is_relative_to(root) for root in resolved_roots
|
||||
owned = binary in exact_binaries or any(
|
||||
binary.is_relative_to(root) for root in resolved_roots
|
||||
)
|
||||
if not is_ours:
|
||||
if not owned:
|
||||
continue
|
||||
|
||||
proc.kill()
|
||||
logger.info(
|
||||
f"Killed orphaned llama-server process (pid={proc.info['pid']})"
|
||||
)
|
||||
except (
|
||||
psutil.NoSuchProcess,
|
||||
psutil.AccessDenied,
|
||||
psutil.ZombieProcess,
|
||||
):
|
||||
pass
|
||||
try:
|
||||
os.kill(pid, signal.SIGKILL)
|
||||
logger.info(f"Killed orphaned llama-server process (pid={pid})")
|
||||
except ProcessLookupError:
|
||||
pass
|
||||
except PermissionError:
|
||||
pass
|
||||
except Exception:
|
||||
pass
|
||||
logger.warning("Error during orphan server cleanup", exc_info = True)
|
||||
|
||||
def _cleanup(self):
|
||||
"""atexit handler to ensure llama-server is terminated."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue