Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions hermes_cli/oneshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,54 @@ def _create_session_db_for_oneshot():
return None


def _register_oneshot_memory_cleanup(agent: object) -> None:
"""Register an ``atexit`` hook that tears down ``agent``'s memory provider.

Oneshot bypasses the interactive CLI's ``atexit``/``_run_cleanup`` wiring,
so memory providers (e.g. Honcho) whose daemon worker threads are still
blocked in HTTP recv at interpreter exit never get ``shutdown()`` called.
CPython then forcibly kills those threads at ``Py_FinalizeEx`` via
``PyThread_exit_thread`` -> ``__pthread_unwind`` -> ``abort()``, producing
SIGABRT (exit 134) with no Python traceback. The hook tears the provider
down cleanly before finalize, then exits directly to bypass finalization.
"""
import atexit
import os
import sys

def _shutdown_oneshot_and_exit() -> None:
# Best-effort graceful teardown of the memory provider's background
# threads (joins + httpx client close; see HonchoMemoryProvider.shutdown).
try:
if hasattr(agent, "shutdown_memory_provider"):
# Forward the agent's own transcript so providers'
# ``on_session_end`` hooks see the real conversation instead of
# an empty list (#15165), mirroring the interactive CLI's
# ``_run_cleanup``. Fall back to no-arg when the attribute is
# missing (test stubs / partially-initialised agents).
session_msgs = getattr(agent, "_session_messages", None)
if isinstance(session_msgs, list):
agent.shutdown_memory_provider(session_msgs)
else:
agent.shutdown_memory_provider()
except Exception:
pass
# Some providers (Honcho, cloud-browser sessions) leave daemon worker
# threads blocked in C-level I/O that can't always be interrupted —
# notably when another async provider is also active — and CPython
# then aborts at Py_FinalizeEx (SIGABRT, exit 134). The oneshot has
# already emitted its output, so flush and exit directly to bypass
# interpreter finalization entirely (the OS reclaims the threads).
for stream in (sys.stdout, sys.stderr):
try:
stream.flush()
except Exception:
pass
os._exit(0)

atexit.register(_shutdown_oneshot_and_exit)


