From 6cfcd564c0684c52618235e6366f4a81c02b9a5b Mon Sep 17 00:00:00 2001 From: historia Date: Tue, 1 Sep 2026 14:32:05 -0400 Subject: slop refactor/dedup --- app/backends/servers.py | 133 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 109 insertions(+), 24 deletions(-) (limited to 'app/backends/servers.py') diff --git a/app/backends/servers.py b/app/backends/servers.py index c0f8f4d..25ab472 100644 --- a/app/backends/servers.py +++ b/app/backends/servers.py @@ -146,6 +146,43 @@ def _pid_alive(pid: int) -> bool: return True +def _process_start_token(pid: int) -> Optional[str]: + """A token that changes when the OS recycles PID (best effort). + + On Linux the token is /proc's process start time (field 22): after a + crash and pid reuse, a bare signal-0 probe would happily bless the new + unrelated process as "our server", so the recorded token no longer + matching is what breaks that identification. Platforms without an + equivalent (macOS, Windows) return None — bare-pid probing stays, as + before. + """ + if not sys.platform.startswith("linux"): + return None + try: + stat = Path(f"/proc/{pid}/stat").read_text(encoding="utf-8") + # Fields 1/2 ("comm") may contain spaces and parentheses; starttime + # is field 22, i.e. the 20th entry after the closing paren. + fields = stat.rsplit(")", 1)[1].split() + return fields[19] + except (OSError, IndexError): + return None + + +def _pid_owned(pid: int, token: Optional[str]) -> bool: + """True when PID is alive AND still the process the token recorded. + + An empty/missing token (legacy pid files, non-Linux platforms) falls + back to the bare liveness probe; a recorded token must match the + process's current start time, so a recycled pid is reported dead. + """ + if not _pid_alive(pid): + return False + if not token: + return True + current = _process_start_token(pid) + return current is None or current == token + + def _reap_exited(pid: int) -> bool: """Reap PID when it is our exited child; True when confirmed dead. @@ -169,20 +206,23 @@ def _reap_exited(pid: int) -> bool: def _kill_pid(pid: int) -> bool: - """Terminate PID (and its process group on POSIX). Returns True when dead.""" + """Terminate PID (and its process tree). Returns True when dead.""" if sys.platform == "win32": + # TerminateProcess hits a single pid; the server may have spawned + # children that would survive as orphans. taskkill /T kills the + # whole tree (the same mechanism run_console_subprocess uses). try: - os.kill(pid, signal.SIGTERM) - except (ProcessLookupError, PermissionError, OSError): - return not _pid_alive(pid) + subprocess.run(["taskkill", "/T", "/F", "/PID", str(pid)], + capture_output=True, timeout=STOP_GRACE_SECONDS) + except (OSError, subprocess.SubprocessError): + try: + os.kill(pid, signal.SIGTERM) + except (ProcessLookupError, PermissionError, OSError): + return not _pid_alive(pid) for _ in range(int(STOP_GRACE_SECONDS * 10)): if not _pid_alive(pid): return True time.sleep(0.1) - try: - os.kill(pid, signal.SIGTERM) - except OSError: - pass return not _pid_alive(pid) # POSIX: kill the whole process group (started with start_new_session=True). try: @@ -279,6 +319,19 @@ def start(spec, progress: ProgressCallback = None, pid_file.unlink() except OSError: pass + # Reserve the slot atomically (O_EXCL): two concurrent starters can + # both pass the liveness check above, but only one wins the create — + # the loser refuses instead of spawning a duplicate server on the port. + try: + pid_handle = pid_file.open("x") + except FileExistsError: + report({"kind": "error", + "message": f"a {spec.name} server is already starting " + "(its pid file appeared while this start was " + "running); stop it first"}) + return False + except OSError: + pid_handle = None cwd = getattr(spec, "cwd", None) # Append so an earlier boot's output survives (crash-loop debugging); @@ -299,13 +352,28 @@ def start(spec, progress: ProgressCallback = None, report({"kind": "error", "message": f"could not start server: {exc}"}) log_handle.close() + if pid_handle is not None: + pid_handle.close() + try: + pid_file.unlink() + except OSError: + pass return False log_handle.write(f"\n=== boot {datetime.now():%Y-%m-%d %H:%M:%S} " f"(pid {proc.pid}) ===\n") log_handle.flush() log_handle.close() - pid_file.write_text(str(proc.pid), encoding="utf-8") + # "pid token": the process's start time where the platform provides + # one, so a recycled pid is never mistaken for our server (see + # _pid_owned). Empty token = bare-pid probing. + token = _process_start_token(proc.pid) or "" + if pid_handle is not None: + try: + pid_handle.write(f"{proc.pid} {token}\n".strip() + "\n") + pid_handle.close() + except OSError: + pass report({"kind": "starting", "name": spec.name, "argv": " ".join(str(a) for a in argv), "cwd": str(cwd) if cwd is not None else None, @@ -354,7 +422,10 @@ def stop(name: str) -> bool: Returns True when the process was terminated (or already gone). Returns False when there is no pid file — the server was not started by this tool, - so the user must stop it manually (e.g. close its terminal). + so the user must stop it manually (e.g. close its terminal). The pid + file is removed only once the kill succeeded: while the process is + still running, the record is what keeps a later ``start`` from spawning + a duplicate onto the same port. """ pid_file = _pid_path(name) if not pid_file.exists(): @@ -362,15 +433,15 @@ def stop(name: str) -> bool: "(not started by this tool — stop it manually)") return False try: - pid = int(pid_file.read_text(encoding="utf-8").strip()) - except (OSError, ValueError): + pid = int(pid_file.read_text(encoding="utf-8").split()[0]) + except (OSError, ValueError, IndexError): print(f"[WARNING] could not read pid file {pid_file}; removing it") try: pid_file.unlink() except OSError: pass return False - if not _pid_alive(pid): + if not _pid_owned_recorded(pid, pid_file): print(f"[INFO] {name} server (pid {pid}) already stopped") try: pid_file.unlink() @@ -381,13 +452,24 @@ def stop(name: str) -> bool: killed = _kill_pid(pid) if killed: print(f"[OK] {name} server stopped") + try: + pid_file.unlink() + except OSError: + pass else: - print(f"[WARNING] could not stop pid {pid}; stop it manually") + print(f"[WARNING] could not stop pid {pid}; stop it manually — the " + "pid file is kept so the server is not started twice") + return killed + + +def _pid_owned_recorded(pid: int, pid_file: Path) -> bool: + """PID alive and still the process recorded in PID_FILE (token aware).""" try: - pid_file.unlink() + fields = pid_file.read_text(encoding="utf-8").split() except OSError: - pass - return killed + return _pid_alive(pid) + token = fields[1] if len(fields) > 1 else None + return _pid_owned(pid, token) def manages(specs) -> bool: @@ -395,13 +477,15 @@ def manages(specs) -> bool: A server counts as ours when ``start`` recorded a pid file for it and that pid is still alive — the same ownership rule ``stop`` applies - before refusing ("not started by this tool"). Used by the backends' - ``detect()`` so the hub's status table can tag an up server as - "[remote]" when it was launched outside this tool. + before refusing ("not started by this tool"). Where the platform + provides a start-time token, a recycled pid no longer counts as ours. + Used by the backends' ``detect()`` so the hub's status table can tag + an up server as "[remote]" when it was launched outside this tool. """ for spec in specs: + pid_file = _pid_path(spec.name) pid = pid_for(spec.name) - if pid is not None and _pid_alive(pid): + if pid is not None and _pid_owned_recorded(pid, pid_file): return True return False @@ -412,12 +496,13 @@ def pid_for(name: str): if not pid_file.exists(): return None try: - return int(pid_file.read_text(encoding="utf-8").strip()) - except (OSError, ValueError): + return int(pid_file.read_text(encoding="utf-8").split()[0]) + except (OSError, ValueError, IndexError): return None def alive(name: str) -> bool: """True when the server named NAME was started by us and is still alive.""" + pid_file = _pid_path(name) pid = pid_for(name) - return pid is not None and _pid_alive(pid) + return pid is not None and _pid_owned_recorded(pid, pid_file) -- cgit v1.2.3