fix(oneshot): shut down memory provider to prevent SIGABRT on exit (Honcho crash) - #49498
fix(oneshot): shut down memory provider to prevent SIGABRT on exit (Honcho crash)#49498koshaji wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a oneshot-mode shutdown gap where external memory providers (notably Honcho) could leave daemon threads blocked in C-level I/O during interpreter teardown, leading to SIGABRT on exit. The PR adds oneshot cleanup and strengthens Honcho provider shutdown/thread tracking to enable a clean exit.
Changes:
- Register an
atexitcleanup inhermes_cli/oneshot.pyto shut down the agent’s memory provider on process exit. - Track Honcho “fire-and-forget” context-prefetch threads in
HonchoSessionManagerand join them during shutdown. - Harden Honcho provider shutdown to join worker threads and close the underlying Honcho SDK HTTP client to unblock in-flight reads.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
plugins/memory/honcho/session.py |
Adds tracking/joining of context-prefetch threads during shutdown. |
plugins/memory/honcho/__init__.py |
Expands Honcho provider shutdown logic to join worker threads and close the Honcho HTTP client. |
hermes_cli/oneshot.py |
Adds an atexit hook in oneshot mode to shut down the memory provider before interpreter finalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for t in self._context_prefetch_threads: | ||
| if t.is_alive(): | ||
| t.join(timeout=5.0) | ||
| self._context_prefetch_threads.clear() |
| t = threading.Thread(target=_run, name="honcho-context-prefetch", daemon=True) | ||
| # Track so shutdown() can join it before interpreter teardown. | ||
| self._context_prefetch_threads.append(t) | ||
| t.start() |
| def _shutdown_oneshot_memory() -> None: | ||
| try: | ||
| if hasattr(agent, "shutdown_memory_provider"): | ||
| agent.shutdown_memory_provider() | ||
| except Exception: | ||
| pass |
| # 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. Register a direct atexit hook here | ||
| # so the memory provider is torn down cleanly before finalize. | ||
| import atexit as _atexit | ||
|
|
||
| def _shutdown_oneshot_memory() -> None: | ||
| try: | ||
| if hasattr(agent, "shutdown_memory_provider"): | ||
| agent.shutdown_memory_provider() | ||
| except Exception: | ||
| pass | ||
|
|
||
| _atexit.register(_shutdown_oneshot_memory) |
In oneshot mode (`hermes -z`), the memory provider's shutdown() was never called — only the interactive CLI registers the atexit cleanup. Providers with daemon worker threads blocked in C-level I/O at interpreter exit (e.g. Honcho's httpx socket recv) are then forcibly killed by CPython at Py_FinalizeEx (PyThread_exit_thread → __pthread_unwind → abort()), producing SIGABRT (exit 134) with no Python traceback. Built-in memory is unaffected (no daemon threads). Fix: - hermes_cli/oneshot.py: register an atexit hook in _run_agent() calling agent.shutdown_memory_provider(), mirroring the interactive CLI's _run_cleanup. This is the general fix for any provider with background threads. - plugins/memory/honcho: harden shutdown() to join all daemon threads (init/prefetch/sync) and close the SDK's underlying httpx.Client, which interrupts any worker still blocked in recv so the joins actually complete instead of racing a 30s read poll. Track fire-and-forget context-prefetch threads in HonchoSessionManager so they can be joined. Verified: `hermes -z` with Honcho enabled now exits 0 (was 134) across multiple runs/prompts; built-in memory still exits 0 (no regression). Refs: CPython gh-97940 / bpo-20526 (daemon threads blocked in C I/O at interpreter exit abort the process). Co-Authored-By: Claude <noreply@anthropic.com>
c50aeca to
38fb1c8
Compare
|
Update: strengthened the oneshot fix to also cover the multi-provider case. The graceful shutdown alone resolves Honcho-only, but I found a residual SIGABRT when Honcho + a cloud-browser provider (Browser Use) are both active — Honcho's daemon worker threads (
for _stream in (sys.stdout, sys.stderr): _stream.flush()
os._exit(0)i.e. once the oneshot has emitted its output and attempted graceful teardown, it exits directly to bypass Verified: |
Addresses the four review comments on NousResearch#49498: - oneshot.py: forward the agent's `_session_messages` transcript to `shutdown_memory_provider()` so providers' `on_session_end()` hooks see the real conversation, mirroring the interactive CLI's `_run_cleanup` (NousResearch#15165). Extract the atexit registration into a module-level `_register_oneshot_memory_cleanup(agent)` helper so it is unit-testable. - honcho/session.py: in `shutdown()`, retain threads still alive after the timed join instead of clearing unconditionally, so the provider's post-httpx-close retry can re-join workers that were blocked in recv (clearing dropped the references and re-exposed the SIGABRT failure mode). - honcho/session.py: prune finished threads before appending in `prefetch_context()` so `_context_prefetch_threads` stays bounded over a long-lived session. - tests: add tests/cli/test_oneshot_memory_cleanup.py covering hook registration, transcript forwarding, no-arg fallback, and exit-on-raise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed a follow-up commit addressing all four review comments:
Ready for another look. |
|
Superseded by #61875 — cleaner approach using atexit hook without the _os._exit(0) bypass. Closing this in favor of the rebased version. |
Problem
Running
hermes -z(oneshot mode) with a memory provider that spawns daemon worker threads — e.g. Honcho — crashes on exit with SIGABRT (exit 134) and no Python traceback:Built-in memory (
provider: '') is unaffected.Root cause
The interactive CLI registers an
atexithandler (_run_cleanupincli.py) that callsagent.shutdown_memory_provider(), buthermes_cli/oneshot.pydoes not —_run_agent()builds itsAIAgentdirectly andsys.exit(run_oneshot(...))without registering that cleanup.So when the provider has daemon threads still blocked in C-level I/O at interpreter exit (Honcho's httpx socket
recv), CPython forcibly kills them duringPy_FinalizeExviaPyThread_exit_thread → __pthread_unwind → abort(). Confirmed by gdb: the aborting stack isPy_FinalizeEx → PyGC_Collect → … → take_gil → PyThread_exit_thread → __pthread_unwind → abort(), with worker threads blocked in__GI___poll → sock_recv.This is the well-known CPython daemon-thread-at-exit issue (gh-97940 / bpo-20526).
Fix
hermes_cli/oneshot.py(general fix): register anatexithook in_run_agent()that callsagent.shutdown_memory_provider()before the interpreter finalizes — mirroring the interactive CLI. This protects any memory provider that uses background threads (Honcho today; potentially mem0/retaindb/etc.).plugins/memory/honcho(defensive hardening, soshutdown()actually unblocks the workers):HonchoMemoryProvider.shutdown().httpx.Client(manager._honcho._http) — this interrupts any worker still blocked insock_recv(httpx raises a connection-closed error the worker absorbs), so the subsequentjoin(timeout=...)calls actually complete instead of racing a 30s read poll.honcho-context-prefetchthreads inHonchoSessionManagerso they can be joined inshutdown().Verification
hermes -zwith Honcho enabled: exit 0 (was 134), reproduced across 3 runs with different prompts.hermes -zwith built-in memory: exit 0 (no regression).memory.providerleft at''(built-in) in the repro so the gateway stays safe; users can set it tohonchoafter this fix.Notes
The Honcho server itself was healthy throughout (it built a correct user model via its LLM); the crash was entirely on the Hermes side at session teardown. The oneshot-vs-interactive cleanup asymmetry is the real bug.