From e63786fc23a91a01bf033db461f65c36acc2fc30 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 24 Aug 2026 04:07:13 +0530 Subject: [PATCH] fix(agent): break immediately on interpreter shutdown in conversation loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/conversation_loop.py | 56 +++++++++++++++ ..._conversation_loop_interpreter_shutdown.py | 68 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tests/agent/test_conversation_loop_interpreter_shutdown.py diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 2d49501518..6cbfd3f4cd 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -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 " + ) + # 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: diff --git a/tests/agent/test_conversation_loop_interpreter_shutdown.py b/tests/agent/test_conversation_loop_interpreter_shutdown.py new file mode 100644 index 0000000000..f917b6a6b9 --- /dev/null +++ b/tests/agent/test_conversation_loop_interpreter_shutdown.py @@ -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