mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-09-10 01:28:01 +00:00
fix(cookbook): record real Windows pid for local serve so Stop kills the model (#5912)
* fix(cookbook): record real Windows pid for local serve so Stop kills the model The Windows-local serve runner recorded Git Bash's `$$`, which is the MSYS/Cygwin pid, not the Windows pid. Win32 tooling (taskkill, Get-CimInstance ParentProcessId, Stop-Process) can't match an MSYS pid, so the frontend Stop-Tree walk found nothing and the llama-server child survived after Stop, leaving the model loaded and the GPU pinned. Record the serving shell's true Win32 pid via `/proc/$$/winpid`, falling back to the outer proc.pid already written from Python when the map is unavailable. The existing pid-tracking test asserted the buggy `$$` literal at the source level, so it passed while the feature was broken; update it to the winpid behavior and add a focused regression test. * fix(cookbook): make Windows serve pid handoff deterministic --------- Co-authored-by: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Co-authored-by: Alexandre Teixeira <alexandremagteixeira@gmail.com>
This commit is contained in:
parent
17ee856d1c
commit
53869d194d
3 changed files with 226 additions and 3 deletions
|
|
@ -73,6 +73,30 @@ _HF_TOKEN_STATUS_SNIPPET = (
|
|||
)
|
||||
|
||||
|
||||
def _windows_local_pid_record_line(pid_path: Path, ready_path: Path) -> str:
|
||||
"""Build the Git Bash prelude that records a Win32-stoppable PID.
|
||||
|
||||
Python publishes the detached outer process's Win32 PID first, then touches
|
||||
``ready_path``. The inner Git Bash runner waits for that publication before
|
||||
replacing the fallback with its own Win32 PID from /proc/<msys-pid>/winpid.
|
||||
|
||||
Missing, malformed, or late mappings leave the valid outer PID untouched.
|
||||
"""
|
||||
pp = shlex.quote(pid_path.as_posix())
|
||||
rp = shlex.quote(ready_path.as_posix())
|
||||
return (
|
||||
"i=0; "
|
||||
f"while [ ! -e {rp} ] && [ \"$i\" -lt 500 ]; do "
|
||||
"i=$((i+1)); sleep 0.01; done; "
|
||||
f"if [ -e {rp} ]; then "
|
||||
"winpid=\"$(cat /proc/$$/winpid 2>/dev/null || true)\"; "
|
||||
"case \"$winpid\" in ''|*[!0-9]*) ;; "
|
||||
f"*) printf '%s\\n' \"$winpid\" > {pp} ;; esac; "
|
||||
"fi; "
|
||||
f"rm -f {rp}"
|
||||
)
|
||||
|
||||
|
||||
def _append_mlx_image_server_script(runner_lines: list[str]) -> None:
|
||||
"""Write the MLX image API helper next to the tmux runner on remote hosts."""
|
||||
script_path = Path(__file__).resolve().parents[1] / "scripts" / "mlx_image_server.py"
|
||||
|
|
@ -978,15 +1002,18 @@ def setup_cookbook_routes() -> APIRouter:
|
|||
directly (simple commands only). Returns the launched job record."""
|
||||
log_path = TMUX_LOG_DIR / f"{session_id}.log"
|
||||
pid_path = TMUX_LOG_DIR / f"{session_id}.pid"
|
||||
pid_ready_path: Path | None = None
|
||||
bash = find_bash()
|
||||
if bash:
|
||||
# Run the existing bash wrapper verbatim through Git Bash, redirecting
|
||||
# all output to the log the poller reads. Paths handed to bash use
|
||||
# POSIX form + shell-quoting so drive paths / spaces survive.
|
||||
inner = TMUX_LOG_DIR / f"{session_id}_run.sh"
|
||||
pp = shlex.quote(pid_path.as_posix())
|
||||
pid_ready_path = TMUX_LOG_DIR / f"{session_id}.pid.ready"
|
||||
pid_ready_path.unlink(missing_ok=True)
|
||||
inner.write_text(
|
||||
f"printf '%s\\n' \"$$\" > {pp}\n" + "\n".join(bash_lines) + "\n",
|
||||
_windows_local_pid_record_line(pid_path, pid_ready_path) + "\n"
|
||||
+ "\n".join(bash_lines) + "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
lp = shlex.quote(log_path.as_posix())
|
||||
|
|
@ -1020,7 +1047,18 @@ def setup_cookbook_routes() -> APIRouter:
|
|||
env=env,
|
||||
**detached_popen_kwargs(),
|
||||
)
|
||||
# Publish a valid Win32 ancestor first. The Git Bash runner may then
|
||||
# replace it with its own Win32 pid, but never before this fallback exists.
|
||||
pid_path.write_text(str(proc.pid), encoding="utf-8")
|
||||
if pid_ready_path is not None:
|
||||
try:
|
||||
pid_ready_path.touch()
|
||||
except OSError as e:
|
||||
logger.warning(
|
||||
"Could not publish Windows local PID handoff for %s: %s",
|
||||
session_id,
|
||||
e,
|
||||
)
|
||||
return {"pid": proc.pid, "log_path": str(log_path)}
|
||||
|
||||
@router.post("/api/model/download")
|
||||
|
|
|
|||
|
|
@ -723,7 +723,12 @@ def test_local_windows_download_pid_tracks_inner_bash_and_stop_kills_tree():
|
|||
routes_src = (Path(__file__).resolve().parents[1] / "routes" / "cookbook_routes.py").read_text(encoding="utf-8")
|
||||
running_src = (Path(__file__).resolve().parents[1] / "static" / "js" / "cookbookRunning.js").read_text(encoding="utf-8")
|
||||
|
||||
assert 'printf \'%s\\\\n\' \\"$$\\" > {pp}' in routes_src
|
||||
# The Windows-local runner publishes Python's valid Win32 fallback before
|
||||
# allowing Git Bash to replace it with /proc/$$/winpid.
|
||||
assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in routes_src
|
||||
assert "/proc/$$/winpid" in routes_src
|
||||
assert "pid_ready_path.touch()" in routes_src
|
||||
assert '\\"$$\\" > {pp}' not in routes_src
|
||||
assert "function Stop-Tree([int]$Id)" in running_src
|
||||
assert "('ParentProcessId = ' + $Id)" in running_src
|
||||
assert "Stop-Tree ([int]$p)" in running_src
|
||||
|
|
|
|||
180
tests/test_cookbook_local_serve_pid_winpid.py
Normal file
180
tests/test_cookbook_local_serve_pid_winpid.py
Normal file
|
|
@ -0,0 +1,180 @@
|
|||
"""Behavioral regression coverage for Windows-local Cookbook PID recording."""
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
from routes.cookbook_routes import _windows_local_pid_record_line
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
COOKBOOK_ROUTES = ROOT / "routes" / "cookbook_routes.py"
|
||||
|
||||
|
||||
def _fake_cat(tmp_path: Path, body: str) -> Path:
|
||||
fake_bin = tmp_path / "bin"
|
||||
fake_bin.mkdir()
|
||||
cat = fake_bin / "cat"
|
||||
cat.write_text("#!/bin/sh\n" + body + "\n", encoding="utf-8")
|
||||
cat.chmod(0o755)
|
||||
return fake_bin
|
||||
|
||||
|
||||
def _env_for(fake_bin: Path, **extra: str) -> dict[str, str]:
|
||||
env = dict(os.environ)
|
||||
env["PATH"] = str(fake_bin) + os.pathsep + env.get("PATH", "")
|
||||
env.update(extra)
|
||||
return env
|
||||
|
||||
|
||||
def _run_pid_line(
|
||||
pid_path: Path,
|
||||
ready_path: Path,
|
||||
fake_bin: Path,
|
||||
**extra_env: str,
|
||||
) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
["bash", "-c", _windows_local_pid_record_line(pid_path, ready_path)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=_env_for(fake_bin, **extra_env),
|
||||
timeout=10,
|
||||
)
|
||||
|
||||
|
||||
def test_windows_local_pid_line_records_numeric_winpid_after_fallback(tmp_path):
|
||||
pid_path = tmp_path / "serve.pid"
|
||||
ready_path = tmp_path / "serve.pid.ready"
|
||||
|
||||
pid_path.write_text("11111", encoding="utf-8")
|
||||
ready_path.touch()
|
||||
|
||||
cat_arg = tmp_path / "cat-arg.txt"
|
||||
fake_bin = _fake_cat(
|
||||
tmp_path,
|
||||
'printf "%s\\n" "$1" > "$FAKE_CAT_ARG"\n'
|
||||
'printf "%s\\n" "$FAKE_WINPID"',
|
||||
)
|
||||
|
||||
result = _run_pid_line(
|
||||
pid_path,
|
||||
ready_path,
|
||||
fake_bin,
|
||||
FAKE_CAT_ARG=str(cat_arg),
|
||||
FAKE_WINPID="42324",
|
||||
)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert pid_path.read_text(encoding="utf-8").strip() == "42324"
|
||||
assert not ready_path.exists()
|
||||
|
||||
proc_path = cat_arg.read_text(encoding="utf-8").strip()
|
||||
parts = proc_path.strip("/").split("/")
|
||||
assert len(parts) == 3
|
||||
assert parts[0] == "proc"
|
||||
assert parts[1].isdigit()
|
||||
assert parts[2] == "winpid"
|
||||
|
||||
|
||||
def test_windows_local_pid_line_waits_for_python_fallback_before_replacing(tmp_path):
|
||||
pid_path = tmp_path / "serve.pid"
|
||||
ready_path = tmp_path / "serve.pid.ready"
|
||||
|
||||
fake_bin = _fake_cat(
|
||||
tmp_path,
|
||||
'printf "%s\\n" "$FAKE_WINPID"',
|
||||
)
|
||||
|
||||
proc = subprocess.Popen(
|
||||
[
|
||||
"bash",
|
||||
"-c",
|
||||
_windows_local_pid_record_line(pid_path, ready_path),
|
||||
],
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
env=_env_for(fake_bin, FAKE_WINPID="42324"),
|
||||
)
|
||||
|
||||
# The inner shell has started, but Python has not published its fallback yet.
|
||||
time.sleep(0.05)
|
||||
assert proc.poll() is None
|
||||
assert not pid_path.exists()
|
||||
|
||||
# Simulate the post-Popen Python publication order.
|
||||
pid_path.write_text("31100", encoding="utf-8")
|
||||
ready_path.touch()
|
||||
|
||||
stdout, stderr = proc.communicate(timeout=10)
|
||||
|
||||
assert proc.returncode == 0, stderr or stdout
|
||||
assert pid_path.read_text(encoding="utf-8").strip() == "42324"
|
||||
assert not ready_path.exists()
|
||||
|
||||
|
||||
def test_windows_local_pid_line_preserves_outer_pid_when_mapping_missing(tmp_path):
|
||||
pid_path = tmp_path / "serve.pid"
|
||||
ready_path = tmp_path / "serve.pid.ready"
|
||||
|
||||
pid_path.write_text("31100", encoding="utf-8")
|
||||
ready_path.touch()
|
||||
|
||||
fake_bin = _fake_cat(tmp_path, "exit 1")
|
||||
|
||||
result = _run_pid_line(
|
||||
pid_path,
|
||||
ready_path,
|
||||
fake_bin,
|
||||
)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert pid_path.read_text(encoding="utf-8").strip() == "31100"
|
||||
assert not ready_path.exists()
|
||||
|
||||
|
||||
def test_windows_local_pid_line_rejects_malformed_mapping(tmp_path):
|
||||
pid_path = tmp_path / "serve.pid"
|
||||
ready_path = tmp_path / "serve.pid.ready"
|
||||
|
||||
pid_path.write_text("31100", encoding="utf-8")
|
||||
ready_path.touch()
|
||||
|
||||
fake_bin = _fake_cat(
|
||||
tmp_path,
|
||||
'printf "not-a-win32-pid\\n"',
|
||||
)
|
||||
|
||||
result = _run_pid_line(
|
||||
pid_path,
|
||||
ready_path,
|
||||
fake_bin,
|
||||
)
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert pid_path.read_text(encoding="utf-8").strip() == "31100"
|
||||
assert not ready_path.exists()
|
||||
|
||||
|
||||
def test_local_windows_launcher_publishes_fallback_before_releasing_inner_runner():
|
||||
source = COOKBOOK_ROUTES.read_text(encoding="utf-8")
|
||||
start = source.index(" def _launch_local_detached(")
|
||||
end = source.index(
|
||||
' @router.post("/api/model/download")',
|
||||
start,
|
||||
)
|
||||
launcher = source[start:end]
|
||||
|
||||
assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in launcher
|
||||
assert "pid_ready_path.unlink(missing_ok=True)" in launcher
|
||||
|
||||
fallback = launcher.index(
|
||||
'pid_path.write_text(str(proc.pid), encoding="utf-8")'
|
||||
)
|
||||
release = launcher.index("pid_ready_path.touch()")
|
||||
|
||||
assert fallback < release
|
||||
|
||||
# Never write Git Bash's bare MSYS $$ to the session pid file.
|
||||
assert '\\"$$\\" > {pp}' not in launcher
|
||||
Loading…
Add table
Reference in a new issue