Never signal PID 0 or 1, and verify a per-port record before trusting it
This commit is contained in:
parent
7db3ee073c
commit
bdd0cf3835
4 changed files with 60 additions and 5 deletions
|
|
@ -817,6 +817,10 @@ def _read_pid_record(path: Path) -> "tuple[int, float | None, str | None] | None
|
|||
return None
|
||||
if not lines or not lines[0].strip().isdigit():
|
||||
return None
|
||||
pid = int(lines[0].strip())
|
||||
# kill(0) signals our whole process group; kill(1) is init. Never either.
|
||||
if pid < 2:
|
||||
return None
|
||||
created = None
|
||||
if len(lines) > 1:
|
||||
try:
|
||||
|
|
@ -824,7 +828,7 @@ def _read_pid_record(path: Path) -> "tuple[int, float | None, str | None] | None
|
|||
except ValueError:
|
||||
created = None
|
||||
address = lines[2].strip() if len(lines) > 2 and lines[2].strip() else None
|
||||
return int(lines[0].strip()), created, address
|
||||
return pid, created, address
|
||||
|
||||
|
||||
def _pid_is_studio_backend(pid: int, created_times: "Sequence[float | None]" = ()) -> "bool | None":
|
||||
|
|
@ -891,9 +895,11 @@ def _legacy_studio_on_port(port: int) -> "int | None":
|
|||
if not _pid_alive(pid):
|
||||
return None
|
||||
# A current build writes a per-port file too, so its port is already known --
|
||||
# and this port's records were just checked. Only unported records get here.
|
||||
if any(r and r[0] == pid for r in _per_port_records()):
|
||||
return None
|
||||
# and this port's records were just checked. Only count a record that still
|
||||
# matches the live process: a stale one may just share a reused PID.
|
||||
for other in _per_port_records():
|
||||
if other and other[0] == pid and _pid_is_studio_backend(pid, [other[1]]) is not False:
|
||||
return None
|
||||
blocker = _get_pid_on_port(port)
|
||||
if blocker is not None and blocker[0] != pid:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -117,6 +117,15 @@ def test_read_pid_record_tolerates_a_bare_pid(tmp_path):
|
|||
assert run._read_pid_record(tmp_path / "r.pid") == (8550, None, None)
|
||||
|
||||
|
||||
def test_read_pid_record_rejects_pid_zero_and_init(tmp_path):
|
||||
# kill(0) signals our whole process group.
|
||||
(tmp_path / "zero.pid").write_text("0", encoding = "utf-8")
|
||||
(tmp_path / "init.pid").write_text("1", encoding = "utf-8")
|
||||
|
||||
assert run._read_pid_record(tmp_path / "zero.pid") is None
|
||||
assert run._read_pid_record(tmp_path / "init.pid") is None
|
||||
|
||||
|
||||
def test_read_pid_record_rejects_a_corrupt_file(tmp_path):
|
||||
(tmp_path / "r.pid").write_text("not-a-pid", encoding = "utf-8")
|
||||
|
||||
|
|
@ -261,6 +270,19 @@ def test_a_dead_legacy_record_falls_back(tmp_path, monkeypatch):
|
|||
assert run._own_studio_on_port(8901, "127.0.0.1") is None
|
||||
|
||||
|
||||
def test_a_stale_per_port_record_does_not_mask_a_legacy_server(tmp_path, monkeypatch):
|
||||
# Crashed current build left studio-8901-8550.pid; 8550 was then reused by a
|
||||
# pre-upgrade server recorded only in studio.pid. The stale record must not
|
||||
# count as "port already known" and send us falling back past the live one.
|
||||
monkeypatch.setattr(run, "_pid_is_studio_backend", _REAL_IS_STUDIO_BACKEND)
|
||||
monkeypatch.setattr(run, "_process_create_time", lambda pid: 999.0)
|
||||
monkeypatch.setattr(run, "_get_pid_on_port", lambda p: (8550, "python"))
|
||||
(tmp_path / "studio-8901-8550.pid").write_text("8550\n111.5\n127.0.0.1", encoding = "utf-8")
|
||||
(tmp_path / "studio.pid").write_text("8550", encoding = "utf-8")
|
||||
|
||||
assert run._own_studio_on_port(8901, "127.0.0.1") == 8550
|
||||
|
||||
|
||||
def test_a_current_server_elsewhere_does_not_block_a_foreign_port(tmp_path, monkeypatch):
|
||||
# Current builds write studio.pid too. Without psutil the legacy check can't
|
||||
# see the listener, so it must not claim our 8901 server holds jupyter's 8888.
|
||||
|
|
|
|||
|
|
@ -2432,13 +2432,17 @@ def _read_pid_record(path: Path) -> "tuple[int, float | None] | None":
|
|||
return None
|
||||
if not lines or not lines[0].strip().isdigit():
|
||||
return None
|
||||
pid = int(lines[0].strip())
|
||||
# kill(0) signals our whole process group; kill(1) is init. Never either.
|
||||
if pid < 2:
|
||||
return None
|
||||
created = None
|
||||
if len(lines) > 1:
|
||||
try:
|
||||
created = float(lines[1].strip())
|
||||
except ValueError:
|
||||
created = None
|
||||
return int(lines[0].strip()), created
|
||||
return pid, created
|
||||
|
||||
|
||||
def _pid_file_entries() -> "list[tuple[int, list[float | None], list[Path]]]":
|
||||
|
|
@ -2507,6 +2511,8 @@ def _signal_stop(pid: int) -> "str | None":
|
|||
"""SIGTERM (or taskkill) the pid. Returns an error string, or None on success."""
|
||||
import signal as _signal
|
||||
|
||||
if pid < 2:
|
||||
return f"refusing to signal PID {pid}"
|
||||
try:
|
||||
if sys.platform == "win32":
|
||||
# /T also stops llama-server children, which otherwise keep GPU and port.
|
||||
|
|
|
|||
|
|
@ -381,6 +381,27 @@ def test_stop_continues_after_one_server_fails_to_stop(monkeypatch, tmp_path):
|
|||
assert "8550" in combined
|
||||
|
||||
|
||||
def test_stop_never_signals_pid_zero_or_init(monkeypatch, tmp_path):
|
||||
# os.kill(0, SIGTERM) hits our whole process group -- the shell and its jobs.
|
||||
studio_mod, _live, killed = _install(monkeypatch, tmp_path, alive = {0, 1})
|
||||
_write_pid(tmp_path, "studio-8901-0.pid", 0)
|
||||
_write_pid(tmp_path, "studio-8902-1.pid", 1)
|
||||
|
||||
result = _run_stop(studio_mod)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert killed == []
|
||||
assert not list(tmp_path.glob("*.pid"))
|
||||
|
||||
|
||||
def test_signal_stop_refuses_pid_zero_or_init(monkeypatch, tmp_path):
|
||||
studio_mod, _live, killed = _install(monkeypatch, tmp_path, alive = {0, 1})
|
||||
|
||||
assert studio_mod._signal_stop(0) is not None
|
||||
assert studio_mod._signal_stop(1) is not None
|
||||
assert killed == []
|
||||
|
||||
|
||||
def test_stop_discards_a_corrupt_pid_file(monkeypatch, tmp_path):
|
||||
studio_mod, _live, _killed = _install(monkeypatch, tmp_path, alive = set())
|
||||
(tmp_path / "studio-8901-8550.pid").write_text("not-a-pid", encoding = "utf-8")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue