From 01c02c6f8f93e0c6a145b4594cb0705e1dbeb5d4 Mon Sep 17 00:00:00 2001 From: x7peeps Date: Mon, 20 Jul 2026 05:12:01 +0800 Subject: [PATCH] fix(tui): recover from spurious stdin EOF caused by child O_NONBLOCK flip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #67639 根因分析: 子进程继承 fd 0 (stdin) 后设置 O_NONBLOCK 标志时，该标志作用于共享的 open file description 而非单个文件描述符。这导致 gateway 的下一次 read() 返回 EAGAIN，CPython 的缓冲层将其转换为 b''，表现为 EOF，gateway 因此 意外退出。这不是真正的 TUI 关闭管道，而是子进程修改了共享文件状态。 修复涉及三个 Gap: Gap 1 — check_subprocess_stdin.py 扫描范围不足: - 原正则仅匹配 subprocess.run/Popen，扩展到 call/check_output/check_call/ os.system/asyncio.create_subprocess_exec/shell - 新增扫描 ~/.hermes/plugins/ 和 ./.hermes/plugins/ 用户插件目录 (hermes_cli/plugins.py:10-12 定义的插件加载路径) Gap 2 — Gateway 无自愈能力: - entry.py 和 slash_worker.py 的 stdin 循环改为 while True + readline() 模式 - 当 readline() 返回空字符串时，检查 O_NONBLOCK 标志判断是否为虚假 EOF - 虚假 EOF → 恢复 blocking 模式并继续; 真实 EOF → 正常退出 - 添加恢复频率限制 (10次/分钟)，防止无限循环 - 添加 _diagnose_stdin_state() 诊断函数，记录 O_NONBLOCK/SO_RCVTIMEO 状态 Gap 3 — 日志消息误导: - 原 "stdin EOF (TUI closed the command pipe)" 改为 "stdin EOF (peer closed)" 或 "stdin spurious EOF (subprocess O_NONBLOCK flip)"，附带诊断信息 附带修复: compute_host.py 的 subprocess.check_output 缺少 stdin= 参数 --- scripts/check_subprocess_stdin.py | 53 ++++++++++++++++++--- tui_gateway/compute_host.py | 2 +- tui_gateway/entry.py | 78 +++++++++++++++++++++++++++++-- tui_gateway/slash_worker.py | 63 ++++++++++++++++++++++++- 4 files changed, 184 insertions(+), 12 deletions(-) diff --git a/scripts/check_subprocess_stdin.py b/scripts/check_subprocess_stdin.py index 4d312a594..9ee42e1d4 100644 --- a/scripts/check_subprocess_stdin.py +++ b/scripts/check_subprocess_stdin.py @@ -37,6 +37,25 @@ TUI_CONTEXT_DIRS = [ "tui_gateway/", ] +# User plugin roots — scanned at runtime if they exist. Plugins load from +# ~/.hermes/plugins/ and ./.hermes/plugins/ (hermes_cli/plugins.py:10-12), +# but the guard only checked the bundled plugins/ dir, missing user-installed +# code that spawns subprocesses (gap reported in #67639). +USER_PLUGIN_DIRS = [ + Path.home() / ".hermes" / "plugins", + Path.cwd() / ".hermes" / "plugins", +] + +# subprocess and os APIs that inherit stdin by default when called without +# an explicit stdin= argument. The original regex only covered run/Popen +# (gap #1 in #67639); call, check_output, check_call, os.system, and +# asyncio.create_subprocess_* all inherit fd 0 equally. +_SUBPROCESS_PATTERNS = [ + r"subprocess\.(run|Popen|call|check_output|check_call)\s*\([\"'a-zA-Z_\[\(]", + r"os\.system\s*\([\"'a-zA-Z_\[\(]", + r"asyncio\.create_subprocess_(exec|shell)\s*\([\"'a-zA-Z_\[\(]", +] + # Files with intentional stdin= override (e.g. input= creates a pipe). # Format: "filepath:line" or just "filepath" to skip the whole file. KNOWN_SAFE = { @@ -64,16 +83,14 @@ SKIP_DIRS = { def find_subprocess_calls(content: str, filepath: str) -> list[dict]: - """Find all subprocess.run/Popen calls missing stdin= in content.""" + """Find all subprocess/os/asyncio calls missing stdin= in content.""" violations = [] lines = content.split("\n") # Match only actual function calls — not comments, docstrings, or prose. - # The pattern requires an opening paren followed by an arg character - # (quote, bracket, letter, or closing paren for empty calls). - # This excludes ``subprocess.Popen(...)`` in docstrings and - # subprocess.run(...) in comments. - pattern = re.compile(r'subprocess\.(run|Popen)\s*\(["\'a-zA-Z_\[\(]') + # Multiple patterns cover subprocess.run/Popen/call/check_output/check_call, + # os.system, and asyncio.create_subprocess_exec/shell. + patterns = [re.compile(p) for p in _SUBPROCESS_PATTERNS] for i, line in enumerate(lines): # Skip comments. @@ -85,7 +102,7 @@ def find_subprocess_calls(content: str, filepath: str) -> list[dict]: if "``subprocess" in line: continue - if not pattern.search(line): + if not any(p.search(line) for p in patterns): continue # Collect the full call (may span multiple lines). @@ -161,6 +178,28 @@ def main() -> int: violations = find_subprocess_calls(content, rel) all_violations.extend(violations) + # Scan user plugin directories (Gap 1: guard missed ~/.hermes/plugins/ + # and ./.hermes/plugins/, where user-installed plugins like ori/hooks.py + # can spawn subprocesses with inherited stdin — #67639). + seen_roots: set[Path] = set() + for plugin_root in USER_PLUGIN_DIRS: + resolved = plugin_root.resolve() + if resolved in seen_roots or not resolved.is_dir(): + continue + seen_roots.add(resolved) + + for py_file in resolved.rglob("*.py"): + rel = str(py_file) + if py_file.name in ("conftest.py",) or "/tests/" in rel: + continue + + try: + content = py_file.read_text() + except Exception: + continue + violations = find_subprocess_calls(content, rel) + all_violations.extend(violations) + if all_violations: print(f"❌ {len(all_violations)} subprocess calls missing stdin=:") for v in all_violations: diff --git a/tui_gateway/compute_host.py b/tui_gateway/compute_host.py index bc2a42d4f..bfcf1b777 100644 --- a/tui_gateway/compute_host.py +++ b/tui_gateway/compute_host.py @@ -599,7 +599,7 @@ class ComputeHost: def _rss_mb(pid: int) -> float: try: - out = subprocess.check_output(["ps", "-o", "rss=", "-p", str(pid)], text=True, stderr=subprocess.DEVNULL, timeout=2).strip() + out = subprocess.check_output(["ps", "-o", "rss=", "-p", str(pid)], text=True, stdin=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=2).strip() return int(out.splitlines()[-1].strip()) / 1024.0 if out else 0.0 except Exception: return 0.0 diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index c738330f6..743bc144c 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -10,9 +10,11 @@ import hermes_bootstrap hermes_bootstrap.harden_import_path() +import fcntl import json import logging import signal +import socket import time import traceback @@ -290,6 +292,40 @@ def join_mcp_discovery(timeout: float | None = None) -> bool: return entry_done and startup_done +# Spurious stdin-EOF recovery tracker (shared open-file-description O_NONBLOCK flip). +_recovery_times: list[float] = [] +_MAX_RECOVERIES_PER_MINUTE = 10 + + +def _diagnose_stdin_state() -> str: + """Return a diagnostic string about stdin's current state. + + Used for crash-log forensics when stdin iteration falls through. + Distinguishes genuine peer-close (flag clear) from spurious EOF + caused by a child setting O_NONBLOCK on the shared file description. + """ + parts: list[str] = [] + try: + flags = fcntl.fcntl(0, fcntl.F_GETFL) + parts.append(f"O_NONBLOCK={'1' if flags & os.O_NONBLOCK else '0'}") + except Exception as e: + parts.append(f"F_GETFL error: {e}") + # SO_RCVTIMEO is a socket option (not a file-status flag), equally shared + # on the open file description. A child setting it via setsockopt launders + # into the same spurious-EOF path with O_NONBLOCK clear, so we report it + # alongside the flag. + try: + s = socket.fromfd(0, socket.AF_UNIX, socket.SOCK_STREAM) + try: + tv = s.getsockopt(socket.SOL_SOCKET, socket.SO_RCVTIMEO) + parts.append(f"SO_RCVTIMEO={tv}") + finally: + s.detach() + except Exception: + pass + return ", ".join(parts) if parts else "unknown" + + def main(): _install_sidecar_publisher() @@ -354,7 +390,45 @@ def main(): _log_exit("startup write failed (broken stdout pipe before first event)") sys.exit(0) - for raw in sys.stdin: + while True: + raw = sys.stdin.readline() + if not raw: + # Stdin iteration fell through — check if spurious (O_NONBLOCK flip + # by a child on the shared open file description) or genuine EOF. + try: + flags = fcntl.fcntl(0, fcntl.F_GETFL) + is_nonblock = bool(flags & os.O_NONBLOCK) + except Exception: + is_nonblock = False + + if not is_nonblock: + # Genuine peer-close — no subprocess flag tampering detected. + _log_exit("stdin EOF (peer closed)") + break + + # Spurious EOF: a child set O_NONBLOCK (or SO_RCVTIMEO) on the + # shared file description, laundered into b''/EAGAIN by CPython's + # buffered layer. Restore blocking mode and resume. + now = time.time() + _recovery_times.append(now) + _recovery_times[:] = [t for t in _recovery_times if t > now - 60] + if len(_recovery_times) > _MAX_RECOVERIES_PER_MINUTE: + _log_exit( + f"stdin spurious-EOF recovery rate exceeded " + f"({len(_recovery_times)}/min, cap {_MAX_RECOVERIES_PER_MINUTE})" + ) + break + + diag = _diagnose_stdin_state() + _log_exit( + f"stdin spurious EOF (subprocess O_NONBLOCK flip), recovering: {diag}" + ) + os.set_blocking(0, True) + # _io.TextIOWrapper readline returns empty string on EAGAIN but + # does NOT stick EOF; after restoring blocking, the next call + # will block until data arrives or the peer truly closes. + continue + line = raw.strip() if not line: continue @@ -374,8 +448,6 @@ def main(): _log_exit(f"response write failed for method={method!r} (broken stdout pipe)") sys.exit(0) - _log_exit("stdin EOF (TUI closed the command pipe)") - if __name__ == "__main__": main() diff --git a/tui_gateway/slash_worker.py b/tui_gateway/slash_worker.py index 78905a563..372addfbc 100644 --- a/tui_gateway/slash_worker.py +++ b/tui_gateway/slash_worker.py @@ -18,9 +18,11 @@ hermes_bootstrap.harden_import_path() import argparse import contextlib +import fcntl import io import json import os +import socket import sys import threading import time @@ -140,7 +142,66 @@ def main(): with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): cli = HermesCLI(model=args.model or None, compact=True, resume=args.session_key, verbose=False) - for raw in sys.stdin: + # Spurious stdin-EOF recovery (same O_NONBLOCK shared file-description + # issue as the gateway entry point — any child inheriting fd 0 can flip + # the flag and launder EAGAIN into an apparent EOF). + _sw_recovery_times: list[float] = [] + _SW_MAX_RECOVERIES_PER_MINUTE = 10 + + def _sw_diagnose_stdin() -> str: + parts: list[str] = [] + try: + flags = fcntl.fcntl(0, fcntl.F_GETFL) + parts.append(f"O_NONBLOCK={'1' if flags & os.O_NONBLOCK else '0'}") + except Exception as e: + parts.append(f"F_GETFL error: {e}") + try: + s = socket.fromfd(0, socket.AF_UNIX, socket.SOCK_STREAM) + try: + tv = s.getsockopt(socket.SOL_SOCKET, socket.SO_RCVTIMEO) + parts.append(f"SO_RCVTIMEO={tv}") + finally: + s.detach() + except Exception: + pass + return ", ".join(parts) if parts else "unknown" + + while True: + raw = sys.stdin.readline() + if not raw: + try: + flags = fcntl.fcntl(0, fcntl.F_GETFL) + is_nonblock = bool(flags & os.O_NONBLOCK) + except Exception: + is_nonblock = False + + if not is_nonblock: + # Genuine peer-close + break + + # Spurious EOF — recover + now = time.time() + _sw_recovery_times.append(now) + _sw_recovery_times[:] = [t for t in _sw_recovery_times if t > now - 60] + if len(_sw_recovery_times) > _SW_MAX_RECOVERIES_PER_MINUTE: + print( + f"[slash-worker] stdin spurious-EOF recovery rate exceeded " + f"({len(_sw_recovery_times)}/min)", + file=sys.stderr, + flush=True, + ) + break + + diag = _sw_diagnose_stdin() + print( + f"[slash-worker] stdin spurious EOF (subprocess O_NONBLOCK flip), " + f"recovering: {diag}", + file=sys.stderr, + flush=True, + ) + os.set_blocking(0, True) + continue + line = raw.strip() if not line: continue