Skip to content

fix(memory): concurrent writes silently drop entries — add file locking - #1726

Merged
teknium1 merged 1 commit into
mainfrom
fix/memory-tool-file-locking
Mar 17, 2026
Merged

fix(memory): concurrent writes silently drop entries — add file locking#1726
teknium1 merged 1 commit into
mainfrom
fix/memory-tool-file-locking

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Two concurrent gateway sessions calling memory add/replace/remove simultaneously could both read the old state, apply their changes independently, and write back. The last writer's os.replace() would silently overwrite the first writer's changes.

What changed

  • tools/memory_tool.py: All three mutating methods (add, replace, remove) now:
    1. Acquire an exclusive file lock (fcntl.flock on a .lock file)
    2. Re-read entries from disk to get the latest state
    3. Apply the mutation
    4. Write to disk (still under lock)
    5. Release lock

The lock uses a separate .lock file since the memory file itself is atomically replaced via os.replace() (can't flock a file that gets replaced). Readers remain lock-free — atomic rename ensures they always see a complete file.

Added helpers: _file_lock() context manager, _path_for(), _reload_target().

Test plan

  • python -m pytest tests/ -n0 -q -k memory → 108 passed, 2 skipped ✔
  • Concurrent memory writes from multiple gateway sessions no longer lose entries

Two concurrent gateway sessions calling memory add/replace/remove
simultaneously could both read the old state, apply their changes
independently, and write — the last writer silently drops the first
writer's entry.

Fix: wrap each mutation in a file lock (fcntl.flock on a .lock file).
Under the lock, re-read entries from disk to get the latest state,
apply the mutation, then write. This ensures concurrent writers
serialize properly.

The lock uses a separate .lock file since the memory file itself is
atomically replaced via os.replace() (can't flock a replaced file).
Readers remain lock-free since atomic rename ensures they always see
a complete file.
@teknium1
teknium1 merged commit 21b823d into main Mar 17, 2026
1 check passed
angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
…l-file-locking

fix(memory): concurrent writes silently drop entries — add file locking
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…l-file-locking

fix(memory): concurrent writes silently drop entries — add file locking
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…l-file-locking

fix(memory): concurrent writes silently drop entries — add file locking
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…l-file-locking

fix(memory): concurrent writes silently drop entries — add file locking
michielhdoteth added a commit to michielhdoteth/hermes-agent that referenced this pull request Aug 27, 2026
…data loss

save_to_disk() previously wrote self._entries_for(target) without
re-reading the file, so a session's stale in-memory snapshot could
overwrite entries from concurrent writers (other sessions, daemons,
cron jobs, scripts) — issue NousResearch#85858.

The fix re-reads the file under the already-held lock and merges
on-disk entries with pending entries before writing. This preserves
writes from external writers that don't use the memory tool's lock
(the primary gap) and adds defense-in-depth for concurrent sessions
(beyond PR NousResearch#1726's file locking).

Merge strategy: union-where-session-wins — all pending entries
preserved, on-disk entries not in pending appended, no duplicates.
Conflicts (same entry modified by both) result in both versions
present — safe, no data loss.

Closes NousResearch#85858

Co-Authored-By: Hermes Agent <noreply@nousresearch.com>
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…l-file-locking

fix(memory): concurrent writes silently drop entries — add file locking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant