fix(honcho): fully shut down async writer + close HTTP client on exit - #7627
fix(honcho): fully shut down async writer + close HTTP client on exit#7627btcjon wants to merge 1 commit into
Conversation
HonchoMemoryProvider.shutdown() only called flush_all(), which left the async writer thread alive and the underlying httpx connection pool open. At interpreter teardown this race caused intermittent crashes. Now calls HonchoSessionManager.shutdown() (flush + _ASYNC_SHUTDOWN + join) and explicitly closes honcho._http so the httpx pool is torn down before Python finalization.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing deterministic Honcho teardown. The current provider still calls only flush_all() at plugins/memory/honcho/__init__.py:1421, while HonchoSessionManager.shutdown() owns the async-writer sentinel and join at plugins/memory/honcho/session.py:548-553, so the underlying target is real.
Problems
- The replacement at this PR's right-side line 707 regresses
writeFrequency: "session":HonchoSessionManager.shutdown()only callsflush_all()when an async queue/thread exists (plugins/memory/honcho/session.py:550-553), but session mode has no queue and intentionally defers writes toflush_all()(plugins/memory/honcho/session.py:517-519). Preserve the provider's final flush for every mode. - This one-file PR adds no regression tests. Existing tests cover manager async shutdown (
tests/honcho_plugin/test_async_memory.py:291-336), not provider shutdown or the privateclient._http.close()path. - The unchanged 5-second provider prefetch join (
plugins/memory/honcho/__init__.py:1415-1417) remains relevant to the timeout mismatch documented by cross-referenced issue #33485.
Suggested changes
- Move the universal flush into
HonchoSessionManager.shutdown()before its async-worker conditional, then test session-mode flush, writer termination, and idempotent HTTP close. - Reconcile the prefetch-thread join with #33485 before closing the shared client.
Automated hermes-sweeper review.
| except Exception: | ||
| pass | ||
| self._manager.shutdown() | ||
| except Exception as e: |
There was a problem hiding this comment.
HonchoSessionManager.shutdown() currently invokes flush_all() only when an async queue/thread exists (plugins/memory/honcho/session.py:550-553). For supported writeFrequency: "session", no queue exists and writes defer until flush_all() (session.py:517-519), so this replacement removes the provider's final flush. Make manager shutdown flush all modes unconditionally, or retain that flush here before invoking the worker teardown.
Problem
HonchoMemoryProvider.shutdown()only calledflush_all(), which leaves two things alive at interpreter teardown:honcho-async-writerbackground thread (owned byHonchoSessionManager) —flush_all()drains the queue but never sends_ASYNC_SHUTDOWNand never joins the thread.httpx.Clientinside the Honcho SDK (honcho._http._client) — its connection pool is still open when Python finalizes.This race caused intermittent crashes during clean exit.
Fix
HonchoMemoryProvider.shutdown()now:HonchoSessionManager.shutdown()(which already does flush →_ASYNC_SHUTDOWN→join).honcho._httpso the httpx pool is torn down before finalization.Both wrapped in
try/exceptwith debug logging — shutdown must never raise.Verification
Offline smoke test with a mocked Honcho client:
Full honcho test suite: 141 passed, 3 skipped.
Sibling memory plugins (byterover, hindsight, holographic) were audited for the same pattern — none affected. supermemory is not installed in this environment.