fix(agent): break immediately on interpreter shutdown in conversation loop
When the Python interpreter begins teardown (user closes hermes, SIGTERM, OOM-kill), every executor-backed operation raises 'cannot schedule new futures after interpreter shutdown'. The outer except handler in run_conversation caught this error but did not recognize it as fatal — it kept retrying (API calls #4, #5, #6) until max_iterations, each time hitting the same dead executor and printing another traceback. The fix adds an early check: if sys.is_finalizing() or the error matches the 'cannot schedule new futures' pattern, break immediately with a clean interpreter_shutdown exit reason instead of retrying. The codebase already had this pattern in cron/scheduler.py and agent/tool_executor.py — the conversation loop just wasn't using it.
This commit is contained in:
@@ -22,6 +22,7 @@ import os
|
||||
import random
|
||||
import re
|
||||
import ssl
|
||||
import sys
|
||||
import time
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
@@ -272,6 +273,24 @@ _API_CALL_MODULES = frozenset({
|
||||
})
|
||||
|
||||
|
||||
def _is_interpreter_shutdown_error(exc: Exception) -> bool:
|
||||
"""Check if *exc* is a fatal interpreter-shutdown failure.
|
||||
|
||||
During teardown, ``concurrent.futures`` refuses new work with
|
||||
``RuntimeError: cannot schedule new futures after interpreter shutdown``
|
||||
(or the shorter ``... after shutdown`` variant from a plain
|
||||
ThreadPoolExecutor). Both are documented in #58720. The common
|
||||
prefix catches both; the module-global ``is_finalizing`` flag can
|
||||
lag the error by a hair, so matching the error text is the safe
|
||||
fallback for that race.
|
||||
"""
|
||||
if isinstance(exc, RuntimeError):
|
||||
msg = str(exc).lower()
|
||||
if "cannot schedule new futures" in msg:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _moa_client_consumes_prepared_request(client: Any) -> bool:
|
||||
"""True when ``client`` is the in-process MoA facade.
|
||||
|
||||
@@ -8309,6 +8328,43 @@ def run_conversation(
|
||||
# local post-processing helpers and never entered the interruptible
|
||||
# API-call helpers, it is almost certainly a local processing bug.
|
||||
# (#66267)
|
||||
#
|
||||
# Interpreter shutdown: if the process is tearing down, every
|
||||
# executor-backed operation (API call, tool dispatch, memory sync)
|
||||
# raises ``RuntimeError: cannot schedule new futures after
|
||||
# interpreter shutdown``. Retrying is pointless — the executor is
|
||||
# gone for good — and each retry just spams another traceback.
|
||||
# Break immediately so the turn exits cleanly. (#93217)
|
||||
if sys.is_finalizing() or _is_interpreter_shutdown_error(e):
|
||||
error_msg = (
|
||||
f"Interpreter is shutting down — cannot continue "
|
||||
f"(API call #{api_call_count}): {e}"
|
||||
)
|
||||
try:
|
||||
agent._safe_print(f"❌ {error_msg}")
|
||||
except (OSError, ValueError):
|
||||
pass
|
||||
logger.warning(error_msg)
|
||||
# Best-effort persist — the executor is dying, so this may
|
||||
# raise the same RuntimeError. Don't let that mask the
|
||||
# shutdown exit. finalize_turn will retry the persist.
|
||||
try:
|
||||
agent._persist_session(messages, conversation_history)
|
||||
except Exception:
|
||||
pass
|
||||
_turn_exit_reason = "interpreter_shutdown"
|
||||
final_response = (
|
||||
"Session is shutting down. Your conversation can be "
|
||||
"resumed with: hermes --resume <session-id>"
|
||||
)
|
||||
# Don't append the assistant message here — a thinking-prefill
|
||||
# or interim assistant may already be the tail, and appending
|
||||
# would create assistant→assistant. finalize_turn handles
|
||||
# this case safely (lines 341-353: appends only when
|
||||
# _tail_role != "assistant"), matching the pattern at other
|
||||
# break sites that set final_response without appending.
|
||||
break
|
||||
|
||||
tb_module_names: set[str] = set()
|
||||
_tb = e.__traceback__
|
||||
while _tb is not None:
|
||||
|
||||
68
tests/agent/test_conversation_loop_interpreter_shutdown.py
Normal file
68
tests/agent/test_conversation_loop_interpreter_shutdown.py
Normal file
@@ -0,0 +1,68 @@
|
||||
"""Regression guard: interpreter-shutdown errors must abort the conversation
|
||||
loop immediately instead of retrying.
|
||||
|
||||
When the Python interpreter begins its teardown sequence, every executor-backed
|
||||
operation (API calls, tool dispatch, memory sync) raises::
|
||||
|
||||
RuntimeError: cannot schedule new futures after interpreter shutdown
|
||||
|
||||
Before the fix, the outer ``except Exception`` handler in
|
||||
``run_conversation`` caught this error but did not recognise it as fatal.
|
||||
Since ``api_call_count`` was nowhere near ``agent.max_iterations - 1``, the
|
||||
loop continued — each iteration hit the same dead executor and failed
|
||||
identically, producing a cascade of ``❌ Error during OpenAI-compatible API
|
||||
call #N`` messages (#93217).
|
||||
|
||||
The fix adds an early check in the outer except handler: if
|
||||
``sys.is_finalizing()`` is True or the error message matches the
|
||||
``"cannot schedule new futures"`` pattern, the loop breaks immediately with
|
||||
a clean ``interpreter_shutdown`` exit reason.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from agent.conversation_loop import _is_interpreter_shutdown_error
|
||||
|
||||
|
||||
class TestInterpreterShutdownDetection:
|
||||
"""Verify the interpreter-shutdown error matcher used by the
|
||||
conversation loop's outer except handler."""
|
||||
|
||||
def test_matches_full_interpreter_shutdown_message(self):
|
||||
"""The canonical CPython asyncio shutdown message."""
|
||||
exc = RuntimeError(
|
||||
"cannot schedule new futures after interpreter shutdown"
|
||||
)
|
||||
assert _is_interpreter_shutdown_error(exc) is True
|
||||
|
||||
def test_matches_short_shutdown_variant(self):
|
||||
"""Plain ThreadPoolExecutor shutdown variant (no 'interpreter')."""
|
||||
exc = RuntimeError("cannot schedule new futures after shutdown")
|
||||
assert _is_interpreter_shutdown_error(exc) is True
|
||||
|
||||
def test_case_insensitive_match(self):
|
||||
"""Error text may arrive in different case from some executor types."""
|
||||
exc = RuntimeError("Cannot Schedule New Futures After Interpreter Shutdown")
|
||||
assert _is_interpreter_shutdown_error(exc) is True
|
||||
|
||||
def test_does_not_match_unrelated_runtime_error(self):
|
||||
"""Unrelated RuntimeErrors must not trigger the shutdown path."""
|
||||
exc = RuntimeError("connection reset by peer")
|
||||
assert _is_interpreter_shutdown_error(exc) is False
|
||||
|
||||
def test_does_not_match_non_runtime_error(self):
|
||||
"""Non-RuntimeError exceptions must not match."""
|
||||
exc = ValueError("cannot schedule new futures")
|
||||
assert _is_interpreter_shutdown_error(exc) is False
|
||||
|
||||
def test_does_not_match_none(self):
|
||||
"""None must not match (defensive — caller may pass None)."""
|
||||
try:
|
||||
result = _is_interpreter_shutdown_error(None) # type: ignore[arg-type]
|
||||
except TypeError:
|
||||
result = False
|
||||
assert result is False
|
||||
|
||||
def test_does_not_match_empty_string_exception(self):
|
||||
"""Empty-message exceptions must not match."""
|
||||
exc = RuntimeError("")
|
||||
assert _is_interpreter_shutdown_error(exc) is False
|
||||
Reference in New Issue
Block a user