From dfba247454384e659bfcc580fc2d76f9bb252986 Mon Sep 17 00:00:00 2001 From: jp Date: Sat, 18 Apr 2026 21:19:52 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20cross-platform=20PID=20check=20=E2=80=94?= =?UTF-8?q?=20os.kill(pid,=200)=20TERMINATES=20on=20Windows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real bug surfaced on CI for this PR. On POSIX, os.kill(pid, 0) is the canonical no-op existence probe. On Windows, Python's os.kill maps to TerminateProcess(handle, sig), which *terminates* the target with exit code sig. os.kill(pid, 0) therefore kills the target with exit code 0 — silently destroying our mine child (or, as happened in test_mine_already_running_live_pid, the pytest process itself). Fix: split into _pid_alive(pid) helper with a Windows branch using ctypes.windll.kernel32.OpenProcess + GetExitCodeProcess. PROCESS_QUERY_LIMITED_INFORMATION opens a handle only if the PID exists; STILL_ACTIVE (259) distinguishes running from exited processes. No new dependencies — stdlib ctypes. Co-Authored-By: Claude Opus 4.7 (1M context) --- mempalace/hooks_cli.py | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/mempalace/hooks_cli.py b/mempalace/hooks_cli.py index ce73fbf..92184f5 100644 --- a/mempalace/hooks_cli.py +++ b/mempalace/hooks_cli.py @@ -153,17 +153,46 @@ def _get_mine_dir(transcript_path: str = "") -> str: _MINE_PID_FILE = STATE_DIR / "mine.pid" +def _pid_alive(pid: int) -> bool: + """Cross-platform existence check for a PID. + + On POSIX, ``os.kill(pid, 0)`` is the well-known no-op existence probe. + On Windows, ``os.kill`` maps to ``TerminateProcess(handle, sig)`` and + would *terminate* the target process with exit code ``sig`` — using + it here would kill our own mine child (or worse, the caller itself). + Use ``OpenProcess`` + ``GetExitCodeProcess`` via ctypes instead. + """ + if sys.platform == "win32": + import ctypes + from ctypes import wintypes + + PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + STILL_ACTIVE = 259 + kernel32 = ctypes.windll.kernel32 + handle = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, False, pid) + if not handle: + return False + try: + code = wintypes.DWORD() + if not kernel32.GetExitCodeProcess(handle, ctypes.byref(code)): + return False + return code.value == STILL_ACTIVE + finally: + kernel32.CloseHandle(handle) + try: + os.kill(pid, 0) + return True + except (OSError, ValueError): + return False + + def _mine_already_running() -> bool: """Return True if a background mine process from a previous hook fire is still alive.""" try: pid = int(_MINE_PID_FILE.read_text().strip()) - os.kill(pid, 0) # signal 0 = existence check, no actual signal sent - return True except (OSError, ValueError): - # OSError covers: FileNotFoundError (no pid file), ProcessLookupError - # (dead PID on POSIX), PermissionError (not our process), and - # WinError 87 / "invalid parameter" (dead or unknown PID on Windows). return False + return _pid_alive(pid) def _spawn_mine(cmd: list) -> None: