diff --git a/tests/tools/test_code_kernel.py b/tests/tools/test_code_kernel.py index 9a474d3583..c3dbc29203 100644 --- a/tests/tools/test_code_kernel.py +++ b/tests/tools/test_code_kernel.py @@ -127,165 +127,50 @@ class TestSessionStatePersistence(unittest.TestCase): class TestKernelLifecycle(unittest.TestCase): - @pytest.mark.skipif(sys.platform != "win32", reason="Windows process-handle contract") def test_kernel_exits_when_its_backend_parent_dies(self): - """A long-running cell must not outlive the backend that spawned it.""" + """A kernel must not outlive the host that spawned it, even when the + host dies without cleanup (SIGKILL/OOM/crash). Windows: inherited + SYNCHRONIZE handle; POSIX: inherited death pipe. Both are proven the + same way — kill the host mid-cell, the kernel is gone within seconds.""" import psutil - tmpdir = tempfile.TemporaryDirectory(prefix="hermes_parent_death_test_") - tmp_path = Path(tmpdir.name) - repo_root = Path(__file__).resolve().parents[2] - state_path = tmp_path / "kernel-state.json" - cell_started_path = tmp_path / "cell-started" - parent_script = tmp_path / "spawn-kernel-parent.py" - parent_script.write_text( - textwrap.dedent( - f""" - import json - import sys - import time - from pathlib import Path - - import psutil - - from tools.code_kernel import SessionKernel, _spawn - - state_path = Path(sys.argv[1]) - cell_started_path = Path(sys.argv[2]) - kernel = SessionKernel(("parent-death-test",)) - _spawn( - kernel, - task_id="parent-death-test", - child_python=sys.executable, - child_cwd="", - sandbox_tools=frozenset(), - max_tool_calls=1, - ) - child = psutil.Process(kernel.proc.pid) - state_tmp_path = state_path.with_suffix(".tmp") - state_tmp_path.write_text( - json.dumps( - {{ - "pid": kernel.proc.pid, - "create_time": child.create_time(), - "tmpdir": kernel.tmpdir, - }} - ), - encoding="utf-8", - ) - state_tmp_path.replace(state_path) - request = json.dumps( - {{ - "id": "long-cell", - "code": ( - "import __main__, json, os\\n" - "from pathlib import Path\\n" - f"Path({{str(cell_started_path)!r}}).write_text(\\n" - " json.dumps({{\\n" - " 'env_present': " - "'HERMES_KERNEL_PARENT_PROCESS_HANDLE' in os.environ,\\n" - " 'global_present': bool(getattr(\\n" - " __main__, '_PARENT_PROCESS_HANDLE', ''\\n" - " )),\\n" - " }})\\n" - ")\\n" - "import time\\n" - "time.sleep(300)" - ), - }} - ) + "\\n" - kernel.proc.stdin.write(request.encode("utf-8")) - kernel.proc.stdin.flush() - while True: - time.sleep(60) - """ - ), - encoding="utf-8", - ) - env = os.environ.copy() - env["PYTHONPATH"] = str(repo_root) + os.pathsep + env.get("PYTHONPATH", "") - env["HERMES_HOME"] = str(tmp_path / "hermes-home") - parent = subprocess.Popen( - [sys.executable, str(parent_script), str(state_path), str(cell_started_path)], - cwd=str(repo_root), - env=env, - stdin=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, - stderr=subprocess.PIPE, - text=True, - encoding="utf-8", - errors="replace", - creationflags=subprocess.CREATE_NO_WINDOW, - ) - - child_pid = None - child_create_time = None - child_tmpdir = None - - def child_identity_alive(): - if child_pid is None or child_create_time is None: - return False - try: - return psutil.Process(child_pid).create_time() == child_create_time - except psutil.NoSuchProcess: - return False - - try: - deadline = time.monotonic() + 20 - while time.monotonic() < deadline: - if child_pid is None and state_path.exists(): - state = json.loads(state_path.read_text(encoding="utf-8")) - child_pid = int(state["pid"]) - child_create_time = float(state["create_time"]) - child_tmpdir = Path(state["tmpdir"]) - if child_pid is not None and cell_started_path.exists(): - break - if parent.poll() is not None: - stderr = parent.stderr.read() if parent.stderr is not None else "" - self.fail( - f"kernel parent exited before setup: {parent.returncode}: {stderr}" - ) - time.sleep(0.05) - self.assertIsNotNone(child_pid, "kernel child did not start") - assert child_pid is not None - self.assertIsNotNone(child_create_time, "kernel identity was not recorded") - self.assertTrue(cell_started_path.exists(), "kernel cell did not start") - cell_state = json.loads(cell_started_path.read_text(encoding="utf-8")) - self.assertFalse(cell_state["env_present"], "parent handle leaked via env") - self.assertFalse(cell_state["global_present"], "parent handle leaked via runner globals") - self.assertTrue(child_identity_alive(), "kernel child was never alive") - - parent.kill() - parent.wait(timeout=10) - - deadline = time.monotonic() + 10 - while time.monotonic() < deadline and child_identity_alive(): - time.sleep(0.05) - self.assertFalse( - child_identity_alive(), - "session kernel survived its backend parent", + repo_root = str(Path(__file__).resolve().parents[2]) + host_src = textwrap.dedent(f""" + import json, os, sys, time + os.environ["HERMES_HOME"] = sys.argv[1] + sys.path.insert(0, {repo_root!r}) + from tools.code_kernel import SessionKernel, _spawn + k = SessionKernel(("parent-death",)) + _spawn(k, task_id="parent-death", child_python=sys.executable, + child_cwd="", sandbox_tools=frozenset(), max_tool_calls=1) + cell = json.dumps({{"id": "x", "code": "import os, time\\n" + "assert 'HERMES_KERNEL_PARENT_PROCESS_HANDLE' not in os.environ\\n" + "assert 'HERMES_KERNEL_PARENT_DEATH_FD' not in os.environ\\n" + "time.sleep(300)"}}) + "\\n" + k.proc.stdin.write(cell.encode()); k.proc.stdin.flush() + print(k.proc.pid, flush=True) + time.sleep(600) + """) + with tempfile.TemporaryDirectory() as home: + host = subprocess.Popen( + [sys.executable, "-c", host_src, home], + stdout=subprocess.PIPE, text=True, + creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), ) - finally: - if parent.poll() is None: - parent.kill() - parent.wait(timeout=10) - if child_pid is not None: + try: + kernel = psutil.Process(int(host.stdout.readline())) + time.sleep(0.5) + self.assertTrue(kernel.is_running(), "kernel never came up") + host.kill() + host.wait(timeout=10) try: - child = psutil.Process(child_pid) - if child.create_time() == child_create_time: - child.kill() - child.wait(timeout=10) - except psutil.NoSuchProcess: - pass - if child_tmpdir is not None: - temp_root = Path(tempfile.gettempdir()).resolve() - resolved_tmpdir = child_tmpdir.resolve() - if ( - resolved_tmpdir.parent == temp_root - and resolved_tmpdir.name.startswith("hermes_kernel_") - ): - shutil.rmtree(resolved_tmpdir, ignore_errors=True) - tmpdir.cleanup() + kernel.wait(timeout=10) + except psutil.TimeoutExpired: + kernel.kill() + self.fail("session kernel survived its backend parent") + finally: + if host.poll() is None: + host.kill() def test_timeout_kills_the_kernel_and_reports_state_loss(self): with _kernel_config(timeout=1): diff --git a/tools/code_kernel.py b/tools/code_kernel.py index 8044b41c1b..166e32badc 100644 --- a/tools/code_kernel.py +++ b/tools/code_kernel.py @@ -86,6 +86,39 @@ _CAPTURE_LIMIT = {capture_limit} _SPILL_DIR = os.environ.get("HERMES_KERNEL_SPILL_DIR", "") _SPILL_CAP = {spill_cap} _PARENT_PROCESS_HANDLE = os.environ.pop("HERMES_KERNEL_PARENT_PROCESS_HANDLE", "") +_PARENT_DEATH_FD = os.environ.pop("HERMES_KERNEL_PARENT_DEATH_FD", "") + + +def _start_parent_death_pipe_watchdog(): + """POSIX twin of the Windows handle watchdog: exit when the parent dies. + + The host holds the only write end of an inherited pipe; a blocking read + returns EOF the instant the host exits by ANY means (SIGKILL, OOM, crash), + exactly like the MCP death supervisor. Stdin EOF alone is not enough: the + main loop only sees it between cells, so a kernel SIGKILLed mid-cell + outlived its host. Not PR_SET_PDEATHSIG — that is bound to the spawning + THREAD, and kernels are spawned from per-cell threads that exit. + """ + global _PARENT_DEATH_FD + raw_fd = _PARENT_DEATH_FD + _PARENT_DEATH_FD = "" + if sys.platform == "win32" or not raw_fd: + return + try: + fd = int(raw_fd) + os.set_inheritable(fd, False) + except (OSError, ValueError): + return + + def _wait(): + try: + while os.read(fd, 1): + pass + except OSError: + pass + os._exit(0) + + threading.Thread(target=_wait, name="hermes-parent-watchdog", daemon=True).start() def _start_parent_process_watchdog(): @@ -139,6 +172,7 @@ def _start_parent_process_watchdog(): _start_parent_process_watchdog() +_start_parent_death_pipe_watchdog() # The persistent cell namespace. `__name__` is `__main__` so scripts behave # like the per-call path; builtins resolve normally through exec. @@ -314,6 +348,7 @@ class SessionKernel: self.stop_event = threading.Event() self.rpc_token: str = "" self.sentinel: str = "" + self.death_pipe_w: Optional[int] = None self.tool_call_log: List = [] self.tool_call_counter: List[int] = [0] # Cells currently attached to this kernel (bumped under _KERNELS_LOCK @@ -475,6 +510,12 @@ atexit.register(shutdown_all_kernels) def _teardown(kernel: SessionKernel) -> None: kernel.stop_event.set() + if kernel.death_pipe_w is not None: + try: + os.close(kernel.death_pipe_w) + except OSError: + pass + kernel.death_pipe_w = None if kernel.proc is not None and kernel.proc.poll() is None: from tools.code_execution_tool import _kill_process_group @@ -706,6 +747,13 @@ def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, close_parent_process_handle = None startupinfo = None + death_r: Optional[int] = None + pass_fds: Tuple[int, ...] = () + if not _IS_WINDOWS: + death_r, kernel.death_pipe_w = os.pipe() + child_env["HERMES_KERNEL_PARENT_DEATH_FD"] = str(death_r) + pass_fds = (death_r,) + try: kernel.proc = subprocess.Popen( [child_python, runner_path], @@ -719,11 +767,14 @@ def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, start_new_session=True, creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0, close_fds=True, + pass_fds=pass_fds, startupinfo=startupinfo, ) finally: if parent_process_handle and close_parent_process_handle is not None: close_parent_process_handle(parent_process_handle) + if death_r is not None: + os.close(death_r) # Deliberately NOT propagate_context_to_thread: that would freeze the # spawning cell's context/callbacks into the server thread for the