def _run_agent(
prompt: str,
model: Optional[str] = None,
Expand Down Expand Up @@ -364,6 +412,10 @@ def _run_agent(
agent.stream_delta_callback = None
agent.tool_gen_callback = None

# Oneshot bypasses the interactive CLI's atexit/_run_cleanup wiring, so
# register an equivalent memory-provider teardown hook directly.
_register_oneshot_memory_cleanup(agent)

return agent.chat(prompt) or ""


Expand Down
63 changes: 60 additions & 3 deletions plugins/memory/honcho/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1399,16 +1399,73 @@ def handle_tool_call(self, tool_name: str, args: dict, **kwargs) -> str:
return tool_error(f"Honcho {tool_name} failed: {e}")

def shutdown(self) -> None:
# CPython aborts (SIGABRT, exit 134) when daemon threads are still
# blocked in C-level I/O (httpx socket recv) at Py_FinalizeEx — the
# finalizer forcibly kills them via PyThread_exit_thread →
# __pthread_unwind → abort(). The Honcho SDK's HTTP calls run on
# daemon worker threads, so we must join EVERY outstanding one here
# before the interpreter begins teardown. See CPython gh-97940 /
# bpo-20526.
#
# Closing the Honcho SDK's underlying httpx.Client is the key step:
# it interrupts any worker thread blocked in sock_recv (httpx raises
# a connection-closed error that the worker's try/except absorbs),
# so the subsequent joins actually complete instead of timing out
# against a 30s read poll.
if self._init_thread and self._init_thread.is_alive():
self._init_thread.join(timeout=2.0)

for t in (self._prefetch_thread, self._sync_thread):
if t and t.is_alive():
t.join(timeout=2.0)

if self._manager:
if not (self._init_thread and self._init_thread.is_alive()):
try:
self._manager.flush_all()
except Exception:
pass
# Stop the manager's async writer + tracked prefetch threads.
try:
self._manager.shutdown()
except Exception:
pass

# Close the Honcho httpx.Client to unblock any worker still in recv,
# then join the tracked threads once more so they exit cleanly.
self._close_honcho_http_client()
for t in (self._init_thread, self._prefetch_thread, self._sync_thread):
if t and t.is_alive():
t.join(timeout=5.0)
# Flush any remaining messages
if self._manager and not (self._init_thread and self._init_thread.is_alive() and not self._session_initialized):
if self._manager:
try:
self._manager.flush_all()
self._manager.shutdown()
except Exception:
pass

def _close_honcho_http_client(self) -> None:
"""Close the Honcho SDK's httpx.Client to interrupt in-flight recvs.

The honcho-ai SDK stores its sync HTTP client on ``Honcho._http``
(a ``HonchoHTTPClient`` wrapping ``httpx.Client``). Closing it
forces any worker thread blocked in a socket recv to error out,
which is required for a clean interpreter shutdown.
"""
client = getattr(self, "_manager", None)
client = getattr(client, "_honcho", None) if client else None
if client is None:
return
for attr in ("_http", "_async_http"):
http_client = getattr(client, attr, None)
if http_client is None:
continue
closer = getattr(http_client, "close", None)
if callable(closer):
try:
closer()
except Exception:
pass


# ---------------------------------------------------------------------------
# Plugin entry point
Expand Down
29 changes: 28 additions & 1 deletion plugins/memory/honcho/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ def __init__(
# Async write queue — started lazily on first enqueue
self._async_queue: queue.Queue | None = None
self._async_thread: threading.Thread | None = None
# Tracked fire-and-forget context-prefetch threads, joined in shutdown()
# so none is left blocked in HTTP recv at interpreter teardown (which
# aborts CPython — see HonchoMemoryProvider.shutdown).
self._context_prefetch_threads: list[threading.Thread] = []
if write_frequency == "async":
self._async_queue = queue.Queue()
self._async_thread = threading.Thread(
Expand Down Expand Up @@ -543,11 +547,27 @@ def flush_all(self) -> None:
break

def shutdown(self) -> None:
"""Gracefully shut down the async writer thread."""
"""Gracefully shut down background worker threads.

Joins the async writer and any in-flight context-prefetch threads so
none is left blocked in HTTP recv at interpreter teardown (which would
abort CPython via PyThread_exit_thread → __pthread_unwind → abort()).
"""
if self._async_queue is not None and self._async_thread is not None:
self.flush_all()
self._async_queue.put(_ASYNC_SHUTDOWN)
self._async_thread.join(timeout=10)
for t in self._context_prefetch_threads:
if t.is_alive():
t.join(timeout=5.0)
# Retain any thread still alive after the timed join. The provider's
# shutdown() closes the httpx client and then calls this again to
# unblock workers stuck in recv; clearing unconditionally would drop
# those references and leave the threads alive until interpreter
# teardown — the original SIGABRT failure mode.
self._context_prefetch_threads = [
t for t in self._context_prefetch_threads if t.is_alive()
]

def delete(self, key: str) -> bool:
"""Delete a session from local cache."""
Expand Down Expand Up @@ -673,6 +693,13 @@ def _run():
self.set_context_result(session_key, result)

t = threading.Thread(target=_run, name="honcho-context-prefetch", daemon=True)
# Track so shutdown() can join it before interpreter teardown. Prune
# already-finished threads first so the list stays bounded over a
# long-lived session instead of growing once per prefetch.
self._context_prefetch_threads = [
existing for existing in self._context_prefetch_threads if existing.is_alive()
]
self._context_prefetch_threads.append(t)
t.start()

def set_context_result(self, session_key: str, result: dict[str, str]) -> None:
Expand Down
77 changes: 77 additions & 0 deletions tests/cli/test_oneshot_memory_cleanup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
"""Regression tests for the oneshot (``hermes -z``) memory-provider cleanup.

Oneshot bypasses the interactive CLI's ``atexit``/``_run_cleanup`` wiring, so
``hermes_cli.oneshot._register_oneshot_memory_cleanup`` registers its own
``atexit`` hook to tear the memory provider down before interpreter finalize.
Without it, providers with daemon worker threads still blocked in HTTP recv
(e.g. Honcho) crash the process with SIGABRT (exit 134) at ``Py_FinalizeEx``.

These tests assert the hook is registered and, when invoked, forwards the
agent's transcript to ``shutdown_memory_provider`` (mirroring ``_run_cleanup``,
#15165) before exiting the process.
"""

from __future__ import annotations

from unittest.mock import MagicMock, patch

from hermes_cli import oneshot


def _capture_registered_hook(agent):
"""Register the cleanup hook against ``agent`` and return the callback."""
with patch("atexit.register") as mock_register:
oneshot._register_oneshot_memory_cleanup(agent)
mock_register.assert_called_once()
return mock_register.call_args.args[0]


def test_registers_atexit_hook():
"""``_register_oneshot_memory_cleanup`` registers exactly one atexit hook."""
agent = MagicMock()
hook = _capture_registered_hook(agent)
assert callable(hook)


def test_hook_forwards_session_transcript():
"""The hook forwards a populated ``_session_messages`` list (#15165)."""
transcript = [
{"role": "user", "content": "remember my cat is named Mochi"},
{"role": "assistant", "content": "Got it — Mochi."},
]
agent = MagicMock()
agent._session_messages = transcript

hook = _capture_registered_hook(agent)
with patch("os._exit") as mock_exit:
hook()

agent.shutdown_memory_provider.assert_called_once_with(transcript)
mock_exit.assert_called_once_with(0)


def test_hook_falls_back_to_no_arg_when_transcript_missing():
"""A non-list ``_session_messages`` (test stub) keeps no-arg behaviour."""
agent = MagicMock()
agent._session_messages = None

hook = _capture_registered_hook(agent)
with patch("os._exit") as mock_exit:
hook()

agent.shutdown_memory_provider.assert_called_once_with()
mock_exit.assert_called_once_with(0)


def test_hook_exits_even_if_shutdown_raises():
"""A provider whose ``shutdown_memory_provider`` raises must not prevent
the direct ``os._exit`` that bypasses interpreter finalization."""
agent = MagicMock()
agent._session_messages = []
agent.shutdown_memory_provider.side_effect = RuntimeError("boom")

hook = _capture_registered_hook(agent)
with patch("os._exit") as mock_exit:
hook()

mock_exit.assert_called_once_with(0)