From 365e2835d490a053d076daa3b429371d6f35210f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:39:43 -0700 Subject: [PATCH] fix(execute_code): POSIX kernels exit when their host dies mid-cell (death pipe) Widens the Windows parent-death fix to the class: the kernel inherits the read end of a pipe whose only write end lives in the host, so host death by any means (SIGKILL, OOM, crash) is EOF and the kernel exits. Stdin EOF alone only reaches the runner between cells, so a kernel SIGKILLed mid-cell used to outlive its host indefinitely. The integration test now runs on every platform and is trimmed to the contract (kill host mid-cell, kernel gone), not the handle plumbing. --- tests/tools/test_code_kernel.py | 193 +++++++------------------------- tools/code_kernel.py | 51 +++++++++ 2 files changed, 90 insertions(+), 154 deletions(-) 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