diff options
Diffstat (limited to 'app/backends')
| -rw-r--r-- | app/backends/audiocpp/catalog.py | 16 | ||||
| -rw-r--r-- | app/backends/audiocpp/configsync.py | 17 | ||||
| -rw-r--r-- | app/backends/audiocpp/models.py | 90 | ||||
| -rw-r--r-- | app/backends/audiocpp/remote.py | 1 | ||||
| -rw-r--r-- | app/backends/audiocpp/status.py | 34 | ||||
| -rw-r--r-- | app/backends/audiocpp/wizard.py | 2 | ||||
| -rw-r--r-- | app/backends/common.py | 67 | ||||
| -rwxr-xr-x | app/backends/faster.py | 31 | ||||
| -rw-r--r-- | app/backends/probe.py | 21 | ||||
| -rw-r--r-- | app/backends/qwen.py | 24 | ||||
| -rw-r--r-- | app/backends/servers.py | 133 |
11 files changed, 266 insertions, 170 deletions
diff --git a/app/backends/audiocpp/catalog.py b/app/backends/audiocpp/catalog.py index 1048185..048e325 100644 --- a/app/backends/audiocpp/catalog.py +++ b/app/backends/audiocpp/catalog.py @@ -44,6 +44,8 @@ def request_options_families(audiocpp_dir: Path) -> Dict[str, dict]: spec = json.loads(spec_path.read_text(encoding="utf-8")) except (OSError, ValueError): continue + if not isinstance(spec, dict): + continue options = spec.get("options") request = options.get("request") if isinstance(options, dict) else None if not isinstance(request, list) or not request: @@ -161,12 +163,12 @@ def _default_package(packages: List[dict]) -> Optional[dict]: if not packages: return None for package in packages: - if package.get("default"): + if isinstance(package, dict) and package.get("default"): return package for package in packages: - if package.get("format") == "gguf": + if isinstance(package, dict) and package.get("format") == "gguf": return package - return packages[0] + return packages[0] if isinstance(packages[0], dict) else None def spec_gguf_rooted(spec: dict) -> bool: @@ -292,6 +294,11 @@ def load_model_catalog(audiocpp_dir: Path) -> List[dict]: spec = json.loads(spec_path.read_text(encoding="utf-8")) except (OSError, ValueError): continue + if not isinstance(spec, dict): + # Valid JSON that is not an object (a list, a string, a + # number): skip it like an unparsable one instead of crashing + # the wizard on a half-written spec file. + continue sanitize_model_spec(spec) tasks = spec.get("tasks") or [] if tasks: @@ -305,7 +312,8 @@ def load_model_catalog(audiocpp_dir: Path) -> List[dict]: # No task list: fall back to the category as before. continue family = spec.get("family") or spec_path.stem - packages = spec.get("packages") or [] + packages = [package for package in (spec.get("packages") or []) + if isinstance(package, dict)] package = _default_package(packages) if package is None: # No installable package: skip (cannot be hosted from a path). diff --git a/app/backends/audiocpp/configsync.py b/app/backends/audiocpp/configsync.py index bce8142..0a0e5ad 100644 --- a/app/backends/audiocpp/configsync.py +++ b/app/backends/audiocpp/configsync.py @@ -14,10 +14,7 @@ from .constants import FALLBACK_PORT def config_port() -> int: """Return the port of AUDIOCPP_API_URL in app/converter/config.py.""" - try: - return urllib.parse.urlsplit(config.AUDIOCPP_API_URL).port or FALLBACK_PORT - except ValueError: - return FALLBACK_PORT + return common.port_of(config.AUDIOCPP_API_URL, FALLBACK_PORT) def update_config_api_url_port(port: int, config_path: Optional[Path] = None) -> bool: @@ -75,18 +72,6 @@ def update_server_config_port(port: int) -> bool: return True -def _apply_port_sync(port: int, accepted: bool) -> None: - """Write the port into app/converter/config.py, or report when declined.""" - if accepted: - if not update_config_api_url_port(port): - print(f"[WARNING] Could not update {CONFIG_PATH}; edit " - "AUDIOCPP_API_URL by hand so audiobook.py uses the " - "new port") - else: - print("[WARNING] Left AUDIOCPP_API_URL unchanged; audiobook.py " - f"will still use port {config_port()}") - - def update_server_backend(backend: str) -> bool: """Rewrite the 'backend' in the checkout's server.json, or True when none. diff --git a/app/backends/audiocpp/models.py b/app/backends/audiocpp/models.py index 16c61e9..d150f62 100644 --- a/app/backends/audiocpp/models.py +++ b/app/backends/audiocpp/models.py @@ -16,10 +16,6 @@ from . import catalog as _catalog # runner kills it and reports exit 124 (see run_console_subprocess). DOWNLOAD_STALL_TIMEOUT = 300 -# Spec sanitizing lives with the catalog (the wizard's catalog view applies -# the same in-memory repair; the download path materializes it through its -# sanitized specs copy). The alias keeps this module's historical name. -_sanitize_model_spec = _catalog.sanitize_model_spec def _installed_display_names(audiocpp_dir: Path, model_entries: Optional[List[dict]], @@ -255,52 +251,21 @@ def _manager_supports_progress(manager: Path) -> bool: and _manager_supports_flag(manager, "--cancel-file")) -def _sanitize_model_spec(spec: dict) -> bool: - """Repair dot ``strip_prefix`` packages in SPEC, in place. - - A package's ``strip_prefix`` is stripped from the front of every file - path to get the local layout, so it only works when every file is - listed under that prefix (``<prefix>/<file>``). A dot prefix ("." or - "./") is meant for files written ``./<file>``; when the package instead - lists repo-root files bare (``model.gguf``), the manager rejects the - whole package ("file path does not start with strip_prefix '.': ...") - and nothing can be downloaded. Root-level files need no prefix at all - (upstream specs like minimax_music3.json store ""), so dropping the dot - prefix is the safe repair. Prefixes naming a real directory are left - alone — the correct remote paths cannot be guessed. Returns True when - SPEC changed. - """ - changed = False - for package in spec.get("packages") or []: - if not isinstance(package, dict): - continue - prefix = str(package.get("strip_prefix") or "").rstrip("/") - if prefix not in (".", ".."): - continue - files = package.get("files") - if not isinstance(files, list) or not files: - continue - if all(isinstance(item, str) and item.startswith(prefix + "/") - for item in files): - continue - package["strip_prefix"] = "" - changed = True - return changed - - def _prepare_specs_dir(audiocpp_dir: Path) -> Optional[Path]: - """Return a temp specs dir with dot ``strip_prefix`` entries repaired. + """Return a temp specs dir with broken ``strip_prefix`` entries repaired. audio.cpp's model manager accepts ``--specs-dir``, so a checkout whose specs carry a broken ``strip_prefix`` can be installed from a sanitized copy without modifying the checkout. Every ``model_specs/*.json`` is - copied; the ones needing a repair are rewritten via - ``_sanitize_model_spec`` (specs that fail to parse are copied verbatim - so the manager reports them exactly as it would upstream). Returns None - when no spec needed a repair (or the specs directory is missing or - unreadable) — the caller then runs against the checkout's own specs. - The caller owns the returned directory and removes it when the installs - are done. + copied; the ones needing a repair are rewritten via the catalog's + ``sanitize_model_spec`` (both upstream spec bug classes: dot prefixes + and missing prefixes on $gguf-rooted single-GGUF packages — glm_tts/ + outetts shipped like that). Specs that fail to parse are copied + verbatim so the manager reports them exactly as it would upstream. + Returns None when no spec needed a repair (or the specs directory is + missing or unreadable) — the caller then runs against the checkout's + own specs. The caller owns the returned directory and removes it when + the installs are done. """ specs_dir = audiocpp_dir / "model_specs" try: @@ -321,7 +286,7 @@ def _prepare_specs_dir(audiocpp_dir: Path) -> Optional[Path]: except ValueError: payloads.append((spec_path.name, text)) continue - if isinstance(spec, dict) and _sanitize_model_spec(spec): + if isinstance(spec, dict) and _catalog.sanitize_model_spec(spec): sanitized = True text = json.dumps(spec, indent=2, ensure_ascii=False) + "\n" payloads.append((spec_path.name, text)) @@ -707,12 +672,15 @@ def delete_model_files(server_json: Path, entries: List[dict]) -> int: Each entry's ``rel`` is resolved exactly like the server resolves it (relative against ``server_json``'s directory; absolute paths honored), - then removed as a directory tree or a single file. Missing entries are - ignored. Returns the number of paths removed. Used by the wizard's + then removed as a directory tree or a single file. Paths that escape + the checkout (``..`` segments, or an absolute path outside the + checkout's tree) are refused rather than deleted — the value comes + from a user-editable server.json. Missing entries are ignored. + Returns the number of paths removed. Used by the wizard's "Delete unused models?" step — the regenerated server.json already only lists the kept models, so no entry cleanup is needed here. """ - base = server_json.parent + base = server_json.parent.resolve() removed = 0 for item in entries: rel = item.get("rel") @@ -720,16 +688,28 @@ def delete_model_files(server_json: Path, entries: List[dict]) -> int: continue path = Path(rel) if Path(rel).is_absolute() else base / rel try: - if not path.exists(): + resolved = path.resolve() + except OSError: + continue + if base not in resolved.parents and resolved != base: + print(f"[WARNING] Refusing to remove {path}: outside the " + "audio.cpp checkout") + continue + if resolved == base: + print(f"[WARNING] Refusing to remove {path}: it is the " + "checkout directory itself") + continue + try: + if not resolved.exists(): continue - if path.is_dir(): - shutil.rmtree(path, ignore_errors=True) + if resolved.is_dir(): + shutil.rmtree(resolved, ignore_errors=True) else: - path.unlink() + resolved.unlink() except OSError as exc: - print(f"[WARNING] Could not remove {path}: {exc}") + print(f"[WARNING] Could not remove {resolved}: {exc}") continue - print(f"[OK] Removed unused model {path}") + print(f"[OK] Removed unused model {resolved}") removed += 1 return removed diff --git a/app/backends/audiocpp/remote.py b/app/backends/audiocpp/remote.py index 31eddbf..20d4cef 100644 --- a/app/backends/audiocpp/remote.py +++ b/app/backends/audiocpp/remote.py @@ -1,6 +1,7 @@ """Query a running audiocpp_server for its models and voices.""" import json +import urllib.parse import urllib.request from typing import Dict, List, Optional diff --git a/app/backends/audiocpp/status.py b/app/backends/audiocpp/status.py index 9ccf8cc..a687066 100644 --- a/app/backends/audiocpp/status.py +++ b/app/backends/audiocpp/status.py @@ -54,16 +54,20 @@ def detect() -> BackendStatus: launch = format_launch_hint(specs) managed = servers.manages(specs) remote_running, remote_urls = _detect_remote(managed) + running = managed or remote_running # A more specific "part-way set up" label than unavailable/installed: - # cloned but never built, or built but not configured. + # cloned but never built, or built but not configured. Only meaningful + # while the backend is not usable (a running server makes even a + # non-built checkout usable remotely — see the BackendStatus docstring). partial = "" - if not built: - partial = "downloaded (not built)" - elif not configured: - partial = "built (not configured)" + if not running: + if not built: + partial = "downloaded (not built)" + elif not configured: + partial = "built (not configured)" return BackendStatus("audiocpp", "audio.cpp", installed=built, configured=configured, - running=managed or remote_running, + running=running, details=details, launch_hint=launch, servers=specs, managed=managed, remote=remote_running, remote_urls=remote_urls, @@ -73,18 +77,12 @@ def detect() -> BackendStatus: def _detect_remote(managed: bool = False) -> Tuple[bool, dict]: """Detect an externally-run audiocpp_server at the remote URL. - Returns ``(running, {spec_name: url})``. The remote URL is probed only - when configured (non-empty); a server answering there is ignored when it - is this tool's own managed server (remote URL == local URL and our pid is - still alive) — that instance is already reported as "[local]". + Returns ``(running, {spec_name: url})``; see probe.detect_remote_url + for the shared semantics. """ - url = (config.AUDIOCPP_REMOTE_URL or "").strip() - if not url: - return False, {} - if managed and probe.same_endpoint(url, config.AUDIOCPP_API_URL): - return False, {} - if probe.identify_server(url) == probe.IDENTITY_AUDIOCPP: - return True, {"audiocpp": url} - return False, {} + return probe.detect_remote_url(config.AUDIOCPP_REMOTE_URL, + config.AUDIOCPP_API_URL, + probe.IDENTITY_AUDIOCPP, "audiocpp", + managed) diff --git a/app/backends/audiocpp/wizard.py b/app/backends/audiocpp/wizard.py index a1a2a6a..52a4f3f 100644 --- a/app/backends/audiocpp/wizard.py +++ b/app/backends/audiocpp/wizard.py @@ -331,7 +331,6 @@ def _wizard(stdscr, args: argparse.Namespace, parser: argparse.ArgumentParser "build_mode": s.get("build_mode"), "prebuilt_forced": False, "lazy_load": True, - "sync_port": None, "wav_dir": s["wav_dir"], "plan": s["plan"], "download": s["download"], @@ -1093,7 +1092,6 @@ def _collect_from_flags(args: argparse.Namespace, "build_mode": build_mode, "prebuilt_forced": prebuilt_forced, "lazy_load": lazy_load, - "sync_port": None, "wav_dir": wav_dir, "plan": plan, "download": args.download, diff --git a/app/backends/common.py b/app/backends/common.py index d25a03e..24746b6 100644 --- a/app/backends/common.py +++ b/app/backends/common.py @@ -193,12 +193,32 @@ def wav_dir_preview(directory: Path) -> Tuple[str, str]: return ("no .wav files", "info") +def port_of(url: str, fallback: int) -> int: + """URL's explicit port, else FALLBACK (invalid URLs fall back too).""" + try: + return urllib.parse.urlsplit(url).port or fallback + except ValueError: + return fallback + + def url_with_port(url: str, port: int) -> str: - """Return URL with its port replaced/inserted as PORT.""" + """Return URL with its port replaced/inserted as PORT. + + Preserves the userinfo ("user:pass@host") and brackets IPv6 hosts + ("[::1]"), which a plain f"{host}:{port}" rebuild would mangle. + """ parts = urllib.parse.urlsplit(url) host = parts.hostname or "127.0.0.1" + if ":" in host and not host.startswith("["): + host = f"[{host}]" + netloc = f"{host}:{port}" + if parts.username: + cred = parts.username + if parts.password: + cred = f"{cred}:{parts.password}" + netloc = f"{cred}@{netloc}" return urllib.parse.urlunsplit( - (parts.scheme or "http", f"{host}:{port}", parts.path, "", "")) + (parts.scheme or "http", netloc, parts.path, "", "")) def normalize_remote_url(value: str) -> str: @@ -408,14 +428,27 @@ def run_console_subprocess(argv: List[str], cwd: Optional[Path] = None, def _reader() -> None: nonlocal last_output try: - for raw in iter(proc.stdout.readline, b""): - if not raw: + pending = "" + while True: + # read1 (not read/readline) returns whatever a single pipe + # read yields, without waiting to fill the buffer, and \r + # is treated as a line break: carriage-return progress bars + # (git clone --progress, tqdm/HuggingFace downloads) then + # surface incrementally and keep the stall watchdog fed — + # a readline-based reader blocked until the next \n would + # let a healthy download starve to the timeout. + chunk = proc.stdout.read1(4096) + if not chunk: break last_output = time.monotonic() - text = raw.decode("utf-8", errors="replace") - for line in text.splitlines(): + pending += chunk.decode("utf-8", errors="replace") + parts = re.split(r"[\r\n]+", pending) + pending = parts.pop() + for line in parts: if line: emit(line) + if pending.strip(): + emit(pending) except (OSError, ValueError): pass @@ -448,11 +481,10 @@ def run_console_subprocess(argv: List[str], cwd: Optional[Path] = None, _terminate_process_group(proc) break time.sleep(0.1) - try: - reader.join(timeout=5) - finally: - if reader.is_alive(): - reader.join(timeout=0) + # The reader is a daemon: a join timeout here only means the child + # closed its stdout but the thread is still draining — there is + # nothing useful left to wait for. + reader.join(timeout=5) if cancelled: return 130 if stalled: @@ -476,10 +508,17 @@ def _terminate_process_group(proc) -> None: """ import signal if sys.platform == "win32": + # TerminateProcess hits a single pid; children the server spawned + # would survive as orphans. taskkill /T walks and kills the whole + # process tree, then the poll loop reaps the direct child. try: - proc.terminate() - except OSError: - pass + subprocess.run(["taskkill", "/T", "/F", "/PID", str(proc.pid)], + capture_output=True, timeout=10) + except (OSError, subprocess.SubprocessError): + try: + proc.terminate() + except OSError: + pass deadline = time.time() + 10 while time.time() < deadline: if proc.poll() is not None: diff --git a/app/backends/faster.py b/app/backends/faster.py index 2d0cad3..34fae45 100755 --- a/app/backends/faster.py +++ b/app/backends/faster.py @@ -59,6 +59,8 @@ from ui import taskview, tui FASTER_DIR_NAME = "faster-qwen3-tts" FASTER_GIT_URL = "https://github.com/andimarafioti/faster-qwen3-tts" FASTER_PIP_PKG = "faster-qwen3-tts[demo]" +# The managed server's port when the configured URL names none. +DEFAULT_PORT = 8000 # The dedicated venv faster-qwen3-tts is installed into (never the app env # or the qwen backend's; the wheel pulls its own qwen-tts-hf dependency, # which ships the same qwen_tts module upstream qwen-tts does). @@ -78,14 +80,6 @@ def _is_cloned() -> bool: return (_checkout() / "examples" / "openai_server.py").is_file() -def _config_port() -> int: - import urllib.parse - try: - return urllib.parse.urlsplit(config.FASTER_API_URL).port or 8000 - except ValueError: - return 8000 - - def build_voices(wav_files: list, language: str, whisper_model: str) -> dict: """Transcribe each wav file and build the voices mapping.""" voices = {} @@ -363,7 +357,7 @@ def _print_launch_hint(voices_path: Path) -> None: return print("[INFO] Clone faster-qwen3-tts to get examples/openai_server.py,") print(f" then run it with --voices {voices_path} " - f"--port {_config_port()}") + f"--port {common.port_of(config.FASTER_API_URL, DEFAULT_PORT)}") def setup_screen(stdscr) -> int: @@ -471,7 +465,8 @@ def detect() -> BackendStatus: if cloned and voices_json.exists(): argv = [str(envs.env_python(FASTER_ENV)), str(_checkout() / "examples" / "openai_server.py"), - "--voices", str(voices_json), "--port", str(_config_port())] + "--voices", str(voices_json), "--port", + str(common.port_of(config.FASTER_API_URL, DEFAULT_PORT))] # identity: /health must report model_loaded before the server is # really usable (the model loads after the port opens). specs = [ServerSpec("faster", config.FASTER_API_URL, argv, @@ -491,17 +486,13 @@ def detect() -> BackendStatus: def _detect_remote(managed: bool = False): """Detect an externally-run faster server at the remote URL. - Returns ``(running, {spec_name: url})``; see audiocpp._detect_remote for - the shared semantics (empty URL disables, own server not counted twice). + Returns ``(running, {spec_name: url})``; see probe.detect_remote_url + for the shared semantics (empty URL disables, own server not counted + twice). """ - url = (config.FASTER_REMOTE_URL or "").strip() - if not url: - return False, {} - if managed and probe.same_endpoint(url, config.FASTER_API_URL): - return False, {} - if probe.identify_server(url) == probe.IDENTITY_FASTER: - return True, {"faster": url} - return False, {} + return probe.detect_remote_url(config.FASTER_REMOTE_URL, + config.FASTER_API_URL, + probe.IDENTITY_FASTER, "faster", managed) def update(*, emit=None, cancel=None) -> int: diff --git a/app/backends/probe.py b/app/backends/probe.py index 64dc7ab..818aadb 100644 --- a/app/backends/probe.py +++ b/app/backends/probe.py @@ -142,6 +142,27 @@ def same_endpoint(url_a: str, url_b: str) -> bool: return host_a == host_b and port_a == port_b +def detect_remote_url(remote_url: str, local_url: str, identity: str, + name: str, managed: bool = False) -> tuple: + """The shared backend "is something answering at the remote URL?" check. + + Returns ``(running, {name: url})``. An empty URL disables the check; a + remote URL equal to the configured local endpoint is ignored while + MANAGED (that server was started by this tool and is already reported + as "[local]"); otherwise the URL must answer HTTP as IDENTITY to count. + The faster and audio.cpp backends use it verbatim; qwen's is + model-aware and keeps its own variant. + """ + url = (remote_url or "").strip() + if not url: + return False, {} + if managed and same_endpoint(url, local_url): + return False, {} + if identify_server(url) == identity: + return True, {name: url} + return False, {} + + def health_payload(url: str, timeout: float = DEFAULT_TIMEOUT) -> Optional[dict]: """Return the server's ``/health`` JSON document, or None. diff --git a/app/backends/qwen.py b/app/backends/qwen.py index a94b3cc..c099c84 100644 --- a/app/backends/qwen.py +++ b/app/backends/qwen.py @@ -160,7 +160,10 @@ def delete_model_weights(models: Optional[List[str]] = None) -> int: if not directory.is_dir(): continue print(f"[INFO] Removing cached {MODEL_REPOS[name]} weights...") - shutil.rmtree(directory, ignore_errors=True) + shutil.rmtree(directory) + if directory.exists(): + print(f"[WARNING] Could not fully remove {directory}") + continue removed += 1 if removed: print(f"[OK] Deleted cached weights for {removed} " @@ -174,19 +177,6 @@ def _is_installed() -> bool: return envs.module_available("qwen_tts", QWEN_ENV) -def _config_port(url: str, fallback: int) -> int: - import urllib.parse - try: - return urllib.parse.urlsplit(url).port or fallback - except ValueError: - return fallback - - -def current_model() -> str: - """The model a fresh managed start hosts (DEFAULT_MODEL).""" - return DEFAULT_MODEL - - def model_for_identity(identity: Optional[str]) -> Optional[str]: """The model name a qwen demo answers as (None when not a known identity).""" return IDENTITY_TO_MODEL.get(identity) @@ -308,7 +298,7 @@ def build_spec(model: str) -> ServerSpec: "qwen", url, [str(envs.env_script("qwen-tts-demo", QWEN_ENV)), MODEL_REPOS[model], "--ip", "127.0.0.1", - "--port", str(_config_port(url, DEFAULT_PORT))], + "--port", str(common.port_of(url, DEFAULT_PORT))], identity=desired_identity(model)) @@ -336,12 +326,12 @@ def detect() -> BackendStatus: differs from the default one. """ installed = _is_installed() - model = current_model() + model = DEFAULT_MODEL url = config.QWEN_API_URL details: List[str] = [] details.append("pip: installed" if installed else "not installed — run setup to pip install qwen-tts") - details.append(f"port: {_config_port(url, DEFAULT_PORT)}") + details.append(f"port: {common.port_of(url, DEFAULT_PORT)}") details.append(f"default model: {model}") specs = [build_spec(model)] managed = servers.manages(specs) 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) |