Skip to content

fix(oneshot): join memory daemon threads before process exit (#37632) - #37635

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/oneshot-memory-shutdown
Open

fix(oneshot): join memory daemon threads before process exit (#37632)#37635
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/oneshot-memory-shutdown

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Calls shutdown_memory_provider() after agent.chat() in the hermes -z one-shot path, joining Honcho memory daemon threads before process exit to prevent SIGABRT (exit 134) during Python interpreter finalization.

Related Issue

Fixes #37632

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/oneshot.py: Added agent.shutdown_memory_provider() call after agent.chat() in _run_agent(), wrapped in try/except since memory providers are best-effort. This joins daemon threads (honcho-sync, honcho-prewarm-dialectic, honcho-context-prefetch) with bounded timeouts before the process exits.
  • tests/cli/test_oneshot_memory_shutdown.py: Added regression tests verifying (1) shutdown_memory_provider() is called after chat, and (2) a raising shutdown_memory_provider does not crash _run_agent.

How to Test

  1. Enable Honcho memory provider in config.yaml
  2. Run hermes -z "reply with exactly: ok" --yolo > /dev/null 2>&1; echo "exit=$?"
  3. Before fix: exit=134 (SIGABRT). After fix: exit=0.
  4. Run pytest tests/cli/test_oneshot_memory_shutdown.py -v — both tests should pass.
  5. Run pytest tests/cli/test_cli_shutdown_memory_messages.py -v — existing tests still pass.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: hermes_cli/oneshot.py:_run_agent (callers: 2 — run_oneshot and Termux fast path)
  • Blast radius: LOW — change is isolated to the one-shot exit path, no behavioral change for interactive CLI or gateway
  • Related patterns: cli.py:959-970 calls shutdown_memory_provider on interactive CLI exit; gateway/run.py:3640-3655 calls it on gateway session expiry. This PR fills the missing one-shot gap.

…earch#37632)

shutdown_memory_provider() was never called on the hermes -z path.
Honcho's daemon threads (honcho-sync, honcho-prewarm-dialectic,
honcho-context-prefetch) remained blocked in httpx I/O when the
interpreter finalized, causing glibc abort() → SIGABRT → exit 134.

Add a shutdown_memory_provider() call after agent.chat() in
_run_agent(), wrapped in try/except since memory providers are
best-effort. This joins the daemon threads with bounded timeouts
before the process exits, preventing the finalization-time crash.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/memory Memory tool and memory providers labels Jun 2, 2026
@Morad37

Morad37 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Nice catch. The shutdown_memory_provider() call in oneshot mode closes the same SIGABRT gap that the gateway handles via a longer-lived process lifecycle.

The test mimics oneshot's early-return model via a nested try block:

try:
    return agent.chat(prompt) or ""
except Exception:
    return ""

That swallowed return before the fix — the early-return paths on lines 324 and 328 also skip cleanup. Might be worth adding shutdown_memory_provider() via a finally or context manager so it fires on all exit paths, not just the successful-completion path. Separately, the try/except after response = agent.chat(prompt) or "" could use contextlib.suppress since the catch is unconditional.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the one-shot lifecycle gap. Current main still has no memory-provider teardown in _run_agent: it calls agent.run_conversation(prompt) and returns directly at hermes_cli/oneshot.py:425-426.

Problems

  • The patch is stale: current main no longer uses agent.chat(), so the proposed edit and tests do not cover the live path.
  • Cleanup is after the successful agent call rather than in finally; an exception from current run_conversation() skips teardown and is caught only by run_oneshot (hermes_cli/oneshot.py:232-250).
  • The no-argument shutdown differs from current CLI/gateway cleanup, which forwards _session_messages (cli.py:1141, gateway/run.py:6055). The default becomes on_session_end([]) (run_agent.py:3323-3329).

Suggested changes

  • Salvage this around current run_conversation() with best-effort finally cleanup, preserving the original exception.
  • Forward a list-valued _session_messages transcript and add success, exception, and transcript-forwarding tests.
  • The Jun. 12 evidence in #37632 reports that a bounded join can still leave an aborting daemon thread alive; confirm deterministic provider termination before treating shutdown alone as conclusive.

Automated hermes-sweeper review.

Comment thread hermes_cli/oneshot.py
# still-running daemon threads via pthread_exit(), and glibc converts
# that forced unwind into abort() → SIGABRT → exit 134. (#37632)
try:
agent.shutdown_memory_provider()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be a finally around the agent execution. If agent.chat() raises, control escapes before this block and one-shot still exits with provider threads unshut down; current main's run_conversation() path has the same exception route.

Comment thread hermes_cli/oneshot.py
# that forced unwind into abort() → SIGABRT → exit 134. (#37632)
try:
agent.shutdown_memory_provider()
except Exception:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please forward the agent's list-valued _session_messages here, matching cli.py:1141 and gateway/run.py:6055; no argument causes shutdown_memory_provider() to call provider on_session_end([]).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes -z one-shot crashes with SIGABRT (exit 134) on exit: Honcho memory daemon threads blocked in httpx I/O during interpreter finalization

4 participants