From 1fb9fe330416abe4cf795dc6aed4f0d9aa90e619 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 27 Mar 2026 04:33:04 -0700 Subject: [PATCH] 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//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> --- studio/backend/core/inference/llama_cpp.py | 145 +++++++++++++++------ 1 file changed, 107 insertions(+), 38 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 2b4ed3b2e6..3f9bc0421e 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -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//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//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//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."""