fix(tui): recover from spurious stdin EOF caused by child O_NONBLOCK flip
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= 参数
This commit is contained in:
parent
39b30bacf7
commit
01c02c6f8f
4 changed files with 184 additions and 12 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue