hermes - ✅(Solved) Fix bug: TOCTOU race in get_plugin_manager() allows duplicate PluginManager construction [2 pull requests, 1 comments, 2 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
NousResearch/hermes-agent#24754Fetched 2026-05-14 03:52:01
View on GitHub
Comments
1
Participants
2
Timeline
7
Reactions
0
Timeline (top)
labeled ×4cross-referenced ×2commented ×1

hermes_cli/plugins.py exposes get_plugin_manager() — a lazy-init accessor for the process-wide _plugin_manager singleton (line 1286–1294). The pattern is:

_plugin_manager: Optional[PluginManager] = None

def get_plugin_manager() -> PluginManager:
    global _plugin_manager
    if _plugin_manager is None:
        _plugin_manager = PluginManager()
    return _plugin_manager

There is no lock around the check-then-initialize sequence. When two coroutines or threads call get_plugin_manager() concurrently before the singleton is set, both can pass the is None guard and each construct a fresh PluginManager. The second write overwrites the first, silently discarding any hooks or state already registered on the first instance.

threading is already imported at line 44 of the file (used for threading.Event and threading.Thread in the voice-recording subsystem), so the fix is a one-line lock declaration + double-checked locking guard — no new imports needed.

Root Cause

hermes_cli/plugins.py exposes get_plugin_manager() — a lazy-init accessor for the process-wide _plugin_manager singleton (line 1286–1294). The pattern is:

_plugin_manager: Optional[PluginManager] = None

def get_plugin_manager() -> PluginManager:
    global _plugin_manager
    if _plugin_manager is None:
        _plugin_manager = PluginManager()
    return _plugin_manager

There is no lock around the check-then-initialize sequence. When two coroutines or threads call get_plugin_manager() concurrently before the singleton is set, both can pass the is None guard and each construct a fresh PluginManager. The second write overwrites the first, silently discarding any hooks or state already registered on the first instance.

threading is already imported at line 44 of the file (used for threading.Event and threading.Thread in the voice-recording subsystem), so the fix is a one-line lock declaration + double-checked locking guard — no new imports needed.

Fix Action

Fix

Add _plugin_manager_lock = threading.Lock() and apply double-checked locking in get_plugin_manager().

PR fix notes

PR #24757: fix(plugins): prevent TOCTOU race in get_plugin_manager()

Description (problem / solution / changelog)

Summary

  • get_plugin_manager() in hermes_cli/plugins.py used an unguarded check-then-initialize pattern for the _plugin_manager singleton. Two concurrent callers could both pass the is None guard and construct separate PluginManager instances; the second would overwrite the first, silently dropping any hooks already registered on it.
  • Added _plugin_manager_lock = threading.Lock() (one line — threading was already imported at line 44) and applied double-checked locking: fast lock-free early return when already set, re-checks under the lock before constructing.
  • Added TestGetPluginManagerToctouRace (2 tests): 50-thread barrier verifying all concurrent callers receive the same instance, and a spy subclass confirming the constructor fires exactly once.

Closes #24754

Test plan

  • uv run python -m pytest tests/hermes_cli/test_plugins.py::TestGetPluginManagerToctouRace -v — 2 passed
  • Existing test_plugins.py suite unaffected

🤖 Generated with Claude Code

Changed files

  • hermes_cli/plugins.py (modified, +4/-1)
  • tests/hermes_cli/test_plugins.py (modified, +57/-0)

PR #24764: fix(plugins): lock get_plugin_manager() singleton init against concurrent calls

Description (problem / solution / changelog)

Problem

get_plugin_manager() in hermes_cli/plugins.py uses an unguarded check-then-init pattern:

if _plugin_manager is None:
    _plugin_manager = PluginManager()

Two threads racing before the singleton is set both pass the is None guard, each construct a PluginManager, and the second write silently discards any hooks or state registered on the first instance.

Fix

Add _plugin_manager_lock = threading.Lock() at module level (threading is already imported at line 44) and apply double-checked locking in get_plugin_manager():

  • Outer unlocked check keeps the fast-path free once the singleton is set.
  • Inner locked check prevents duplicate construction on first init.

Test

Added TestGetPluginManagerConcurrency.test_concurrent_calls_return_same_instance: 8 threads synchronised on a Barrier all call get_plugin_manager() simultaneously and assert every call returns the same object (id equality).

Fixes #24754

🤖 Generated with Claude Code

Changed files

  • hermes_cli/plugins.py (modified, +4/-1)
  • tests/hermes_cli/test_plugins.py (modified, +28/-0)

Code Example

_plugin_manager: Optional[PluginManager] = None

def get_plugin_manager() -> PluginManager:
    global _plugin_manager
    if _plugin_manager is None:
        _plugin_manager = PluginManager()
    return _plugin_manager

---

import threading
from concurrent.futures import ThreadPoolExecutor
import hermes_cli.plugins as _plugins_mod
from hermes_cli.plugins import get_plugin_manager

_plugins_mod._plugin_manager = None
barrier = threading.Barrier(20)
results = []

def worker():
    barrier.wait()
    results.append(get_plugin_manager())

with ThreadPoolExecutor(max_workers=20) as pool:
    list(pool.map(lambda _: worker(), range(20)))

assert len({id(r) for r in results}) == 1, "multiple PluginManager instances constructed"
RAW_BUFFERClick to expand / collapse

Description

hermes_cli/plugins.py exposes get_plugin_manager() — a lazy-init accessor for the process-wide _plugin_manager singleton (line 1286–1294). The pattern is:

_plugin_manager: Optional[PluginManager] = None

def get_plugin_manager() -> PluginManager:
    global _plugin_manager
    if _plugin_manager is None:
        _plugin_manager = PluginManager()
    return _plugin_manager

There is no lock around the check-then-initialize sequence. When two coroutines or threads call get_plugin_manager() concurrently before the singleton is set, both can pass the is None guard and each construct a fresh PluginManager. The second write overwrites the first, silently discarding any hooks or state already registered on the first instance.

threading is already imported at line 44 of the file (used for threading.Event and threading.Thread in the voice-recording subsystem), so the fix is a one-line lock declaration + double-checked locking guard — no new imports needed.

Impact

Concurrent agent sessions that share a process (e.g. CLI with background workers) can trigger double plugin discovery, duplicate lifecycle hook calls, or lost hook registrations — all silent and hard to reproduce.

Steps to reproduce

import threading
from concurrent.futures import ThreadPoolExecutor
import hermes_cli.plugins as _plugins_mod
from hermes_cli.plugins import get_plugin_manager

_plugins_mod._plugin_manager = None
barrier = threading.Barrier(20)
results = []

def worker():
    barrier.wait()
    results.append(get_plugin_manager())

with ThreadPoolExecutor(max_workers=20) as pool:
    list(pool.map(lambda _: worker(), range(20)))

assert len({id(r) for r in results}) == 1, "multiple PluginManager instances constructed"

Expected

Single PluginManager instance constructed regardless of concurrency.

Fix

Add _plugin_manager_lock = threading.Lock() and apply double-checked locking in get_plugin_manager().

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING