Skip to content

fix(daemon): snapshot state before threaded save to end tick crash#47

Open
Marsu6996 wants to merge 1 commit into
CodeAbra:mainfrom
Marsu6996:fix/save-state-snapshot-race
Open

fix(daemon): snapshot state before threaded save to end tick crash#47
Marsu6996 wants to merge 1 commit into
CodeAbra:mainfrom
Marsu6996:fix/save-state-snapshot-race

Conversation

@Marsu6996

@Marsu6996 Marsu6996 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Problem

save_state is offloaded to a worker thread (asyncio.to_thread(save_state, state)) while the event loop keeps mutating the same state dict. json.dump iterating that live dict can raise:

tick failed: dictionary changed size during iteration
  daemon/__init__.py  json.dump(state, f, indent=2)
RuntimeError: dictionary changed size during iteration

The exception propagates out of the scheduler tick, so consolidation silently stops: episodes keep being captured, but they're never consolidated/indexed, and memory_recall only returns older, already-consolidated memories. It shows up under concurrent load (several MCP wrappers hitting the daemon at once), which is why it can look fine on a lightly-used setup and stall on a busy one.

Fix

Add a small _persist() helper that takes an atomic snapshot in the event-loop thread before offloading:

async def _persist(state: dict) -> None:
    snapshot = copy.deepcopy(state)          # synchronous in the loop -> no coroutine interleaves
    await asyncio.to_thread(save_state, snapshot)

deepcopy runs synchronously in the loop thread, so no other coroutine can mutate state mid-copy; the writer thread then serialises an isolated snapshot. All save_state offload sites route through it. State is JSON-serialisable so the copy is cheap.

Test

tests/test_persist_state_snapshot.py — asserts the writer receives a deep copy (not the live dict) and that mutating state after the snapshot can't change what lands on disk. Fails if a call site reverts to passing the live state.

No version bump — leaving the release to you. Happy to rebase/trim or let you reimplement, whatever's least work.


🤖 Designed by Marsu — Refined by Claude

save_state ran in a worker thread (via asyncio.to_thread) over the live
`state` dict while the event loop kept mutating it, so json.dump could hit
"RuntimeError: dictionary changed size during iteration". That crash
propagated out of the scheduler tick and silently froze consolidation:
episodes kept being captured but never got consolidated/indexed, so recall
only ever returned older, already-consolidated memories. It surfaces under
concurrent load (several MCP wrappers hitting the daemon at once).

Route every threaded save through a new _persist() helper that deepcopies
`state` synchronously in the event-loop thread — atomic w.r.t. other
coroutines — then hands the isolated snapshot to the writer thread. Covers
all save_state offload sites. Adds a regression test.

Designed by Marsu — Refined by Claude.

Co-Authored-By: Claude <noreply@anthropic.com>
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