fix(plugins): thread-safe lazy-singleton helpers; fix honcho TOCTOU (#24759)#42150
Merged
Conversation
…OU (#24759) get_honcho_client() and fal's _load_fal_client() used unlocked check-then-init: racing threads both ran the expensive build and the loser's client (open connection) leaked. Rather than one-off locks, add plugins/plugin_utils.py with two reusable primitives every plugin author can drop in: - lazy_singleton: decorator for zero-arg accessors - SingletonSlot: manual slot for config-keyed accessors (first wins) Both use double-checked locking; factory runs at most once; failed builds aren't cached. honcho is the reference consumer; fal's sibling TOCTOU gets a matching double-checked lock. Plugin dev guide documents the pattern so future plugins don't reintroduce the race. Closes #24759
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
3 |
unresolved-import |
2 |
invalid-return-type |
1 |
First entries
tests/test_plugin_utils.py:42: [unresolved-attribute] unresolved-attribute: Object of type `() -> Unknown` has no attribute `reset`
tests/test_honcho_client_concurrency.py:40: [unresolved-attribute] unresolved-attribute: Unresolved attribute `Honcho` on type `ModuleType`
tests/test_plugin_utils.py:10: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
plugins/plugin_utils.py:118: [invalid-return-type] invalid-return-type: Return type does not match returned value: expected `T@SingletonSlot`, found `T@SingletonSlot | None`
tests/test_honcho_client_concurrency.py:11: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
plugins/plugin_utils.py:80: [unresolved-attribute] unresolved-attribute: Unresolved attribute `reset` on type `_Wrapped[(), T@lazy_singleton, (), T@lazy_singleton]`
✅ Fixed issues: none
Unchanged: 5432 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
test_reset_clears_singleton poked the removed _honcho_client module global directly. Assert through the slot's public peek() surface instead, matching the #24759 refactor.
Contributor
|
Code Review — Clean ✅ Reviewed the full diff (7 files, ~29K chars). Findings: Concurrency guard ( Honcho client migration: The Falcon video plugin: Same pattern applied to Test coverage: No issues found. |
jhjaggars-hermes
pushed a commit
to jhjaggars/hermes-agent
that referenced
this pull request
Jun 8, 2026
…ousResearch#24759) (NousResearch#42150) * fix(plugins): add thread-safe lazy-singleton helpers, fix honcho TOCTOU (NousResearch#24759) get_honcho_client() and fal's _load_fal_client() used unlocked check-then-init: racing threads both ran the expensive build and the loser's client (open connection) leaked. Rather than one-off locks, add plugins/plugin_utils.py with two reusable primitives every plugin author can drop in: - lazy_singleton: decorator for zero-arg accessors - SingletonSlot: manual slot for config-keyed accessors (first wins) Both use double-checked locking; factory runs at most once; failed builds aren't cached. honcho is the reference consumer; fal's sibling TOCTOU gets a matching double-checked lock. Plugin dev guide documents the pattern so future plugins don't reintroduce the race. Closes NousResearch#24759 * test(honcho): update reset test for SingletonSlot internals test_reset_clears_singleton poked the removed _honcho_client module global directly. Assert through the slot's public peek() surface instead, matching the NousResearch#24759 refactor.
changman
pushed a commit
to changman/hermes-agent
that referenced
this pull request
Jun 10, 2026
…ousResearch#24759) (NousResearch#42150) * fix(plugins): add thread-safe lazy-singleton helpers, fix honcho TOCTOU (NousResearch#24759) get_honcho_client() and fal's _load_fal_client() used unlocked check-then-init: racing threads both ran the expensive build and the loser's client (open connection) leaked. Rather than one-off locks, add plugins/plugin_utils.py with two reusable primitives every plugin author can drop in: - lazy_singleton: decorator for zero-arg accessors - SingletonSlot: manual slot for config-keyed accessors (first wins) Both use double-checked locking; factory runs at most once; failed builds aren't cached. honcho is the reference consumer; fal's sibling TOCTOU gets a matching double-checked lock. Plugin dev guide documents the pattern so future plugins don't reintroduce the race. Closes NousResearch#24759 * test(honcho): update reset test for SingletonSlot internals test_reset_clears_singleton poked the removed _honcho_client module global directly. Assert through the slot's public peek() surface instead, matching the NousResearch#24759 refactor.
alt-glitch
pushed a commit
that referenced
this pull request
Jun 14, 2026
…24759) (#42150) * fix(plugins): add thread-safe lazy-singleton helpers, fix honcho TOCTOU (#24759) get_honcho_client() and fal's _load_fal_client() used unlocked check-then-init: racing threads both ran the expensive build and the loser's client (open connection) leaked. Rather than one-off locks, add plugins/plugin_utils.py with two reusable primitives every plugin author can drop in: - lazy_singleton: decorator for zero-arg accessors - SingletonSlot: manual slot for config-keyed accessors (first wins) Both use double-checked locking; factory runs at most once; failed builds aren't cached. honcho is the reference consumer; fal's sibling TOCTOU gets a matching double-checked lock. Plugin dev guide documents the pattern so future plugins don't reintroduce the race. Closes #24759 * test(honcho): update reset test for SingletonSlot internals test_reset_clears_singleton poked the removed _honcho_client module global directly. Assert through the slot's public peek() surface instead, matching the #24759 refactor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Plugin authors get two reusable thread-safe lazy-singleton primitives, so the check-then-init TOCTOU race in
get_honcho_client()(#24759) — and its sibling in fal — can't recur.Root cause:
get_honcho_client()and fal's_load_fal_client()didif x is not None: return xthen an unlocked expensive build + assign. Hermes runs multiple threads per process (delegated calls, background workers, the self-improvement fork), so racing first-callers both built a client and the loser's open connection leaked.Changes
plugins/plugin_utils.py(new):lazy_singletondecorator (zero-arg accessors) +SingletonSlot(config-keyed accessors, first-wins). Both use double-checked locking, run the factory at most once, and don't cache failed builds. Stdlib-only so any plugin can import it.plugins/memory/honcho/client.py:get_honcho_client()now routes the build through aSingletonSlot; validation still raises before the slot;reset_honcho_client()delegates to the slot. (Large diff is mostly the +4 reindent of the existing build body into the factory closure.)plugins/video_gen/fal/__init__.py:_load_fal_client()gets a matching double-checked lock (it reads the module global directly at two call sites, so an in-file lock is the lowest-risk fix there).website/docs/guides/build-a-hermes-plugin.md: new "Thread-safe lazy singletons" section next to the lazy-install guidance, pointing authors at the helpers.Validation
tests/test_plugin_utils.py(incl. 16-thread barrier stress for both primitives)tests/test_honcho_client_concurrency.py(20-thread race → exactly 1 build)tests/test_honcho_client_config.py(existing, no regression)tests/plugins/video_gen/test_fal_plugin.py(existing, no regression)plugin_utils+ honcho + fal)Closes #24759
Infographic